Don't use NotFoundException view for resources #11334
Conversation
denis-anisimov
changed the title
|
SonarQube analysis reported 2 issues
|
| || typesWithWeight.contains(TEXT_HTML_TYPE_WEIGHT) | ||
| || typesWithWeight.contains( | ||
| ContentType.APPLICATION_XHTML_XML.getMimeType()) | ||
| || typesWithWeight.contains(XHTML_TYPE_WEIGHT); |
There was a problem hiding this comment.
I think the suggestion from @Legioth makes sense, what do you think @denis-anisimov ?
Anything with text/html in the group with highest priority should certainly cause a UI to be created.
If we want to slightly reduce the risk of regressions, then we could consider to also create a UI for requests with text/html at a lower priority, or with only / as the only value.
We should definitely not create an UI if the browser doesn't at all ask for text/html or if it has / only to supplement some more specific request.
There was a problem hiding this comment.
I don't think I understand what you mean.
Anything with text/html in the group with highest priority should certainly cause a UI to be created.
Implemented here in this PR.
If we want to slightly reduce the risk of regressions, then we could consider to also create a UI for requests with text/html at a lower priority, or with only / as the only value.
We should definitely not create an UI if the browser doesn't at all ask for text/html or if it has / only to supplement some more specific request.
It is possible to configure Accept header manually in the browser so technically it may contain anything (and unexpected things) but any meaningful request contains Accept header which accepts everything with lower weight.
E.g. image request Accept header is image/avif,image/webp,image/apng,image/svg+xml,image/*,*/*;q=0.8.
*/* matches text/html.
So it doesn't add anything to clarify the PR/direction to improve it.
If you see some exact steps which may be done here please just describe them in the details.
There was a problem hiding this comment.
If the browser specifically asks for text/html at any priority, then that's a clear indication that the browser might intend to use the response body as HTML.
Browsers usually include */* at a low priority as a fallback (e.g. text/css,*/*;q=0.1), so it would defeat the purpose if we would interpret that to match text/html even though it does match from a strictly technical sense.
As an exception to the exception, asking for only */* could still be defined to return a Flow bootstrap page since that's a very generic request for which we cannot make any assumptions for what the browser might do.
It is possible to configure
Acceptheader manually in the browser so technically it may contain anything
If the user has configured the browser to send an arbitrary Accept header value, then it's quite acceptable that the error handling might not work in the best possible way. The only result is that the user will see a generic 404 response instead of seeing a Flow view that says that nothing was found for the given URL. If the value doesn't match any of the patterns that we look for, then we can treat it the same way as if no Accept header was present.
|
|
|
Closing this PR for now as it is not deemed important enough to make it better. Please comment on the issue #10965 if this is important for someone. |
Description
Please list all relevant dependencies in this section and provide summary of the change, motivation and context.
Fixes # (issue)
Type of change
Checklist
Additional for
Featuretype of change