-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Return 404 instead of HTML for missing API methods. #24590
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
Conversation
changelog/unreleased/pr-24590.toml
Outdated
| @@ -0,0 +1,4 @@ | |||
| type = "f" | |||
| message = "Return 404 instead of HTML for missing API methods." | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would find "instead of HTML for a non existing API endpoint" more clear?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅
| @Path("{filename:.*}") | ||
| public Response getIndex(@Context ContainerRequest request, | ||
| @Context HttpHeaders headers) { | ||
| if (request.getAbsolutePath().getPath().startsWith("/api")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would add a comment on the purpose here, too. and not only rely on the changelog for explanation. Also, would it make sense to add /api as a constant to be re-used? (although it might be unlikely that we ever change that path)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅
janheise
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
Motivation and Context
This PR changes the default catch-all method which is supposed to return the frontend bootstrapping HTML for deep-links to check if the missing URL is an API endpoint and return a 404 instead.
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: