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: MaxBytesReader's Read panics when n < -1 #45101

Closed
amwolff opened this issue Mar 18, 2021 · 5 comments
Closed

net/http: MaxBytesReader's Read panics when n < -1 #45101

amwolff opened this issue Mar 18, 2021 · 5 comments

Comments

@amwolff
Copy link
Contributor

@amwolff amwolff commented Mar 18, 2021

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

$ go version
go version go1.16.2 linux/amd64

Does this issue reproduce with the latest release?

Affirmative.

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

go env Output
$ go env

What did you do?

Passed n < -1 to the MaxBytesReader constructor.

https://play.golang.org/p/aFTkl-eFmOZ

It won't compile in the playground, not sure why (timeout running go build).

What did you expect to see?

An error; behaviour similar to how io.LimitReader behaves (https://play.golang.org/p/ZcUjYXNlGgR).

What did you see instead?

panic: runtime error: slice bounds out of range [:-1]

goroutine 1 [running]:
net/http.(*maxBytesReader).Read(0xc000121f40, 0xc000022350, 0x4, 0x4, 0xc000048710, 0x44164a, 0xc000026000)
        /usr/local/go/src/net/http/request.go:1147 +0x206
...

The fix would be pretty simple, although far from an elegant one:

func MaxBytesReader(w http.ResponseWriter, r io.ReadCloser, n int64) io.ReadCloser {
+	if n < -1 {
+		return &maxBytesReader{w: w, r: r, n: -1}
+	}
	return &maxBytesReader{w: w, r: r, n: n}
}

It was a bit unexpected for me, and if it's indeed an unexpected behaviour, I can work on this and provide some fix.

@cherrymui cherrymui added this to the Backlog milestone Mar 18, 2021
@cherrymui
Copy link
Contributor

@cherrymui cherrymui commented Mar 18, 2021

@neild
Copy link
Contributor

@neild neild commented Mar 19, 2021

The documentation states that "MaxByteReader is similar to io.LimitReader", and io.LimitedReader's documentation states that "Read returns EOF when N <= 0". Either the documentation or behavior of http.MaxByteReader is wrong.

I think that for consistency with io.LimitReader, MaxBytesReader should also treat negative limits as equivalent to 0. I'm happy to review a CL if you'd like to send me one.

@amwolff
Copy link
Contributor Author

@amwolff amwolff commented Mar 19, 2021

@neild Yes! Absolutely. I will work on this and send you a patch as soon as I have one.

@gopherbot
Copy link

@gopherbot gopherbot commented Mar 21, 2021

Change https://golang.org/cl/303171 mentions this issue: net/http: treat MaxBytesReader's negative limits equivalently to zero

@dmitshur dmitshur modified the milestones: Backlog, Go1.17 Mar 23, 2021
@gopherbot gopherbot closed this in 0525042 Mar 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants