-
Notifications
You must be signed in to change notification settings - Fork 18k
net/http: ServeFile panics when StripPrefix over-strips and results in empty path #30165
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
Comments
Change https://golang.org/cl/161738 mentions this issue: |
Change https://golang.org/cl/180498 mentions this issue: |
I investigated this and here's some more information about why this happens. The URL path for a Calling // redirect if the directory name doesn't end in a slash
if d.IsDir() {
url := r.URL.Path
if url[len(url)-1] != '/' { // panic here, since url is empty string
localRedirect(w, r, path.Base(url)+"/")
return
}
} /cc @bradfitz |
Change https://golang.org/cl/180499 mentions this issue: |
Roll back CL 161738. That fix changed StripPrefix behavior in the general case, not just in the situation where where stripping the prefix from path resulted in the empty string, causing issue #31622. That kind of change to StripPrefix behavior is not backwards compatible, and there can be a smaller, more targeted fix for the original issue. Fixes #31622 Updates #30165 Change-Id: Ie2fcfe6787a32e44f71d564d8f9c9d580fc6f704 Reviewed-on: https://go-review.googlesource.com/c/go/+/180498 Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Re-opening this because the fix was rolled back in CL 180498. For (my) reference, the code where panic happens was added in CL 20128 to fix #13996. It didn't take into account that When fixing this, need to make sure it won't introduce an infinite or otherwise unwanted redirects. The analysis in the comment #13996 (comment) is good and should be taken into account. |
I see this issue was moved to Go1.14 but since it is a regression perhaps for Go1.13, but also Brad's CL https://golang.org/cl/180499 LGTM, just that I don't have access to update his CL with a code review comment that was requested to use @dmitshur @andybons can we perhaps make that happen and submit that CL? Thank you. |
@odeke-em it’s too late for this to go into 1.13 at this point. Sorry. |
Got it! We can wait until Go.1.14 then :) |
….Path Updates #30165 Updates #31622 Change-Id: I7a4b91aa7c5c3af8c0b1273cbb42046feddf7d78 Reviewed-on: https://go-review.googlesource.com/c/go/+/180499 Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com> Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
….Path Updates golang#30165 Updates golang#31622 Change-Id: I7a4b91aa7c5c3af8c0b1273cbb42046feddf7d78 Reviewed-on: https://go-review.googlesource.com/c/go/+/180499 Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com> Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
….Path Updates golang#30165 Updates golang#31622 Change-Id: I7a4b91aa7c5c3af8c0b1273cbb42046feddf7d78 Reviewed-on: https://go-review.googlesource.com/c/go/+/180499 Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com> Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
Was this ever solved elsewhere? |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
YES
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
I did use
http.StripPrefix
accompanied withhttp.ServeFile
to strip URL prefix and serve a folder. And the HTTP server will panic and the web page will receive nothing if I opened http://localhost:8080/download/.Here's a code snippet to reproduce the problem:
What did you expect to see?
I should see contents from my
files
folder. File list or index page content.What did you see instead?
No HTTP response, but panics information from stderr.
The text was updated successfully, but these errors were encountered: