Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net/http/httputil: NewChunkedReader reports incomplete uploads as complete #48861

Closed
omaskery opened this issue Oct 8, 2021 · 2 comments
Closed
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@omaskery
Copy link

omaskery commented Oct 8, 2021

What version of Go are you using (go version)?

$ go version
1.17.2

Issue presents in current Go Playground, at time of writing the About page describes it as version 1.17.2.

Issue appears present in the latest Go source code, and this line hasn't been changed for ~8 years according to Git Annotate.

Does this issue reproduce with the latest release?

Yes (based on using the Go Playground).

What operating system and processor architecture are you using (go env)?

go env Output
$ go env

The Go Playground environment.

What did you do?

Here is a test suite in the Go Playground that demonstrates the issue, with some surrounding boundary cases to show what currently does and does not work.

I believe the issue is this line of code.

Whilst the readChunkLine (link) method takes care to map io.EOF to an io.ErrUnexpectedEOF, it seems that the code that reads the actual "chunk" itself does not actually check for io.EOF and report it as an error.

What did you expect to see?

As demonstrated in the Go Playground link, I believe that an io.EOF from the underlying reader should, at any point, be mapped to io.ErrUnexpectedEOF unless it successfully received the 0-length chunk that indicates the end of the stream.

What did you see instead?

Whilst the current implementation does handle io.EOF correctly during a chunk header line, it erroneously returns io.EOF (not io.ErrUnexpectedEOF) when an io.EOF is encountered part-way through a chunk.

The importance of this bug is that this makes it dangerously ambiguous whether the upload was interrupted, or whether the upload completed successfully. Anyone reading from the chunked decoder will see a normal io.EOF on cancellation, think that the full body was received, and continue processing - unaware that they have a partial upload.

@seankhliao seankhliao changed the title net/http/httputil - NewChunkedReader reports incomplete uploads as complete net/http/httputil: NewChunkedReader reports incomplete uploads as complete Oct 8, 2021
@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 8, 2021
@seankhliao
Copy link
Member

cc @neild

@gopherbot
Copy link

Change https://golang.org/cl/355029 mentions this issue: net/http/internal: return unexpected EOF on incomplete chunk read

AlexanderYastrebov added a commit to AlexanderYastrebov/go that referenced this issue Oct 14, 2021
AlexanderYastrebov added a commit to AlexanderYastrebov/go that referenced this issue Oct 16, 2021
@golang golang locked and limited conversation to collaborators Oct 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants