Skip to content

Commit

Permalink
refactor: prefix cleanup and readable paths
Browse files Browse the repository at this point in the history 
- removed dead code after X-Ipfs-Gateway-Prefix is gone
  (#7702)
- escaped special characters in content paths returned with http.Error
  making them both safer and easier to reason about (e.g. when invisible
  whitespace unicode is copied)
  • Loading branch information
@lidel
lidel committed Mar 15, 2022
1 parent 26c122e commit eb95d2b
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 91 deletions.
17 changes: 13 additions & 4 deletions core/corehttp/gateway_handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package corehttp
import (
"context"
"fmt"
"html"
"html/template"
"io"
"net/http"
Expand Down Expand Up @@ -263,15 +262,16 @@ func (i *gatewayHandler) getOrHeadHandler(w http.ResponseWriter, r *http.Request
switch err {
case nil:
case coreiface.ErrOffline:
webError(w, "ipfs resolve -r "+html.EscapeString(contentPath.String()), err, http.StatusServiceUnavailable)
webError(w, "ipfs resolve -r "+debugStr(contentPath.String()), err, http.StatusServiceUnavailable)
return
default:
// if Accept is text/html, see if ipfs-404.html is present
if i.servePretty404IfPresent(w, r, contentPath) {
logger.Debugw("serve pretty 404 if present")
return
}

webError(w, "ipfs resolve -r "+html.EscapeString(contentPath.String()), err, http.StatusNotFound)
webError(w, "ipfs resolve -r "+debugStr(contentPath.String()), err, http.StatusNotFound)
return
}

Expand Down Expand Up @@ -664,7 +664,7 @@ func webError(w http.ResponseWriter, message string, err error, defaultCode int)
func webErrorWithCode(w http.ResponseWriter, message string, err error, code int) {
http.Error(w, fmt.Sprintf("%s: %s", message, err), code)
if code >= 500 {
log.Warnf("server error: %s: %s", err)
log.Warnf("server error: %s: %s", message, err)
}
}

Expand Down Expand Up @@ -762,6 +762,15 @@ func preferred404Filename(acceptHeaders []string) (string, string, error) {
return "", "", fmt.Errorf("there is no 404 file for the requested content types")
}

// returns unquoted path with all special characters revealed as \u codes
func debugStr(path string) string {
q := fmt.Sprintf("%+q", path)
if len(q) >= 3 {
q = q[1 : len(q)-1]
}
return q
}

// Attempt to fix redundant /ipfs/ namespace as long as resulting
// 'intended' path is valid. This is in case gremlins were tickled
// wrong way and user ended up at /ipfs/ipfs/{cid} or /ipfs/ipns/{id}
Expand Down
90 changes: 3 additions & 87 deletions core/corehttp/gateway_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,6 @@ func newTestServerAndNode(t *testing.T, ns mockNamesys) (*httptest.Server, iface
t.Fatal(err)
}

cfg, err := n.Repo.Config()
if err != nil {
t.Fatal(err)
}
cfg.Gateway.PathPrefixes = []string{"/good-prefix"}

// need this variable here since we need to construct handler with
// listener, and server with handler. yay cycles.
dh := &delegatedHandler{}
Expand Down Expand Up @@ -242,7 +236,7 @@ func TestGatewayGet(t *testing.T) {
{"127.0.0.1:8080", "/" + k.Cid().String(), http.StatusNotFound, "404 page not found\n"},
{"127.0.0.1:8080", k.String(), http.StatusOK, "fnord"},
{"127.0.0.1:8080", "/ipns/nxdomain.example.com", http.StatusNotFound, "ipfs resolve -r /ipns/nxdomain.example.com: " + namesys.ErrResolveFailed.Error() + "\n"},
{"127.0.0.1:8080", "/ipns/%0D%0A%0D%0Ahello", http.StatusNotFound, "ipfs resolve -r /ipns/%0D%0A%0D%0Ahello: " + namesys.ErrResolveFailed.Error() + "\n"},
{"127.0.0.1:8080", "/ipns/%0D%0A%0D%0Ahello", http.StatusNotFound, "ipfs resolve -r /ipns/\\r\\n\\r\\nhello: " + namesys.ErrResolveFailed.Error() + "\n"},
{"127.0.0.1:8080", "/ipns/example.com", http.StatusOK, "fnord"},
{"example.com", "/", http.StatusOK, "fnord"},

Expand Down Expand Up @@ -403,7 +397,6 @@ func TestIPNSHostnameRedirect(t *testing.T) {
t.Fatal(err)
}
req.Host = "example.net"
req.Header.Set("X-Ipfs-Gateway-Prefix", "/good-prefix")

res, err = doWithoutRedirect(req)
if err != nil {
Expand All @@ -417,8 +410,8 @@ func TestIPNSHostnameRedirect(t *testing.T) {
hdr = res.Header["Location"]
if len(hdr) < 1 {
t.Errorf("location header not present")
} else if hdr[0] != "/good-prefix/foo/" {
t.Errorf("location header is %v, expected /good-prefix/foo/", hdr[0])
} else if hdr[0] != "/foo/" {
t.Errorf("location header is %v, expected /foo/", hdr[0])
}

// make sure /version isn't exposed
Expand All @@ -427,7 +420,6 @@ func TestIPNSHostnameRedirect(t *testing.T) {
t.Fatal(err)
}
req.Host = "example.net"
req.Header.Set("X-Ipfs-Gateway-Prefix", "/good-prefix")

res, err = doWithoutRedirect(req)
if err != nil {
Expand Down Expand Up @@ -583,82 +575,6 @@ func TestIPNSHostnameBacklinks(t *testing.T) {
if !strings.Contains(s, k3.Cid().String()) {
t.Fatalf("expected hash in directory listing")
}

// make request to directory listing with prefix
req, err = http.NewRequest(http.MethodGet, ts.URL, nil)
if err != nil {
t.Fatal(err)
}
req.Host = "example.net"
req.Header.Set("X-Ipfs-Gateway-Prefix", "/good-prefix")

res, err = doWithoutRedirect(req)
if err != nil {
t.Fatal(err)
}

// expect correct backlinks with prefix
body, err = ioutil.ReadAll(res.Body)
if err != nil {
t.Fatalf("error reading response: %s", err)
}
s = string(body)
t.Logf("body: %s\n", string(body))

if !matchPathOrBreadcrumbs(s, "/ipns/<a href=\"//example.net/\">example.net</a>") {
t.Fatalf("expected a path in directory listing")
}
if !strings.Contains(s, "<a href=\"/good-prefix/\">") {
t.Fatalf("expected backlink in directory listing")
}
if !strings.Contains(s, "<a href=\"/good-prefix/file.txt\">") {
t.Fatalf("expected file in directory listing")
}
if !strings.Contains(s, k.Cid().String()) {
t.Fatalf("expected hash in directory listing")
}

// make request to directory listing with illegal prefix
req, err = http.NewRequest(http.MethodGet, ts.URL, nil)
if err != nil {
t.Fatal(err)
}
req.Host = "example.net"
req.Header.Set("X-Ipfs-Gateway-Prefix", "/bad-prefix")

// make request to directory listing with evil prefix
req, err = http.NewRequest(http.MethodGet, ts.URL, nil)
if err != nil {
t.Fatal(err)
}
req.Host = "example.net"
req.Header.Set("X-Ipfs-Gateway-Prefix", "//good-prefix/foo")

res, err = doWithoutRedirect(req)
if err != nil {
t.Fatal(err)
}

// expect correct backlinks without illegal prefix
body, err = ioutil.ReadAll(res.Body)
if err != nil {
t.Fatalf("error reading response: %s", err)
}
s = string(body)
t.Logf("body: %s\n", string(body))

if !matchPathOrBreadcrumbs(s, "/") {
t.Fatalf("expected a path in directory listing")
}
if !strings.Contains(s, "<a href=\"/\">") {
t.Fatalf("expected backlink in directory listing")
}
if !strings.Contains(s, "<a href=\"/file.txt\">") {
t.Fatalf("expected file in directory listing")
}
if !strings.Contains(s, k.Cid().String()) {
t.Fatalf("expected hash in directory listing")
}
}

func TestCacheControlImmutable(t *testing.T) {
Expand Down

0 comments on commit eb95d2b

Please sign in to comment.