Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(gw): entity-bytes with negative indexes beyond file size #523

Merged
merged 5 commits into from
Jan 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/gateway-conformance.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,18 @@ jobs:
steps:
# 1. Download the gateway-conformance fixtures
- name: Download gateway-conformance fixtures
uses: ipfs/gateway-conformance/.github/actions/extract-fixtures@v0.4
uses: ipfs/gateway-conformance/.github/actions/extract-fixtures@v0.5
with:
output: fixtures
merged: true

# 2. Build the car-gateway
- name: Setup Go
uses: actions/setup-go@v3
uses: actions/setup-go@v4
with:
go-version: 1.21.x
- name: Checkout boxo
uses: actions/checkout@v3
uses: actions/checkout@v4
with:
path: boxo
- name: Build car-gateway
Expand All @@ -40,7 +40,7 @@ jobs:

# 4. Run the gateway-conformance tests
- name: Run gateway-conformance tests
uses: ipfs/gateway-conformance/.github/actions/test@v0.4
uses: ipfs/gateway-conformance/.github/actions/test@v0.5
with:
gateway-url: http://127.0.0.1:8040
json: output.json
Expand Down
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ The following emojis are used to highlight certain changes:

### Removed

- 🛠 `gateway`: the header configuration `Config.Headers` and `AddAccessControlHeaders` has been replaced by the new middleware provided by `NewHeaders`.
### Fixed

- 🛠 `boxo/gateway`: when making a trustless CAR request with the "entity-bytes" parameter, using a negative index greater than the underlying entity length could trigger reading more data than intended
- 🛠 `boxo/gateway`: the header configuration `Config.Headers` and `AddAccessControlHeaders` has been replaced by the new middleware provided by `NewHeaders`.

### Security

Expand Down
15 changes: 10 additions & 5 deletions gateway/blocks_backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,9 @@
return err
}
from = fileLength + entityRange.From
if from < 0 {
from = 0
}

Check warning on line 513 in gateway/blocks_backend.go

View check run for this annotation

Codecov / codecov/patch

gateway/blocks_backend.go#L511-L513

Added lines #L511 - L513 were not covered by tests
lidel marked this conversation as resolved.
Show resolved Hide resolved
foundFileLength = true
}

Expand All @@ -521,13 +524,15 @@
}

to := *entityRange.To
if (*entityRange.To) < 0 && !foundFileLength {
fileLength, err = f.Seek(0, io.SeekEnd)
if err != nil {
return err
if (*entityRange.To) < 0 {
if !foundFileLength {
fileLength, err = f.Seek(0, io.SeekEnd)
if err != nil {
return err
}
foundFileLength = true

Check warning on line 533 in gateway/blocks_backend.go

View check run for this annotation

Codecov / codecov/patch

gateway/blocks_backend.go#L527-L533

Added lines #L527 - L533 were not covered by tests
}
to = fileLength + *entityRange.To
foundFileLength = true
}

numToRead := 1 + to - from
Expand Down
Loading