Skip to content

Commit

Permalink
feat: switch gateway GetRange API to match Get
Browse files Browse the repository at this point in the history 
Also fixes serving range requests for /ipfs/bafydir when we know we'll
get the index.html or _redirect.
  • Loading branch information
@aschmahmann
aschmahmann committed Mar 30, 2023
1 parent 6f324be commit ff460b5
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 65 deletions.
19 changes: 2 additions & 17 deletions gateway/blocks_gateway.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,23 +180,8 @@ func (api *BlocksGateway) Get(ctx context.Context, path ImmutablePath) (ContentP
return ContentPathMetadata{}, nil, fmt.Errorf("data was not a valid file or directory: %w", ErrInternalServerError) // TODO: should there be a gateway invalid content type to abstract over the various IPLD error types?
}

func (api *BlocksGateway) GetRange(ctx context.Context, path ImmutablePath, ranges ...GetRange) (ContentPathMetadata, files.File, error) {
md, nd, err := api.getNode(ctx, path)
if err != nil {
return md, nil, err
}

// This code path covers full graph, single file/directory, and range requests
n, err := ufile.NewUnixfsFile(ctx, api.dagService, nd)
if err != nil {
return md, nil, err
}
f, ok := n.(files.File)
if !ok {
return ContentPathMetadata{}, nil, NewErrorResponse(fmt.Errorf("can only do range requests on files, but did not get a file"), http.StatusBadRequest)
}

return md, f, nil
func (api *BlocksGateway) GetRange(ctx context.Context, path ImmutablePath, ranges ...GetRange) (ContentPathMetadata, *GetResponse, error) {
return api.Get(ctx, path)
}

func (api *BlocksGateway) GetAll(ctx context.Context, path ImmutablePath) (ContentPathMetadata, files.Node, error) {
Expand Down
9 changes: 5 additions & 4 deletions gateway/gateway.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,11 @@ type IPFSBackend interface {
// Size, and Cid.
Get(context.Context, ImmutablePath) (ContentPathMetadata, *GetResponse, error)

// GetRange returns a full UnixFS file object. Ranges passed in are advisory for pre-fetching data, however
// consumers of this function may require extra data beyond the passed ranges (e.g. the initial bit of the file
// might be used for content type sniffing even if only the end of the file is requested).
GetRange(context.Context, ImmutablePath, ...GetRange) (ContentPathMetadata, files.File, error)
// GetRange returns a full UnixFS file object, or a UnixFS directory. Ranges passed in are advisory for pre-fetching
// data, however consumers of this function may require extra data beyond the passed ranges (e.g. the initial bit of
// the file might be used for content type sniffing even if only the end of the file is requested).
// Note: there is currently no semantic meaning attached to a range request for a directory
GetRange(context.Context, ImmutablePath, ...GetRange) (ContentPathMetadata, *GetResponse, error)

// GetAll returns a UnixFS file or directory depending on what the path is that has been requested. Directories should
// include all content recursively.
Expand Down
2 changes: 1 addition & 1 deletion gateway/gateway_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func (api *mockAPI) Get(ctx context.Context, immutablePath ImmutablePath) (Conte
return api.gw.Get(ctx, immutablePath)
}

func (api *mockAPI) GetRange(ctx context.Context, immutablePath ImmutablePath, ranges ...GetRange) (ContentPathMetadata, files.File, error) {
func (api *mockAPI) GetRange(ctx context.Context, immutablePath ImmutablePath, ranges ...GetRange) (ContentPathMetadata, *GetResponse, error) {
return api.gw.GetRange(ctx, immutablePath, ranges...)
}

Expand Down
73 changes: 30 additions & 43 deletions gateway/handler_defaults.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,35 +48,10 @@ func (i *handler) serveDefaults(ctx context.Context, w http.ResponseWriter, r *h
return false
}
case http.MethodGet:
// TODO: refactor below: we should not have 2x20 duplicated flow control when the only difference is ranges.
var getterFn func(ctx context.Context, imPath ImmutablePath) (ContentPathMetadata, *GetResponse, error)
rangeHeader := r.Header.Get("Range")
if rangeHeader == "" {
var getResp *GetResponse
// TODO: passing resolved path here, instead of contentPath is harming content routing. Knowing original immutableContentPath will allow backend to find providers for parents, even when internal CIDs are not announced, and will provide better key for caching related DAGs.
pathMetadata, getResp, err = i.api.Get(ctx, maybeResolvedImPath)
if err != nil {
if isWebRequest(requestedContentType) {
forwardedPath, continueProcessing := i.handleWebRequestErrors(w, r, maybeResolvedImPath, immutableContentPath, contentPath, err, logger)
if !continueProcessing {
return false
}
pathMetadata, getResp, err = i.api.Get(ctx, forwardedPath)
if err != nil {
err = fmt.Errorf("failed to resolve %s: %w", debugStr(contentPath.String()), err)
webError(w, err, http.StatusInternalServerError)
}
} else {
if !i.handleRequestErrors(w, contentPath, err) {
return false
}
}
}
if getResp.bytes != nil {
bytesResponse = getResp.bytes
defer bytesResponse.Close()
} else {
directoryMetadata = getResp.directoryMetadata
}
getterFn = i.api.Get
} else {
// TODO: Add tests for range parsing
var ranges []GetRange
Expand All @@ -85,26 +60,38 @@ func (i *handler) serveDefaults(ctx context.Context, w http.ResponseWriter, r *h
webError(w, fmt.Errorf("invalid range request: %w", err), http.StatusBadRequest)
return false
}
pathMetadata, bytesResponse, err = i.api.GetRange(ctx, maybeResolvedImPath, ranges...)
if err != nil {
if isWebRequest(requestedContentType) {
forwardedPath, continueProcessing := i.handleWebRequestErrors(w, r, maybeResolvedImPath, immutableContentPath, contentPath, err, logger)
if !continueProcessing {
return false
}
pathMetadata, bytesResponse, err = i.api.GetRange(ctx, forwardedPath, ranges...)
if err != nil {
err = fmt.Errorf("failed to resolve %s: %w", debugStr(contentPath.String()), err)
webError(w, err, http.StatusInternalServerError)
}
} else {
if !i.handleRequestErrors(w, contentPath, err) {
return false
}
getterFn = func(ctx context.Context, imPath ImmutablePath) (ContentPathMetadata, *GetResponse, error) {
return i.api.GetRange(ctx, imPath, ranges...)
}
}

var getResp *GetResponse
// TODO: passing resolved path here, instead of contentPath is harming content routing. Knowing original immutableContentPath will allow backend to find providers for parents, even when internal CIDs are not announced, and will provide better key for caching related DAGs.
pathMetadata, getResp, err = getterFn(ctx, maybeResolvedImPath)
if err != nil {
if isWebRequest(requestedContentType) {
forwardedPath, continueProcessing := i.handleWebRequestErrors(w, r, maybeResolvedImPath, immutableContentPath, contentPath, err, logger)
if !continueProcessing {
return false
}
pathMetadata, getResp, err = getterFn(ctx, forwardedPath)
if err != nil {
err = fmt.Errorf("failed to resolve %s: %w", debugStr(contentPath.String()), err)
webError(w, err, http.StatusInternalServerError)
}
} else {
if !i.handleRequestErrors(w, contentPath, err) {
return false
}
}
}
if getResp.bytes != nil {
bytesResponse = getResp.bytes
defer bytesResponse.Close()
} else {
directoryMetadata = getResp.directoryMetadata
}

default:
// This shouldn't be possible to reach which is why it is a 500 rather than 4XX error
webError(w, fmt.Errorf("invalid method: cannot use this HTTP method with the given request"), http.StatusInternalServerError)
Expand Down

0 comments on commit ff460b5

Please sign in to comment.