diff --git a/.github/workflows/gateway-conformance.yml b/.github/workflows/gateway-conformance.yml index 60a2cfc18..47bb861e7 100644 --- a/.github/workflows/gateway-conformance.yml +++ b/.github/workflows/gateway-conformance.yml @@ -22,7 +22,7 @@ jobs: steps: # 1. Download the gateway-conformance fixtures - name: Download gateway-conformance fixtures - uses: ipfs/gateway-conformance/.github/actions/extract-fixtures@v0.7 + uses: ipfs/gateway-conformance/.github/actions/extract-fixtures@v0.8 with: output: fixtures merged: true @@ -46,8 +46,8 @@ jobs: run: boxo/examples/gateway/car-file/test-gateway -c fixtures/fixtures.car -p 8040 & # 4. Run the gateway-conformance tests - - name: Run gateway-conformance tests - uses: ipfs/gateway-conformance/.github/actions/test@v0.7 + - name: Run gateway-conformance tests without IPNS and DNSLink + uses: ipfs/gateway-conformance/.github/actions/test@v0.8 with: gateway-url: http://127.0.0.1:8040 subdomain-url: http://example.net:8040 @@ -84,7 +84,7 @@ jobs: steps: # 1. Download the gateway-conformance fixtures - name: Download gateway-conformance fixtures - uses: ipfs/gateway-conformance/.github/actions/extract-fixtures@v0.7 + uses: ipfs/gateway-conformance/.github/actions/extract-fixtures@v0.8 with: output: fixtures merged: true @@ -113,8 +113,8 @@ jobs: run: boxo/examples/gateway/proxy-blocks/test-gateway -g http://127.0.0.1:8030 -p 8040 & # 4. Run the gateway-conformance tests - - name: Run gateway-conformance tests - uses: ipfs/gateway-conformance/.github/actions/test@v0.7 + - name: Run gateway-conformance tests without IPNS and DNSLink + uses: ipfs/gateway-conformance/.github/actions/test@v0.8 with: gateway-url: http://127.0.0.1:8040 # we test gateway that is backed by a remote block gateway subdomain-url: http://example.net:8040 @@ -152,7 +152,7 @@ jobs: steps: # 1. Download the gateway-conformance fixtures - name: Download gateway-conformance fixtures - uses: ipfs/gateway-conformance/.github/actions/extract-fixtures@v0.7 + uses: ipfs/gateway-conformance/.github/actions/extract-fixtures@v0.8 with: output: fixtures merged: true @@ -181,8 +181,8 @@ jobs: run: boxo/examples/gateway/proxy-car/test-gateway -g http://127.0.0.1:8030 -p 8040 & # 4. Run the gateway-conformance tests - - name: Run gateway-conformance tests - uses: ipfs/gateway-conformance/.github/actions/test@v0.7 + - name: Run gateway-conformance tests without IPNS and DNSLink + uses: ipfs/gateway-conformance/.github/actions/test@v0.8 with: gateway-url: http://127.0.0.1:8040 # we test gateway that is backed by a remote car gateway subdomain-url: http://example.net:8040 diff --git a/CHANGELOG.md b/CHANGELOG.md index 9299d9f99..7853870d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,8 @@ The following emojis are used to highlight certain changes: ### Fixed +- `gateway`: Fixed suffix range-requests and updated tests to [gateway-conformance v0.8](https://github.com/ipfs/gateway-conformance/releases/tag/v0.8.0) [#922](https://github.com/ipfs/boxo/pull/922) + ### Security diff --git a/gateway/backend_blocks.go b/gateway/backend_blocks.go index c2fde00fe..468d67e94 100644 --- a/gateway/backend_blocks.go +++ b/gateway/backend_blocks.go @@ -141,7 +141,7 @@ func (bb *BlocksBackend) Get(ctx context.Context, path path.ImmutablePath, range } if rootCodec == uint64(mc.Raw) { - if err := seekToRangeStart(f, ra); err != nil { + if err := seekToRangeStart(f, ra, fileSize); err != nil { return ContentPathMetadata{}, nil, err } } @@ -182,7 +182,7 @@ func (bb *BlocksBackend) Get(ctx context.Context, path path.ImmutablePath, range return ContentPathMetadata{}, nil, err } - if err := seekToRangeStart(file, ra); err != nil { + if err := seekToRangeStart(file, ra, fileSize); err != nil { return ContentPathMetadata{}, nil, err } diff --git a/gateway/backend_car.go b/gateway/backend_car.go index 567b0cfac..b7bac515b 100644 --- a/gateway/backend_car.go +++ b/gateway/backend_car.go @@ -363,7 +363,7 @@ func (api *CarBackend) Get(ctx context.Context, path path.ImmutablePath, byteRan if rangeCount > 0 { r := byteRanges[0] carParams.Range = &DagByteRange{ - From: int64(r.From), + From: r.From, } // TODO: move to boxo or to loadRequestIntoSharedBlockstoreAndBlocksGateway after we pass params in a humane way @@ -442,13 +442,25 @@ func loadTerminalEntity(ctx context.Context, c cid.Cid, blk blocks.Block, lsys * } f := files.NewBytesFile(blockData) + size := int64(len(blockData)) + from := int64(0) if params.Range != nil && params.Range.From != 0 { - if _, err := f.Seek(params.Range.From, io.SeekStart); err != nil { + from = params.Range.From + if from < 0 { // negative range from the end of file + if params.Range.To != nil { + return nil, fmt.Errorf("invalid car backend range: negative start without a nil end") + } + from = size + from + if from < 0 { + return nil, fmt.Errorf("invalid car backend range: negative start bigger than the file size") + } + } + if _, err := f.Seek(from, io.SeekStart); err != nil { return nil, err } } - return NewGetResponseFromReader(f, int64(len(blockData))), nil + return NewGetResponseFromReader(f, size), nil } blockData, pbn, ufsFieldData, fieldNum, err := loadUnixFSBase(ctx, c, blk, lsys) @@ -554,6 +566,15 @@ func loadTerminalEntity(ctx context.Context, c cid.Cid, blk blocks.Block, lsys * if params.Range != nil { from = params.Range.From byteRange = *params.Range + if from < 0 { // negative range from the end of file + if params.Range.To != nil { + return nil, fmt.Errorf("invalid car backend range: negative start without a nil end") + } + from = fileSize + from + if from < 0 { + return nil, fmt.Errorf("invalid car backend range: negative start bigger than the file size") + } + } } _, err = f.Seek(from, io.SeekStart) if err != nil { diff --git a/gateway/gateway.go b/gateway/gateway.go index d79689925..6cf78f5af 100644 --- a/gateway/gateway.go +++ b/gateway/gateway.go @@ -219,10 +219,11 @@ type ContentPathMetadata struct { // - From >= 0 and To = nil: Get the file (From, Length) // - From >= 0 and To >= 0: Get the range (From, To) // - From >= 0 and To <0: Get the range (From, Length - To) +// - From < 0 and To = nil: Get the file (Length - From, Length) // // [HTTP Byte Range]: https://httpwg.org/specs/rfc9110.html#rfc.section.14.1.2 type ByteRange struct { - From uint64 + From int64 To *int64 } diff --git a/gateway/handler_block.go b/gateway/handler_block.go index e6bf2267f..27507272e 100644 --- a/gateway/handler_block.go +++ b/gateway/handler_block.go @@ -44,7 +44,7 @@ func (i *handler) serveRawBlock(ctx context.Context, w http.ResponseWriter, r *h return false } - if !i.seekToStartOfFirstRange(w, r, data) { + if !i.seekToStartOfFirstRange(w, r, data, sz) { return false } diff --git a/gateway/handler_codec.go b/gateway/handler_codec.go index fdc69e708..28b6db397 100644 --- a/gateway/handler_codec.go +++ b/gateway/handler_codec.go @@ -243,7 +243,7 @@ func (i *handler) serveCodecRaw(ctx context.Context, w http.ResponseWriter, r *h // ServeContent will take care of // If-None-Match+Etag, Content-Length and setting range request headers after we've already seeked to the start of // the first range - if !i.seekToStartOfFirstRange(w, r, blockData) { + if !i.seekToStartOfFirstRange(w, r, blockData, blockSize) { return false } _, dataSent, _ := serveContent(w, r, modtime, blockSize, blockData) diff --git a/gateway/handler_defaults.go b/gateway/handler_defaults.go index c53daf68f..ca06bd26f 100644 --- a/gateway/handler_defaults.go +++ b/gateway/handler_defaults.go @@ -52,7 +52,7 @@ func (i *handler) serveDefaults(ctx context.Context, w http.ResponseWriter, r *h case http.MethodGet: rangeHeader := r.Header.Get("Range") if rangeHeader != "" { - // TODO: Add tests for range parsing + // Note: proper tests for Range parsing live in https://github.com/ipfs/gateway-conformance ranges, err = parseRangeWithoutLength(rangeHeader) if err != nil { i.webError(w, r, fmt.Errorf("invalid range request: %w", err), http.StatusBadRequest) @@ -185,7 +185,7 @@ func parseRangeWithoutLength(s string) ([]ByteRange, error) { start, end = textproto.TrimString(start), textproto.TrimString(end) var r ByteRange if start == "" { - r.From = 0 + r.To = nil // If no start is specified, end specifies the // range start relative to the end of the file, // and we are dealing with @@ -198,9 +198,9 @@ func parseRangeWithoutLength(s string) ([]ByteRange, error) { if i < 0 || err != nil { return nil, errors.New("invalid range") } - r.To = &i + r.From = -i } else { - i, err := strconv.ParseUint(start, 10, 64) + i, err := strconv.ParseInt(start, 10, 64) if err != nil { return nil, errors.New("invalid range") } @@ -210,7 +210,7 @@ func parseRangeWithoutLength(s string) ([]ByteRange, error) { r.To = nil } else { i, err := strconv.ParseInt(end, 10, 64) - if err != nil || i < 0 || r.From > uint64(i) { + if err != nil || i < 0 || r.From > i { return nil, errors.New("invalid range") } r.To = &i diff --git a/gateway/serve_http_content.go b/gateway/serve_http_content.go index 38b6a5313..6b620bd1a 100644 --- a/gateway/serve_http_content.go +++ b/gateway/serve_http_content.go @@ -112,6 +112,11 @@ func httpServeContent(w http.ResponseWriter, r *http.Request, modtime time.Time, w.Header().Set("Content-Length", strconv.FormatInt(sendSize, 10)) } + if contentType := w.Header().Get("Content-Type"); contentType == "" { + // Ensure empty string is not returned as value + delete(w.Header(), "Content-Type") + } + w.WriteHeader(code) if r.Method != http.MethodHead { @@ -455,7 +460,7 @@ func sumRangesSize(ranges []httpRange) (size int64) { } // seekToStartOfFirstRange seeks to the start of the first Range if the request is an HTTP Range Request -func (i *handler) seekToStartOfFirstRange(w http.ResponseWriter, r *http.Request, data io.Seeker) bool { +func (i *handler) seekToStartOfFirstRange(w http.ResponseWriter, r *http.Request, data io.Seeker, size int64) bool { rangeHeader := r.Header.Get("Range") if rangeHeader != "" { ranges, err := parseRangeWithoutLength(rangeHeader) @@ -465,7 +470,7 @@ func (i *handler) seekToStartOfFirstRange(w http.ResponseWriter, r *http.Request } if len(ranges) > 0 { ra := &ranges[0] - err = seekToRangeStart(data, ra) + err = seekToRangeStart(data, ra, size) if err != nil { i.webError(w, r, fmt.Errorf("could not seek to location in range request: %w", err), http.StatusBadRequest) return false @@ -475,9 +480,21 @@ func (i *handler) seekToStartOfFirstRange(w http.ResponseWriter, r *http.Request return true } -func seekToRangeStart(data io.Seeker, ra *ByteRange) error { +func seekToRangeStart(data io.Seeker, ra *ByteRange, size int64) error { if ra != nil && ra.From != 0 { - if _, err := data.Seek(int64(ra.From), io.SeekStart); err != nil { + start := int64(0) + if ra.From < 0 { + if ra.To != nil { + return fmt.Errorf("invalid range: negative start without a nil end") + } + start = size + ra.From + if start < 0 { + return fmt.Errorf("invalid range: negative start bigger than the file size") + } + } else { + start = ra.From + } + if _, err := data.Seek(start, io.SeekStart); err != nil { return err } }