Skip to content

Commit

Permalink
fix(gateway): return 500 for all /ip[nf]s/id failures
Browse files Browse the repository at this point in the history
  • Loading branch information
hacdias authored and lidel committed Feb 28, 2023
1 parent 0d5d1ca commit 36918f4
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 22 deletions.
2 changes: 2 additions & 0 deletions gateway/gateway_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,9 +352,11 @@ func TestGatewayGet(t *testing.T) {
}{
{"127.0.0.1:8080", "/", http.StatusNotFound, "404 page not found\n"},
{"127.0.0.1:8080", "/" + k.Cid().String(), http.StatusNotFound, "404 page not found\n"},
{"127.0.0.1:8080", "/ipfs/this-is-not-a-cid", http.StatusInternalServerError, "failed to resolve /ipfs/this-is-not-a-cid: invalid path \"/ipfs/this-is-not-a-cid\": invalid CID: illegal base32 data at input byte 3\n"},
{"127.0.0.1:8080", k.String(), http.StatusOK, "fnord"},
{"127.0.0.1:8080", "/ipns/nxdomain.example.com", http.StatusInternalServerError, "failed to resolve /ipns/nxdomain.example.com: " + namesys.ErrResolveFailed.Error() + "\n"},
{"127.0.0.1:8080", "/ipns/%0D%0A%0D%0Ahello", http.StatusInternalServerError, "failed to resolve /ipns/\\r\\n\\r\\nhello: " + namesys.ErrResolveFailed.Error() + "\n"},
{"127.0.0.1:8080", "/ipns/k51qzi5uqu5djucgtwlxrbfiyfez1nb0ct58q5s4owg6se02evza05dfgi6tw5", http.StatusInternalServerError, "failed to resolve /ipns/k51qzi5uqu5djucgtwlxrbfiyfez1nb0ct58q5s4owg6se02evza05dfgi6tw5: " + namesys.ErrResolveFailed.Error() + "\n"},
{"127.0.0.1:8080", "/ipns/example.com", http.StatusOK, "fnord"},
{"example.com", "/", http.StatusOK, "fnord"},

Expand Down
26 changes: 6 additions & 20 deletions gateway/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,9 @@ import (
cid "github.com/ipfs/go-cid"
ipld "github.com/ipfs/go-ipld-format"
logging "github.com/ipfs/go-log"
"github.com/ipfs/go-namesys"
"github.com/ipfs/go-path"
"github.com/ipfs/go-path/resolver"
coreiface "github.com/ipfs/interface-go-ipfs-core"
ipath "github.com/ipfs/interface-go-ipfs-core/path"
routing "github.com/libp2p/go-libp2p/core/routing"
mc "github.com/multiformats/go-multicodec"
prometheus "github.com/prometheus/client_golang/prometheus"
"go.opentelemetry.io/otel"
Expand Down Expand Up @@ -564,8 +561,6 @@ func webRequestError(w http.ResponseWriter, err *requestError) {

func webError(w http.ResponseWriter, err error, defaultCode int) {
switch {
case errors.Is(err, path.ErrInvalidPath{}):
webErrorWithCode(w, err, http.StatusBadRequest)
case isErrNotFound(err):
webErrorWithCode(w, err, http.StatusNotFound)
case errors.Is(err, ErrGatewayTimeout):
Expand All @@ -580,16 +575,13 @@ func webError(w http.ResponseWriter, err error, defaultCode int) {
}

func isErrNotFound(err error) bool {
return isErrNoLink(err) ||
errors.Is(err, routing.ErrNotFound) ||
err == routing.ErrNotFound ||
ipld.IsNotFound(err)
}
if ipld.IsNotFound(err) {
return true
}

// isErrNoLink checks if err is a resolver.ErrNoLink. resolver.ErrNoLink
// does not implement a .Is interface and cannot be directly compared to.
// Therefore, errors.Is always returns false with it.
func isErrNoLink(err error) bool {
// Checks if err is a resolver.ErrNoLink. resolver.ErrNoLink does not implement
// the .Is interface and cannot be directly compared to. Therefore, errors.Is
// always returns false with it.
for {
_, ok := err.(resolver.ErrNoLink)
if ok {
Expand Down Expand Up @@ -772,11 +764,6 @@ func (i *handler) handlePathResolution(w http.ResponseWriter, r *http.Request, r
err = fmt.Errorf("failed to resolve %s: %w", debugStr(contentPath.String()), err)
webError(w, err, http.StatusServiceUnavailable)
return nil, nil, false
case namesys.ErrResolveFailed:
// Note: webError will replace http.StatusBadRequest with StatusNotFound or StatusRequestTimeout if necessary
err = fmt.Errorf("failed to resolve %s: %w", debugStr(contentPath.String()), err)
webError(w, err, http.StatusInternalServerError)
return nil, nil, false
default:
// The path can't be resolved.
if isUnixfsResponseFormat(responseFormat) {
Expand All @@ -801,7 +788,6 @@ func (i *handler) handlePathResolution(w http.ResponseWriter, r *http.Request, r
}
}

// Note: webError will replace http.StatusInternalServerError with StatusNotFound or StatusRequestTimeout if necessary
err = fmt.Errorf("failed to resolve %s: %w", debugStr(contentPath.String()), err)
webError(w, err, http.StatusInternalServerError)
return nil, nil, false
Expand Down
4 changes: 2 additions & 2 deletions gateway/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func (api *errorMockAPI) ResolvePath(ctx context.Context, ip ipath.Path) (ipath.
return nil, api.err
}

func TestGatewayBadRequestInvalidPath(t *testing.T) {
func TestGatewayInternalServerErrorInvalidPath(t *testing.T) {
api, _ := newMockAPI(t)
ts := newTestServer(t, api)
t.Logf("test server url: %s", ts.URL)
Expand All @@ -82,7 +82,7 @@ func TestGatewayBadRequestInvalidPath(t *testing.T) {
res, err := ts.Client().Do(req)
assert.Nil(t, err)

assert.Equal(t, http.StatusBadRequest, res.StatusCode)
assert.Equal(t, http.StatusInternalServerError, res.StatusCode)
}

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

0 comments on commit 36918f4

Please sign in to comment.