diff --git a/gateway/blocks_gateway.go b/gateway/blocks_gateway.go index ce18bfd9d..9b70ab51c 100644 --- a/gateway/blocks_gateway.go +++ b/gateway/blocks_gateway.go @@ -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) { diff --git a/gateway/gateway.go b/gateway/gateway.go index 86d2db8fb..9f198d3b5 100644 --- a/gateway/gateway.go +++ b/gateway/gateway.go @@ -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. diff --git a/gateway/gateway_test.go b/gateway/gateway_test.go index aa36b47e9..9a1e4b5f8 100644 --- a/gateway/gateway_test.go +++ b/gateway/gateway_test.go @@ -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...) } diff --git a/gateway/handler_defaults.go b/gateway/handler_defaults.go index 205ac6064..88fe1ed5b 100644 --- a/gateway/handler_defaults.go +++ b/gateway/handler_defaults.go @@ -29,6 +29,7 @@ func (i *handler) serveDefaults(ctx context.Context, w http.ResponseWriter, r *h isDirectoryHeadRequest bool directoryMetadata *directoryMetadata err error + ranges []GetRange ) switch r.Method { @@ -48,63 +49,51 @@ 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. 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 - } - } else { + if rangeHeader != "" { // TODO: Add tests for range parsing - var ranges []GetRange ranges, err = parseRange(rangeHeader) if err != nil { 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) - } + } + + 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. + if len(ranges) == 0 { + pathMetadata, getResp, err = i.api.Get(ctx, maybeResolvedImPath) + } else { + pathMetadata, getResp, 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 + } + if len(ranges) == 0 { + pathMetadata, getResp, err = i.api.Get(ctx, forwardedPath) } else { - if !i.handleRequestErrors(w, contentPath, err) { - return false - } + pathMetadata, getResp, 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 } } + } + 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) @@ -140,7 +129,7 @@ func (i *handler) serveDefaults(ctx context.Context, w http.ResponseWriter, r *h // Handling Unixfs directory if directoryMetadata != nil || isDirectoryHeadRequest { logger.Debugw("serving unixfs directory", "path", contentPath) - return i.serveDirectory(ctx, w, r, resolvedPath, contentPath, isDirectoryHeadRequest, directoryMetadata, begin, logger) + return i.serveDirectory(ctx, w, r, resolvedPath, contentPath, isDirectoryHeadRequest, directoryMetadata, ranges, begin, logger) } webError(w, fmt.Errorf("unsupported UnixFS type"), http.StatusInternalServerError) diff --git a/gateway/handler_test.go b/gateway/handler_test.go index 0d158042d..bf488d03b 100644 --- a/gateway/handler_test.go +++ b/gateway/handler_test.go @@ -49,7 +49,7 @@ func (api *errorMockAPI) Get(ctx context.Context, path ImmutablePath) (ContentPa return ContentPathMetadata{}, nil, api.err } -func (api *errorMockAPI) GetRange(ctx context.Context, path ImmutablePath, getRange ...GetRange) (ContentPathMetadata, files.File, error) { +func (api *errorMockAPI) GetRange(ctx context.Context, path ImmutablePath, getRange ...GetRange) (ContentPathMetadata, *GetResponse, error) { return ContentPathMetadata{}, nil, api.err } @@ -165,7 +165,7 @@ func (api *panicMockAPI) Get(ctx context.Context, immutablePath ImmutablePath) ( panic("i am panicking") } -func (api *panicMockAPI) GetRange(ctx context.Context, immutablePath ImmutablePath, ranges ...GetRange) (ContentPathMetadata, files.File, error) { +func (api *panicMockAPI) GetRange(ctx context.Context, immutablePath ImmutablePath, ranges ...GetRange) (ContentPathMetadata, *GetResponse, error) { panic("i am panicking") } diff --git a/gateway/handler_unixfs_dir.go b/gateway/handler_unixfs_dir.go index 6c5317246..423af8285 100644 --- a/gateway/handler_unixfs_dir.go +++ b/gateway/handler_unixfs_dir.go @@ -23,7 +23,7 @@ import ( // serveDirectory returns the best representation of UnixFS directory // // It will return index.html if present, or generate directory listing otherwise. -func (i *handler) serveDirectory(ctx context.Context, w http.ResponseWriter, r *http.Request, resolvedPath ipath.Resolved, contentPath ipath.Path, isHeadRequest bool, directoryMetadata *directoryMetadata, begin time.Time, logger *zap.SugaredLogger) bool { +func (i *handler) serveDirectory(ctx context.Context, w http.ResponseWriter, r *http.Request, resolvedPath ipath.Resolved, contentPath ipath.Path, isHeadRequest bool, directoryMetadata *directoryMetadata, ranges []GetRange, begin time.Time, logger *zap.SugaredLogger) bool { ctx, span := spanTrace(ctx, "ServeDirectory", trace.WithAttributes(attribute.String("path", resolvedPath.String()))) defer span.End() @@ -81,7 +81,11 @@ func (i *handler) serveDirectory(ctx context.Context, w http.ResponseWriter, r * } } else { var getResp *GetResponse - _, getResp, err = i.api.Get(ctx, imIndexPath) + if len(ranges) == 0 { + _, getResp, err = i.api.Get(ctx, imIndexPath) + } else { + _, getResp, err = i.api.GetRange(ctx, imIndexPath, ranges...) + } if err == nil { if getResp.bytes == nil { webError(w, fmt.Errorf("%q could not be read: %w", imIndexPath, files.ErrNotReader), http.StatusUnprocessableEntity)