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.

Sign up for GitHub

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

don't use the domain name as a filename in /ipns/a.com #5564

Merged
merged 3 commits into from
Oct 5, 2018
+23 −1
Merged

don't use the domain name as a filename in /ipns/a.com #5564

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
47e7f69
don't use the domain name as a filename in /ipns/a.com
Stebalien Oct 4, 2018
afd81fd
gateway test: document why .man
Stebalien Oct 5, 2018
f498459
gateway_test: comment on platform dependence
Stebalien Oct 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion core/corehttp/gateway_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ func (i *gatewayHandler) getOrHeadHandler(ctx context.Context, w http.ResponseWr
w.Header().Set("Content-Disposition", fmt.Sprintf("inline; filename*=UTF-8''%s", url.PathEscape(urlFilename)))
name = urlFilename
} else {
name = gopath.Base(urlPath)
name = getFilename(urlPath)
}
i.serveFile(w, r, name, modtime, dr)
return
Expand Down Expand Up @@ -624,3 +624,11 @@ func webErrorWithCode(w http.ResponseWriter, message string, err error, code int
func internalWebError(w http.ResponseWriter, err error) {
webErrorWithCode(w, "internalWebError", err, http.StatusInternalServerError)
}

func getFilename(s string) string {
if (strings.HasPrefix(s, ipfsPathPrefix) || strings.HasPrefix(s, ipnsPathPrefix)) && strings.Count(gopath.Clean(s), "/") <= 2 {
// Don't want to treat ipfs.io in /ipns/ipfs.io as a filename.
return ""
}
return gopath.Base(s)
}
12 changes: 12 additions & 0 deletions core/corehttp/gateway_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,12 @@ func TestGatewayGet(t *testing.T) {
ns["/ipns/double.example.com"] = path.FromString("/ipns/working.example.com")
ns["/ipns/triple.example.com"] = path.FromString("/ipns/double.example.com")
ns["/ipns/broken.example.com"] = path.FromString("/ipns/" + k)
// We picked .man because:
// 1. It's a valid TLD.
// 2. Go treats it as the file extension for "man" files (even though
// nobody actually *uses* this extension, AFAIK).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is system dependent. http.ServeContext calls mime.TypeByExtension which in the sandbox go environment does not detect it as a application/x-troff-man. See: https://play.golang.org/p/ohNcHHsVArq

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. Damn, that's why .com wasn't working.

// 3. Go accepts "fnord" (the test value) as a valid man file.
Copy link
Contributor

@kevina kevina Oct 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

???

Did you mean go does not accept "fnord" as a valid man file?

http.DetectContentType([]byte("fnord")) returns text/plain; charset=utf-8

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It accepts it as a valid man file if the extension is ".man". However, it looks like that part isn't important. I thought go looked at both the content and the extension but that doesn't appear to be the case (#5564 (comment)).

I don't know how I can make these tests platform-independent. At least they appear to pass on Linux now...

ns["/ipns/example.man"] = path.FromString("/ipfs/" + k)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I take it that .man has special meaning to the go http library? I would add a test for this. To me it is very non-obvious.

Alternatively I would have the content be a small html file and use example.txtinstead and make sure the content type is an html file not a text file.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I take it that .man has special meaning to the go http library? I would add a test for this. To me it is very non-obvious.
Alternatively I would have the content be a small html file and use example.txtinstead and make sure the content type is an html file not a text file.

I picked .man because it's a valid TLD. I couldn't get this to work with .html or .txt because IPFS will refuse to resolve these as domains. I considered using .com but I needed something that could be a COM file (the original issue used a GIF). I didn't want to deal with that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add a comment.


t.Log(ts.URL)
for _, test := range []struct {
Expand All @@ -175,6 +181,8 @@ func TestGatewayGet(t *testing.T) {
{"working.example.com", "/ipfs/" + k, http.StatusNotFound, "ipfs resolve -r /ipns/working.example.com/ipfs/" + k + ": no link by that name\n"},
{"broken.example.com", "/", http.StatusNotFound, "ipfs resolve -r /ipns/broken.example.com/: " + namesys.ErrResolveFailed.Error() + "\n"},
{"broken.example.com", "/ipfs/" + k, http.StatusNotFound, "ipfs resolve -r /ipns/broken.example.com/ipfs/" + k + ": " + namesys.ErrResolveFailed.Error() + "\n"},
// This test case ensures we don't treat the TLD as a file extension.
{"example.man", "/", http.StatusOK, "fnord"},
} {
var c http.Client
r, err := http.NewRequest("GET", ts.URL+test.path, nil)
Expand All @@ -190,6 +198,10 @@ func TestGatewayGet(t *testing.T) {
continue
}
defer resp.Body.Close()
contentType := resp.Header.Get("Content-Type")
if contentType != "text/plain; charset=utf-8" {
t.Errorf("expected content type to be text/plain, got %s", contentType)
}
if resp.StatusCode != test.status {
t.Errorf("got %d, expected %d from %s", resp.StatusCode, test.status, urlstr)
continue
Expand Down