Skip to content
18 changes: 9 additions & 9 deletions .github/workflows/gateway-conformance.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
4 changes: 2 additions & 2 deletions gateway/backend_blocks.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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
}

Expand Down
27 changes: 24 additions & 3 deletions gateway/backend_car.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion gateway/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💭 This triggered some AI checks – https://github.com/ipfs/boxo/security/code-scanning/7 – seems to be an issue already present in legacy code, unrelated to this PR.

I don't think this is a problem blocking this PR because the gateway range-requests do not mutate UnixFS DAGs.

Still, filled potential fix in #936 to make linter warning go away.

From int64
To *int64
}

Expand Down
2 changes: 1 addition & 1 deletion gateway/handler_block.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion gateway/handler_codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 5 additions & 5 deletions gateway/handler_defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 <suffix-length>
Expand All @@ -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")
}
Expand All @@ -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
Expand Down
25 changes: 21 additions & 4 deletions gateway/serve_http_content.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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
}
}
Expand Down
Loading