feat: switch gateway GetRange API to match Get

Also fixes serving range requests for /ipfs/bafydir when we know we'll
get the index.html or _redirect.

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) {

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.

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...)
 }
 

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)

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")
 }
 

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)