From 8108c45eae7d497b3348fba450259b7c4b251a84 Mon Sep 17 00:00:00 2001 From: Henrique Dias Date: Mon, 27 Feb 2023 10:08:37 +0100 Subject: [PATCH] fix(gateway): return 500 for all /ip[nf]s/id failures --- gateway/gateway_test.go | 2 ++ gateway/handler.go | 26 ++++++-------------------- gateway/handler_test.go | 4 ++-- 3 files changed, 10 insertions(+), 22 deletions(-) diff --git a/gateway/gateway_test.go b/gateway/gateway_test.go index 6b81830aa..8848b3d32 100644 --- a/gateway/gateway_test.go +++ b/gateway/gateway_test.go @@ -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"}, diff --git a/gateway/handler.go b/gateway/handler.go index d34a76c3e..b18aa56cc 100644 --- a/gateway/handler.go +++ b/gateway/handler.go @@ -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" @@ -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): @@ -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 { @@ -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) { @@ -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 diff --git a/gateway/handler_test.go b/gateway/handler_test.go index a225ead53..ea38aa396 100644 --- a/gateway/handler_test.go +++ b/gateway/handler_test.go @@ -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) @@ -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) {