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): 404 when a valid DAG is missing link #9126

Merged
merged 1 commit into from
Jul 22, 2022
Merged

Conversation

lidel
Copy link
Member

@lidel lidel commented Jul 19, 2022

This PR makes sure gateway returns 404 (Not Found) only when a valid DAG is missing the requested child.
In other cases, 400 (Bad Request) is returned.

Uses ipld.IsNotFound added in #8838

Closes #8924
Closes #9064

@lidel lidel self-assigned this Jul 19, 2022
Comment on lines +785 to +786
} else if ipld.IsNotFound(err) {
webErrorWithCode(w, message, err, http.StatusNotFound)
Copy link
Member Author

@lidel lidel Jul 19, 2022

Choose a reason for hiding this comment

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

ℹ️ this makes sure every webError gets 404 when appropriate (even when it was called with http.StatusBadRequest)

@lidel lidel requested review from Jorropo and iand July 19, 2022 20:38
@lidel lidel marked this pull request as ready for review July 19, 2022 20:39
Copy link
Contributor

@iand iand left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -235,17 +235,17 @@ func TestGatewayGet(t *testing.T) {
{"127.0.0.1:8080", "/", http.StatusNotFound, "404 page not found\n"},
{"127.0.0.1:8080", "/" + k.Cid().String(), http.StatusNotFound, "404 page not found\n"},
{"127.0.0.1:8080", k.String(), http.StatusOK, "fnord"},
{"127.0.0.1:8080", "/ipns/nxdomain.example.com", http.StatusNotFound, "ipfs resolve -r /ipns/nxdomain.example.com: " + namesys.ErrResolveFailed.Error() + "\n"},
{"127.0.0.1:8080", "/ipns/%0D%0A%0D%0Ahello", http.StatusNotFound, "ipfs resolve -r /ipns/\\r\\n\\r\\nhello: " + namesys.ErrResolveFailed.Error() + "\n"},
{"127.0.0.1:8080", "/ipns/nxdomain.example.com", http.StatusBadRequest, "ipfs resolve -r /ipns/nxdomain.example.com: " + namesys.ErrResolveFailed.Error() + "\n"},
Copy link
Contributor

Choose a reason for hiding this comment

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

Note I think resolution errors should be reported as 502 Bad Gateway but that's not what the gateway spec says. I'll raise an issue there instead

Copy link
Member Author

@lidel lidel Jul 22, 2022

Choose a reason for hiding this comment

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

Continued in ipfs/specs#300 👍

@lidel lidel merged commit 7992025 into master Jul 22, 2022
@lidel lidel deleted the fix/gw-errors-400-404 branch July 22, 2022 22:16
@BigLep BigLep mentioned this pull request Jul 26, 2022
68 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
2 participants