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: handle _redirects for If-None-Match headers #412

Merged
merged 6 commits into from
Jul 25, 2023
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ The following emojis are used to highlight certain changes:
### Fixed

- Removed mentions of unused ARC algorithm ([#336](https://github.com/ipfs/boxo/issues/366#issuecomment-1597253540))
- Handle `_redirects` file when `If-None-Match` header is present ([#412](https://github.com/ipfs/boxo/pull/412))

### Security

Expand Down
55 changes: 55 additions & 0 deletions gateway/gateway_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,61 @@ func TestRedirects(t *testing.T) {
res = mustDoWithoutRedirect(t, req)
require.Equal(t, http.StatusNotFound, res.StatusCode)
})

t.Run("_redirects file with If-None-Match header", func(t *testing.T) {
t.Parallel()

backend, root := newMockBackend(t, "redirects-spa.car")
backend.namesys["/ipns/example.com"] = path.FromCid(root)

ts := newTestServerWithConfig(t, backend, Config{
Headers: map[string][]string{},
NoDNSLink: false,
PublicGateways: map[string]*PublicGateway{
"example.com": {
UseSubdomains: true,
DeserializedResponses: true,
},
},
DeserializedResponses: true,
})

missingPageURL := ts.URL + "/missing-page"

do := func(method string) {
// Make initial request to non-existing page that should return the contents
// of index.html as per the _redirects file.
req := mustNewRequest(t, method, missingPageURL, nil)
req.Header.Add("Accept", "text/html")
req.Host = "example.com"

res := mustDoWithoutRedirect(t, req)
defer res.Body.Close()

// Check statuses and body.
require.Equal(t, http.StatusOK, res.StatusCode)
body, err := io.ReadAll(res.Body)
require.NoError(t, err)
require.Equal(t, "hello world\n", string(body))

// Check Etag.
etag := res.Header.Get("Etag")
require.NotEmpty(t, etag)

// Repeat request with Etag as If-None-Match value. Expect 304 Not Modified.
req = mustNewRequest(t, method, missingPageURL, nil)
req.Header.Add("Accept", "text/html")
req.Host = "example.com"
req.Header.Add("If-None-Match", etag)

res = mustDoWithoutRedirect(t, req)
defer res.Body.Close()
require.Equal(t, http.StatusNotModified, res.StatusCode)
}

do(http.MethodGet)
do(http.MethodHead)
})
}

func TestDeserializedResponses(t *testing.T) {
Expand Down
16 changes: 13 additions & 3 deletions gateway/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -703,9 +703,19 @@
if ifNoneMatch := r.Header.Get("If-None-Match"); ifNoneMatch != "" {
pathMetadata, err := i.backend.ResolvePath(r.Context(), rq.immutablePath)
if err != nil {
err = fmt.Errorf("failed to resolve %s: %w", debugStr(rq.contentPath.String()), err)
i.webError(w, r, err, http.StatusInternalServerError)
return true
var forwardedPath ImmutablePath
var continueProcessing bool
if isWebRequest(rq.responseFormat) {
forwardedPath, continueProcessing = i.handleWebRequestErrors(w, r, rq.mostlyResolvedPath(), rq.immutablePath, rq.contentPath, err, rq.logger)
if continueProcessing {
pathMetadata, err = i.backend.ResolvePath(r.Context(), forwardedPath)
}
}
if !continueProcessing || err != nil {
err = fmt.Errorf("failed to resolve %s: %w", debugStr(rq.contentPath.String()), err)
i.webError(w, r, err, http.StatusInternalServerError)
return true
}

Check warning on line 718 in gateway/handler.go

View check run for this annotation

Codecov / codecov/patch

gateway/handler.go#L715-L718

Added lines #L715 - L718 were not covered by tests
}

pathCid := pathMetadata.LastSegment.Cid()
Expand Down
19 changes: 17 additions & 2 deletions gateway/handler_defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,23 @@
case http.MethodHead:
var data files.Node
pathMetadata, data, err = i.backend.Head(ctx, rq.mostlyResolvedPath())
if !i.handleRequestErrors(w, r, rq.contentPath, err) {
return false
if err != nil {
if isWebRequest(rq.responseFormat) {
forwardedPath, continueProcessing := i.handleWebRequestErrors(w, r, rq.mostlyResolvedPath(), rq.immutablePath, rq.contentPath, err, rq.logger)
if !continueProcessing {
return false
}
pathMetadata, data, err = i.backend.Head(ctx, forwardedPath)
if err != nil {
err = fmt.Errorf("failed to resolve %s: %w", debugStr(rq.contentPath.String()), err)
i.webError(w, r, err, http.StatusInternalServerError)
return false
}
} else {
if !i.handleRequestErrors(w, r, rq.contentPath, err) {
return false
}

Check warning on line 51 in gateway/handler_defaults.go

View check run for this annotation

Codecov / codecov/patch

gateway/handler_defaults.go#L36-L51

Added lines #L36 - L51 were not covered by tests
}
}
defer data.Close()
if _, ok := data.(files.Directory); ok {
Expand Down
Binary file added gateway/testdata/redirects-spa.car
Binary file not shown.