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(gateway): do not act on template errors #515

Merged
merged 3 commits into from
Nov 22, 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
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ The following emojis are used to highlight certain changes:

### Fixed

* `boxo/gateway`: a panic (which is recovered) could sporadically be triggered inside a CAR request, if the right [conditions were met](https://github.com/ipfs/boxo/pull/511).
* `boxo/gateway`
* a panic (which is recovered) could sporadically be triggered inside a CAR request, if the right [conditions were met](https://github.com/ipfs/boxo/pull/511).
* no longer emits `http: superfluous response.WriteHeader` warnings when an error happens.

### Security

Expand Down
3 changes: 1 addition & 2 deletions gateway/assets/test/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,7 @@ func runTemplate(w http.ResponseWriter, filename string, data interface{}) {
}
err = tpl.Execute(w, data)
if err != nil {
http.Error(w, fmt.Sprintf("failed to execute template: %s", err), http.StatusInternalServerError)
return
_, _ = w.Write([]byte(fmt.Sprintf("error during body generation: %v", err)))
}
}

Expand Down
5 changes: 4 additions & 1 deletion gateway/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,14 +165,17 @@
if acceptsHTML {
w.Header().Set("Content-Type", "text/html")
w.WriteHeader(code)
_ = assets.ErrorTemplate.Execute(w, assets.ErrorTemplateData{
err = assets.ErrorTemplate.Execute(w, assets.ErrorTemplateData{
GlobalData: assets.GlobalData{
Menu: c.Menu,
},
StatusCode: code,
StatusText: http.StatusText(code),
Error: err.Error(),
})
if err != nil {
_, _ = w.Write([]byte(fmt.Sprintf("error during body generation: %v", err)))
}

Check warning on line 178 in gateway/errors.go

View check run for this annotation

Codecov / codecov/patch

gateway/errors.go#L177-L178

Added lines #L177 - L178 were not covered by tests
} else {
http.Error(w, err.Error(), code)
}
Expand Down
7 changes: 4 additions & 3 deletions gateway/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -912,12 +912,13 @@
// - redirects to intendedURL after a short delay

w.WriteHeader(http.StatusBadRequest)
if err := redirectTemplate.Execute(w, redirectTemplateData{
err = redirectTemplate.Execute(w, redirectTemplateData{

Check warning on line 915 in gateway/handler.go

View check run for this annotation

Codecov / codecov/patch

gateway/handler.go#L915

Added line #L915 was not covered by tests
RedirectURL: intendedURL,
SuggestedPath: intendedPath.String(),
ErrorMsg: fmt.Sprintf("invalid path: %q should be %q", r.URL.Path, intendedPath.String()),
}); err != nil {
i.webError(w, r, fmt.Errorf("failed to redirect when fixing superfluous namespace: %w", err), http.StatusBadRequest)
lidel marked this conversation as resolved.
Show resolved Hide resolved
})
if err != nil {
_, _ = w.Write([]byte(fmt.Sprintf("error during body generation: %v", err)))

Check warning on line 921 in gateway/handler.go

View check run for this annotation

Codecov / codecov/patch

gateway/handler.go#L919-L921

Added lines #L919 - L921 were not covered by tests
}

return true
Expand Down
11 changes: 5 additions & 6 deletions gateway/handler_codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,20 +195,19 @@
w.Header().Del("Cache-Control")

cidCodec := mc.Code(resolvedPath.RootCid().Prefix().Codec)
if err := assets.DagTemplate.Execute(w, assets.DagTemplateData{
err = assets.DagTemplate.Execute(w, assets.DagTemplateData{
GlobalData: i.getTemplateGlobalData(r, contentPath),
Path: contentPath.String(),
CID: resolvedPath.RootCid().String(),
CodecName: cidCodec.String(),
CodecHex: fmt.Sprintf("0x%x", uint64(cidCodec)),
Node: parseNode(blockCid, blockData),
}); err != nil {
err = fmt.Errorf("failed to generate HTML listing for this DAG: try fetching raw block with ?format=raw: %w", err)
i.webError(w, r, err, http.StatusInternalServerError)
return false
})
if err != nil {
_, _ = w.Write([]byte(fmt.Sprintf("error during body generation: %v", err)))

Check warning on line 207 in gateway/handler_codec.go

View check run for this annotation

Codecov / codecov/patch

gateway/handler_codec.go#L207

Added line #L207 was not covered by tests
}

return true
return err == nil
}

// parseNode does a best effort attempt to parse this request's block such that
Expand Down
2 changes: 1 addition & 1 deletion gateway/handler_unixfs_dir.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ func (i *handler) serveDirectory(ctx context.Context, w http.ResponseWriter, r *
rq.logger.Debugw("request processed", "tplDataDNSLink", globalData.DNSLink, "tplDataSize", size, "tplDataBackLink", backLink, "tplDataHash", hash)

if err := assets.DirectoryTemplate.Execute(w, tplData); err != nil {
i.webError(w, r, err, http.StatusInternalServerError)
_, _ = w.Write([]byte(fmt.Sprintf("error during body generation: %v", err)))
return false
}

Expand Down
Loading