From ed9d229fb04bd34b04889a10d3698577cccce3c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Mur=C3=A9?= Date: Tue, 30 Jul 2024 16:15:06 +0200 Subject: [PATCH] fix(gateway): ipld.ErrNotFound should result in a 404 (#630) * fix(gateway): ipld.ErrNotFound should result in a 404 In case of blockstore restriction like with --offline or similar restriction, returning a 500 is inappropriate. It's also going back to a previous gateway behavior (at least in kubo v0.22, possibly later). * refactor(gateway): match 410 before 404s just a precaution to ensure 410 takes precedence, until we address https://github.com/ipfs/boxo/issues/591 * docs: clarify 404 use case --------- Co-authored-by: Marcin Rataj --- CHANGELOG.md | 1 + gateway/errors.go | 9 +++++++-- gateway/gateway_test.go | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 29356a8a11..fcb13469cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ The following emojis are used to highlight certain changes: ### Fixed +- `boxo/gateway` now correctly returns 404 Status Not Found instead of 500 when the requested content cannot be found due to offline exchange, gateway running in no-fetch (non-recursive) mode, or a similar restriction that only serves a specific set of CIDs. - `bitswap/client` fix memory leak in BlockPresenceManager due to unlimited map growth. ### Security diff --git a/gateway/errors.go b/gateway/errors.go index c245ae4c11..5ea15ff182 100644 --- a/gateway/errors.go +++ b/gateway/errors.go @@ -13,6 +13,7 @@ import ( "github.com/ipfs/boxo/path" "github.com/ipfs/boxo/path/resolver" "github.com/ipfs/go-cid" + ipld "github.com/ipfs/go-ipld-format" "github.com/ipld/go-ipld-prime/datamodel" "github.com/ipld/go-ipld-prime/schema" ) @@ -185,10 +186,10 @@ func webError(w http.ResponseWriter, r *http.Request, c *Config, err error, defa switch { case errors.Is(err, &cid.ErrInvalidCid{}): code = http.StatusBadRequest - case isErrNotFound(err): - code = http.StatusNotFound case isErrContentBlocked(err): code = http.StatusGone + case isErrNotFound(err): + code = http.StatusNotFound case errors.Is(err, context.DeadlineExceeded): code = http.StatusGatewayTimeout } @@ -226,6 +227,10 @@ func isErrNotFound(err error) bool { return true } + if ipld.IsNotFound(err) { + return true + } + // Checks if err is of a type that does not implement the .Is interface and // cannot be directly compared to. Therefore, errors.Is cannot be used. for { diff --git a/gateway/gateway_test.go b/gateway/gateway_test.go index 5c309a5717..3c1ab3f72d 100644 --- a/gateway/gateway_test.go +++ b/gateway/gateway_test.go @@ -924,7 +924,7 @@ func TestErrorBubblingFromBackend(t *testing.T) { }) } - testError("500 Not Found from IPLD", &ipld.ErrNotFound{}, http.StatusInternalServerError) + testError("404 Not Found from IPLD", &ipld.ErrNotFound{}, http.StatusNotFound) testError("404 Not Found from path resolver", &resolver.ErrNoLink{}, http.StatusNotFound) testError("502 Bad Gateway", ErrBadGateway, http.StatusBadGateway) testError("504 Gateway Timeout", ErrGatewayTimeout, http.StatusGatewayTimeout)