Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(gw): IPIP-402 CARs return useful blocks on not found errors #440

Merged
merged 4 commits into from
Aug 18, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/gateway-conformance.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ jobs:

# 4. Run the gateway-conformance tests
- name: Run gateway-conformance tests
uses: ipfs/gateway-conformance/.github/actions/test@v0.3
uses: ipfs/gateway-conformance/.github/actions/test@261cc49391dcd2bddd7704ad28b236dc4a63c424
hacdias marked this conversation as resolved.
Show resolved Hide resolved
hacdias marked this conversation as resolved.
Show resolved Hide resolved
with:
gateway-url: http://127.0.0.1:8040
json: output.json
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ The following emojis are used to highlight certain changes:
### Fixed

- Address a Bitswap findpeers / connect race condition that can prevent peer communication ([#435](https://github.com/ipfs/boxo/issues/435))
- HTTP Gateway API: Not having a block will result in a 5xx error rather than 404
- HTTP Gateway API: CAR requests will return 200s and a CAR file proving a requested path does not exist rather than returning an error

### Security

Expand Down
36 changes: 36 additions & 0 deletions gateway/blocks_backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,9 +232,45 @@
return md, fileNode, nil
}

// emptyRoot is a CAR root with the empty identity CID. CAR files are recommended
// to always include a CID in their root, even if it's just the empty CID.
// https://ipld.io/specs/transport/car/carv1/#number-of-roots
var emptyRoot = []cid.Cid{cid.MustParse("bafkqaaa")}

func (bb *BlocksBackend) GetCAR(ctx context.Context, p ImmutablePath, params CarParams) (ContentPathMetadata, io.ReadCloser, error) {
pathMetadata, err := bb.ResolvePath(ctx, p)
if err != nil {
rootCid, err := cid.Decode(strings.Split(p.String(), "/")[2])
if err != nil {
return ContentPathMetadata{}, nil, err
}

Check warning on line 246 in gateway/blocks_backend.go

View check run for this annotation

Codecov / codecov/patch

gateway/blocks_backend.go#L243-L246

Added lines #L243 - L246 were not covered by tests

var buf bytes.Buffer
cw, err := storage.NewWritable(&buf, emptyRoot, car.WriteAsCarV1(true))
if err != nil {
return ContentPathMetadata{}, nil, err
}

Check warning on line 252 in gateway/blocks_backend.go

View check run for this annotation

Codecov / codecov/patch

gateway/blocks_backend.go#L248-L252

Added lines #L248 - L252 were not covered by tests

blockGetter := merkledag.NewDAGService(bb.blockService).Session(ctx)

blockGetter = &nodeGetterToCarExporer{
ng: blockGetter,
cw: cw,
}

// Setup the UnixFS resolver.
f := newNodeGetterFetcherSingleUseFactory(ctx, blockGetter)
pathResolver := resolver.NewBasicResolver(f)
ip := ipfspath.FromString(p.String())
_, _, err = pathResolver.ResolveToLastNode(ctx, ip)

if isErrNotFound(err) {
return ContentPathMetadata{
PathSegmentRoots: nil,
LastSegment: ifacepath.NewResolvedPath(ip, rootCid, rootCid, ""),
ContentType: "",
}, io.NopCloser(&buf), nil
}

Check warning on line 273 in gateway/blocks_backend.go

View check run for this annotation

Codecov / codecov/patch

gateway/blocks_backend.go#L254-L273

Added lines #L254 - L273 were not covered by tests
return ContentPathMetadata{}, nil, err
}

Expand Down
7 changes: 2 additions & 5 deletions gateway/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"github.com/ipfs/boxo/gateway/assets"
"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"
)

Expand Down Expand Up @@ -177,11 +176,9 @@ func webError(w http.ResponseWriter, r *http.Request, c *Config, err error, defa
}
}

// isErrNotFound returns true for IPLD errors that should return 4xx errors (e.g. the path doesn't exist, the data is
// the wrong type, etc.), rather than issues with just finding and retrieving the data.
func isErrNotFound(err error) bool {
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 {
Expand Down
2 changes: 1 addition & 1 deletion gateway/gateway_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -763,7 +763,7 @@ func TestErrorBubblingFromBackend(t *testing.T) {
})
}

testError("404 Not Found from IPLD", &ipld.ErrNotFound{}, http.StatusNotFound)
testError("500 Not Found from IPLD", &ipld.ErrNotFound{}, http.StatusInternalServerError)
hacdias marked this conversation as resolved.
Show resolved Hide resolved
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)
Expand Down
Loading