fix(gw): ensure dir URLs have trailing slash
This fixes a regression around directory listing and index.html hosting. Seems that during one of recent refactors code changed and we no longer check for trailing slash in HTTP request path, but look at content path instead. This cleans this up and also ensures dir behavior is the same for both index.html hosting and dir-index-html (generated listing). It also adds more tests so we catch any future regressions.
Marcin Rataj committed
Jul 19, 2022 at 01:17 UTC
3182986151d7ccaa110dd894d6fcba0ab77382f3
6 files changed
+53
-32
core/corehttp/gateway_handler_unixfs_dir.go
+16
-14
@@ -41,28 +41,30 @@ func (i *gatewayHandler) serveDirectory(ctx context.Context, w http.ResponseWrit
41
}
42
originalUrlPath := requestURI.Path
43
44
- // Check if directory has index.html, if so, serveFile
45
- idxPath := ipath.Join(contentPath, "index.html")
46
- idx, err := i.api.Unixfs().Get(ctx, idxPath)
47
- switch err.(type) {
48
- case nil:
49
- cpath := contentPath.String()
50
- dirwithoutslash := cpath[len(cpath)-1] != '/'
44
+ // Ensure directory paths end with '/'
45
+ if originalUrlPath[len(originalUrlPath)-1] != '/' {
46
+ // don't redirect to trailing slash if it's go get
47
+ // https://github.com/ipfs/kubo/pull/3963
48
goget := r.URL.Query().Get("go-get") == "1"
52
- if dirwithoutslash && !goget {
53
- // See comment above where originalUrlPath is declared.
49
+ if !goget {
50
suffix := "/"
51
+ // preserve query parameters
52
if r.URL.RawQuery != "" {
56
- // preserve query parameters
53
suffix = suffix + "?" + r.URL.RawQuery
54
}
59
-
55
+ // /ipfs/cid/foo?bar must be redirected to /ipfs/cid/foo/?bar
56
redirectURL := originalUrlPath + suffix
61
- logger.Debugw("serving index.html file", "to", redirectURL, "status", http.StatusFound, "path", idxPath)
62
- http.Redirect(w, r, redirectURL, http.StatusFound)
57
+ logger.Debugw("directory location moved permanently", "status", http.StatusMovedPermanently)
58
+ http.Redirect(w, r, redirectURL, http.StatusMovedPermanently)
59
return
60
}
61
+ }
62
63
+ // Check if directory has index.html, if so, serveFile
64
+ idxPath := ipath.Join(contentPath, "index.html")
65
+ idx, err := i.api.Unixfs().Get(ctx, idxPath)
66
+ switch err.(type) {
67
+ case nil:
68
f, ok := idx.(files.File)
69
if !ok {
70
internalWebError(w, files.ErrNotReader)
@@ -163,7 +165,7 @@ func (i *gatewayHandler) serveDirectory(ctx context.Context, w http.ResponseWrit
165
// add the correct link depending on whether the path ends with a slash
166
default:
167
if strings.HasSuffix(backLink, "/") {
166
- backLink += "./.."
168
+ backLink += ".."
169
} else {
170
backLink += "/.."
171
}
core/corehttp/gateway_test.go
+8
-8
@@ -380,9 +380,9 @@ func TestIPNSHostnameRedirect(t *testing.T) {
380
t.Fatal(err)
381
}
382
383
- // expect 302 redirect to same path, but with trailing slash
384
- if res.StatusCode != 302 {
385
- t.Errorf("status is %d, expected 302", res.StatusCode)
383
+ // expect 301 redirect to same path, but with trailing slash
384
+ if res.StatusCode != 301 {
385
+ t.Errorf("status is %d, expected 301", res.StatusCode)
386
}
387
hdr := res.Header["Location"]
388
if len(hdr) < 1 {
@@ -403,9 +403,9 @@ func TestIPNSHostnameRedirect(t *testing.T) {
403
t.Fatal(err)
404
}
405
406
- // expect 302 redirect to same path, but with prefix and trailing slash
407
- if res.StatusCode != 302 {
408
- t.Errorf("status is %d, expected 302", res.StatusCode)
406
+ // expect 301 redirect to same path, but with prefix and trailing slash
407
+ if res.StatusCode != 301 {
408
+ t.Errorf("status is %d, expected 301", res.StatusCode)
409
}
410
hdr = res.Header["Location"]
411
if len(hdr) < 1 {
@@ -492,7 +492,7 @@ func TestIPNSHostnameBacklinks(t *testing.T) {
492
if !matchPathOrBreadcrumbs(s, "/ipns/<a href=\"//example.net/\">example.net</a>/<a href=\"//example.net/foo%3F%20%23%3C%27\">foo? #<'</a>") {
493
t.Fatalf("expected a path in directory listing")
494
}
495
- if !strings.Contains(s, "<a href=\"/foo%3F%20%23%3C%27/./..\">") {
495
+ if !strings.Contains(s, "<a href=\"/foo%3F%20%23%3C%27/..\">") {
496
t.Fatalf("expected backlink in directory listing")
497
}
498
if !strings.Contains(s, "<a href=\"/foo%3F%20%23%3C%27/file.txt\">") {
@@ -566,7 +566,7 @@ func TestIPNSHostnameBacklinks(t *testing.T) {
566
if !matchPathOrBreadcrumbs(s, "/ipns/<a href=\"//example.net/\">example.net</a>/<a href=\"//example.net/foo%3F%20%23%3C%27\">foo? #<'</a>/<a href=\"//example.net/foo%3F%20%23%3C%27/bar\">bar</a>") {
567
t.Fatalf("expected a path in directory listing")
568
}
569
- if !strings.Contains(s, "<a href=\"/foo%3F%20%23%3C%27/bar/./..\">") {
569
+ if !strings.Contains(s, "<a href=\"/foo%3F%20%23%3C%27/bar/..\">") {
570
t.Fatalf("expected backlink in directory listing")
571
}
572
if !strings.Contains(s, "<a href=\"/foo%3F%20%23%3C%27/bar/file.txt\">") {
test/sharness/t0110-gateway.sh
+1
@@ -72,6 +72,7 @@ test_expect_success "GET IPFS directory file output looks good" '
72
73
test_expect_success "GET IPFS directory with index.html returns redirect to add trailing slash" "
74
curl -sI -o response_without_slash \"http://127.0.0.1:$port/ipfs/$HASH2/dirwithindex?query=to-remember\" &&
75
+ test_should_contain \"HTTP/1.1 301 Moved Permanently\" response_without_slash &&
76
test_should_contain \"Location: /ipfs/$HASH2/dirwithindex/?query=to-remember\" response_without_slash
77
"
78
test/sharness/t0113-gateway-symlink.sh
+1
-1
@@ -22,7 +22,7 @@ test_expect_success "Add the test directory" '
22
'
23
24
test_expect_success "Test the directory listing" '
25
- curl "$GWAY_ADDR/ipfs/$HASH" > list_response &&
25
+ curl "$GWAY_ADDR/ipfs/$HASH/" > list_response &&
26
test_should_contain ">foo<" list_response &&
27
test_should_contain ">bar<" list_response
28
'
test/sharness/t0114-gateway-subdomains.sh
+2
-2
@@ -268,7 +268,7 @@ test_expect_success "valid file and subdirectory paths in directory listing at {
268
269
test_expect_success "valid parent directory path in directory listing at {cid}.ipfs.localhost/sub/dir" '
270
curl -s --resolve $DIR_HOSTNAME:127.0.0.1 "http://$DIR_HOSTNAME/ipfs/ipns/" > list_response &&
271
- test_should_contain "<a href=\"/ipfs/ipns/./..\">..</a>" list_response &&
271
+ test_should_contain "<a href=\"/ipfs/ipns/..\">..</a>" list_response &&
272
test_should_contain "<a href=\"/ipfs/ipns/bar\">bar</a>" list_response
273
'
274
@@ -441,7 +441,7 @@ test_expect_success "valid file and directory paths in directory listing at {cid
441
442
test_expect_success "valid parent directory path in directory listing at {cid}.ipfs.example.com/sub/dir" '
443
curl -s -H "Host: $DIR_FQDN" http://127.0.0.1:$GWAY_PORT/ipfs/ipns/ > list_response &&
444
- test_should_contain "<a href=\"/ipfs/ipns/./..\">..</a>" list_response &&
444
+ test_should_contain "<a href=\"/ipfs/ipns/..\">..</a>" list_response &&
445
test_should_contain "<a href=\"/ipfs/ipns/bar\">bar</a>" list_response
446
'
447
test/sharness/t0115-gateway-dir-listing.sh
+25
-7
@@ -43,8 +43,14 @@ test_expect_success "path gw: backlink on root CID should be hidden" '
43
test_should_not_contain "<a href=\"/ipfs/$DIR_CID/\">..</a>" list_response
44
'
45
46
-test_expect_success "path gw: Etag should be present" '
46
+test_expect_success "path gw: redirect dir listing to URL with trailing slash" '
47
curl -sD - http://127.0.0.1:$GWAY_PORT/ipfs/${DIR_CID}/ą/ę > list_response &&
48
+ test_should_contain "HTTP/1.1 301 Moved Permanently" list_response &&
49
+ test_should_contain "Location: /ipfs/${DIR_CID}/%c4%85/%c4%99/" list_response
50
+'
51
+
52
+test_expect_success "path gw: Etag should be present" '
53
+ curl -sD - http://127.0.0.1:$GWAY_PORT/ipfs/${DIR_CID}/ą/ę/ > list_response &&
54
test_should_contain "Index of" list_response &&
55
test_should_contain "Etag: \"DirIndex-" list_response
56
'
@@ -72,19 +78,25 @@ test_expect_success "path gw: hash column should be a CID link with filename par
78
DIR_HOSTNAME="${DIR_CID}.ipfs.localhost"
79
# note: we skip DNS lookup by running curl with --resolve $DIR_HOSTNAME:127.0.0.1
80
75
-test_expect_success "path gw: backlink on root CID should be hidden" '
81
+test_expect_success "subdomain gw: backlink on root CID should be hidden" '
82
curl -sD - --resolve $DIR_HOSTNAME:$GWAY_PORT:127.0.0.1 http://$DIR_HOSTNAME:$GWAY_PORT/ > list_response &&
83
test_should_contain "Index of" list_response &&
84
test_should_not_contain "<a href=\"/\">..</a>" list_response
85
'
86
81
-test_expect_success "path gw: Etag should be present" '
87
+test_expect_success "subdomain gw: redirect dir listing to URL with trailing slash" '
88
curl -sD - --resolve $DIR_HOSTNAME:$GWAY_PORT:127.0.0.1 http://$DIR_HOSTNAME:$GWAY_PORT/ą/ę > list_response &&
89
+ test_should_contain "HTTP/1.1 301 Moved Permanently" list_response &&
90
+ test_should_contain "Location: /%c4%85/%c4%99/" list_response
91
+'
92
+
93
+test_expect_success "subdomain gw: Etag should be present" '
94
+ curl -sD - --resolve $DIR_HOSTNAME:$GWAY_PORT:127.0.0.1 http://$DIR_HOSTNAME:$GWAY_PORT/ą/ę/ > list_response &&
95
test_should_contain "Index of" list_response &&
96
test_should_contain "Etag: \"DirIndex-" list_response
97
'
98
87
-test_expect_success "path gw: backlink on subdirectory should point at parent directory" '
99
+test_expect_success "subdomain gw: backlink on subdirectory should point at parent directory" '
100
test_should_contain "<a href=\"/%C4%85/%C4%99/..\">..</a>" list_response
101
'
102
@@ -92,11 +104,11 @@ test_expect_success "subdomain gw: breadcrumbs should leverage path-based router
104
test_should_contain "/ipfs/<a href=\"//localhost:$GWAY_PORT/ipfs/$DIR_CID\">$DIR_CID</a>/<a href=\"//localhost:$GWAY_PORT/ipfs/$DIR_CID/%C4%85\">ą</a>/<a href=\"//localhost:$GWAY_PORT/ipfs/$DIR_CID/%C4%85/%C4%99\">ę</a>" list_response
105
'
106
95
-test_expect_success "path gw: name column should be a link to content root mounted at subdomain origin" '
107
+test_expect_success "subdomain gw: name column should be a link to content root mounted at subdomain origin" '
108
test_should_contain "<a href=\"/%C4%85/%C4%99/file-%C5%BA%C5%82.txt\">file-źł.txt</a>" list_response
109
'
110
99
-test_expect_success "path gw: hash column should be a CID link to path router with filename param" '
111
+test_expect_success "subdomain gw: hash column should be a CID link to path router with filename param" '
112
test_should_contain "<a class=\"ipfs-hash\" translate=\"no\" href=\"//localhost:$GWAY_PORT/ipfs/$FILE_CID?filename=file-%25C5%25BA%25C5%2582.txt\">" list_response
113
'
114
@@ -121,8 +133,14 @@ test_expect_success "dnslink gw: backlink on root CID should be hidden" '
133
test_should_not_contain "<a href=\"/\">..</a>" list_response
134
'
135
124
-test_expect_success "dnslink gw: Etag should be present" '
136
+test_expect_success "dnslink gw: redirect dir listing to URL with trailing slash" '
137
curl -sD - --resolve $DNSLINK_HOSTNAME:$GWAY_PORT:127.0.0.1 http://$DNSLINK_HOSTNAME:$GWAY_PORT/ą/ę > list_response &&
138
+ test_should_contain "HTTP/1.1 301 Moved Permanently" list_response &&
139
+ test_should_contain "Location: /%c4%85/%c4%99/" list_response
140
+'
141
+
142
+test_expect_success "dnslink gw: Etag should be present" '
143
+ curl -sD - --resolve $DNSLINK_HOSTNAME:$GWAY_PORT:127.0.0.1 http://$DNSLINK_HOSTNAME:$GWAY_PORT/ą/ę/ > list_response &&
144
test_should_contain "Index of" list_response &&
145
test_should_contain "Etag: \"DirIndex-" list_response
146
'