From 589f56fcede775de8ebf29fdf53736de72223fc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Mur=C3=A9?= Date: Fri, 22 Mar 2024 18:59:51 +0100 Subject: [PATCH 1/3] fix(gateway): ipld.ErrNotFound should result in a 404 In case of blockstore restriction like with --offline or similar restriction, returning a 500 is inapropriate. It's also going back to a previous gateway behavior (at least in kubo v0.22, possibly later). --- CHANGELOG.md | 2 ++ gateway/errors.go | 5 +++++ gateway/gateway_test.go | 2 +- 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a62354e2..a213db637 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,8 @@ The following emojis are used to highlight certain changes: ### Fixed +- `boxo/gateway` now returns 404 Status Not Found instead of 500 when the requested data cannot be found, without a fallback on bitswap or similar restriction. + ### Security ## [v0.21.0] diff --git a/gateway/errors.go b/gateway/errors.go index c245ae4c1..2bbde1750 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" ) @@ -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 d48334b92..c6fbd67ed 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) From e372bc62de4c804dea26e93360e061f507081c57 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Tue, 30 Jul 2024 15:37:01 +0200 Subject: [PATCH 2/3] refactor(gateway): match 410 before 404s just a precaution to ensure 410 takes precedence, until we address https://github.com/ipfs/boxo/issues/591 --- gateway/errors.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gateway/errors.go b/gateway/errors.go index 2bbde1750..5ea15ff18 100644 --- a/gateway/errors.go +++ b/gateway/errors.go @@ -186,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 } From 2b6f29f9b7f95290f1cf57c2be2833e1ddea9034 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Tue, 30 Jul 2024 15:39:10 +0200 Subject: [PATCH 3/3] docs: clarify 404 use case --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b70004620..fcb13469c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,7 +26,7 @@ The following emojis are used to highlight certain changes: ### Fixed -- `boxo/gateway` now returns 404 Status Not Found instead of 500 when the requested data cannot be found, without a fallback on bitswap or similar restriction. +- `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