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: ServeFile generates wrong links in a directory when a path doesn't end in a slash #16424

Closed
solarsea opened this issue Jul 19, 2016 · 10 comments

Comments

Projects
None yet
6 participants
@solarsea
Copy link

commented Jul 19, 2016

Please answer these questions before submitting your issue. Thanks!

  1. What version of Go are you using (go version)?
    go version go1.6.2 windows/amd64
  2. What operating system and processor architecture are you using (go env)?
    Windows, amd64
  3. What did you do?

I ran the program with the directory structure "c:/temp/dir1/file.txt" and opened
both "http://localhost/dir1/" and "http://localhost/dir1" (without trailing slash)

/-------------------------------------------------------------
package main

import (
"net/http"
)

func main() {
http.ListenAndServe(":80", http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
http.ServeFile(rw, req, "C:/temp")
}))
}
/-------------------------------------------------------------

  1. What did you expect to see?

Same html output for both.

  1. What did you see instead?

The URL without the trailing slash shows correct directory content but generates a wrong link in the form of "http://localhost/file.txt" instead of "http://localhost/dir1/file.txt"

@davecheney

This comment has been minimized.

Copy link
Contributor

commented Jul 19, 2016

rfc 2616 and 4918 are mute on what happens when you request a representation of a collection (rfc 4918 terms for a directory) without the trailing slash. To be explicit, rfc 4918 defines

http://localhost/dir/

is a collection

http://localhost/dir 

is not defined (by my reading). There are two possibly solutions I can see:

a. /dir should issue a 30x redirect to /dir/
b. /dir should return a 404, the name of the resource is /dir/

@solarsea

This comment has been minimized.

Copy link
Author

commented Jul 19, 2016

It seems that option a is implemented already - https://golang.org/src/net/http/fs.go?s=14478:14535#L379 but it is explicitly turned off in the case of ServeFile

@davecheney

This comment has been minimized.

Copy link
Contributor

commented Jul 19, 2016

@solarsea i'm sorry i don't understand, can be more explicit please.

@solarsea

This comment has been minimized.

Copy link
Author

commented Jul 19, 2016

ServeFile uses serveFile internally - https://golang.org/src/net/http/fs.go#L353. The later has a parameter called 'redirect' that when true already implements the behavior that you suggest - that /dir should redirect to /dir/

@solarsea

This comment has been minimized.

Copy link
Author

commented Jul 19, 2016

I traced the implementation back to fa60226 by @rsc where 'serveFile' is named 'serveFileInternal'

@solarsea

This comment has been minimized.

Copy link
Author

commented Jul 19, 2016

I think that a straightforward fix is to drop the 'redirect' parameter of serveFile(...) altogether and therefore unify ServeFile(...) and FileServer(...) behaviors

@ianlancetaylor ianlancetaylor changed the title http.ServeFile generates wrong links in a directory when a path doesn't end in a slash net/http: ServeFile generates wrong links in a directory when a path doesn't end in a slash Jul 19, 2016

@ianlancetaylor ianlancetaylor added this to the Go1.8 milestone Jul 19, 2016

@0xmohit

This comment has been minimized.

Copy link
Contributor

commented Jul 19, 2016

This appears to be the same as #13996

@solarsea Could you verify this with go1.7rc?

@bradfitz

This comment has been minimized.

Copy link
Member

commented Jul 19, 2016

Yeah I think this is a dup.

@solarsea

This comment has been minimized.

Copy link
Author

commented Jul 19, 2016

It is a dup, but the fix looks weird. It is adding corrective code, instead of fixing it by removal of the defective flag.

@bradfitz

This comment has been minimized.

Copy link
Member

commented Jul 19, 2016

Glad to hear it's fixed. Feel free to send a change if you have something which makes the code more clear. I'm just happy we have a test for it now.

@bradfitz bradfitz closed this Jul 19, 2016

@golang golang locked and limited conversation to collaborators Jul 19, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.