setErrorParameter of error view with HasErrorParameter<NotFoundException> called for non-HTML requests

[OlliTietavainenVaadin]

Description of the bug / feature

If a Vaadin 14 application contains a class that implements HasErrorParameter<NotFoundException>, the setErrorParameter method gets invoked for any request that returns a 404 error, even for e.g. image requests (when the requested image is missing).

Minimal reproducible example

PageNotFoundView.java:

@ParentLayout(MainLayout.class)
public class PageNotFoundView extends VerticalLayout implements HasErrorParameter<NotFoundException> {

    @Override
    public int setErrorParameter(BeforeEnterEvent event, ErrorParameter<NotFoundException> parameter) {
        add(new Span("Oh no, not found"));
        return 404;
    }
}

OtherView.java:

    Image image = new Image("not_found.jpeg", "alt");
    add(image);

Expected behavior

setErrorParameter doesn't get invoked when navigating into OtherView

Actual behavior

setErrorParameter gets invoked, even though the PageNotFoundView is never displayed in the UI. The request for the image also gets a text/html response, which is against the request's accept string.

Versions:

- Vaadin / Flow version: 14.5.4
- Java version: 11
- OS version: Windows
- Browser version (if applicable): n/a
- Application Server (if applicable): Spring Boot / Tomcat (n/a) 
- IDE (if applicable): n/a

[Legioth]

which is against the request's accept string

I suspect this is the key aspect here.

We shouldn't return a navigation response (HTML) for something that isn't a navigation request but rather request something like an image or a stylesheet (based on the accept header in the request), regardless of the error condition.

[denis-anisimov]

It's weird : I cannot reproduce this with the master but can with Flow 2.6.

[denis-anisimov]

I don't see this as a bug in fact.
It's rather an enhancement and I think it can't be fixed properly.

not_found.jpeg is relative URI which will be something http://hostname/context/not_found.jpeg.
This URL for image makes a separate request to the server.
And there are two cases:

• the URL is considered as a resource URL (the current usecase)
• the URL has no semantical context.

If I ask http://hostname/context/not_found.jpeg in the browser address bar then there is no way to say whether this is a view name (some Route navigation target) or just an attempt to ask a resource.
The URL looks like a resource but I don't see any reason why I can't use not_found.jpeg as a route target.

So when you ask http://hostname/context/not_found.jpeg directly then static file server doesn't handle the request: there is no such resource and the request is handled by the last Request Handler in the list of handlers (BootstrapHandler ).
The BootstrapHandler always create a UI. So in V14 bootstrap mode a new UI instance will be created and it will try to handle not_found.jpeg. The resource doesn't exist and will be considered as a non-existent route target.
So a RouteNotFoundError component will be called for it.

And this is done by not the same UI instance but it's done by another UI instance for the image resource URL.
So in fact RouteNotFoundError is rendered but it's rendered as an image.

This behavior doesn't look like a bug for me.

And as I mentioned there is no bulletproof way to make this behavior "correct": once you start to describe the exact behavior you will immediately fail with the corner cases.

As I said: in this specific example it's only you who knows that not_found.jpeg is a theoretical image path which doesn't exist.
For the HTTP server it's just a regular URL without any additional context. And I MAY create a view with route not_found.jpeg . In this case the ticket will make a bug: the view won't be rendered.

The suggestion is : rely on the Accept header of the browser. For the not_found.jpeg browser sends Accept header which expects the image with 1.0 weight. And it also accepts everything with less weight.

So OK, I've implemented in the #11334 the logic which create a UI only if Accept header contains text/html with weight 1.0.
And this produces a bug because as I said: not_found.jpeg can be a route target (even though quite unlikely).

So the logic is extremely fragile / produces a bug in a corner case and I don't like it.

[Legioth]

The URL is irrelevant. What matters is how or why that URL is requested. I could still use not_found.jpeg as a navigation target since the browser will request HTML for a URL is in the address bar, regardless of whether the URL ends with .jpeg.

If I have a typo in the browser's address bar, then it's excepted that a new UI instance of created. If I have a typo in the src for an image, then it's confusing that a new UI instance is created.

[denis-anisimov]

The URL is relevant.
The URL is the only thing which browser has.

The Accept header is constructed based on the URL.

The only reason why image has weight 1.0 in the Accept for the not_found.jpeg URI is the jpeg extension.
And nothing else.
Accept header still accepts everything but with less weight and the weight is set only based on the jpeg extension.

So the URI is the only information which matters : everything else is based on it.

Anyway:

• I already described why I don't see a bulletproof way to solve this. The suggestion to use Accept header is implemented in the PR and I don't like it.
• The ticket is not a bug but it's rather an enhancement
• I personally don't see reasons to spend so much time on fixing something which can not be done reliably.
• I cannot do anything better than already done in the provided PR without exact description of how this should be fixed: the description provided in the ticket is an example with false expectation because it's based on expectation which can be easily broken.

So : this needs the exact design with the description what and how should be done to handle this properly.

[pleku]

I agree with Denis. Unless there is a way to do this reliably, we probably should not do it at all as it is not a priority but takes significant time.

[pleku]

Looking at what accept headers browsers set for resources in https://taskmob.demo.vaadin.com/and https://cookbook.vaadin.com/ applications, one option would be to not serve a UI with route not found error view for requests that have Accept header of

text/css,*/*;q=0.1        // chrome + edge
text/css                     // firefox

image/webp,*/*          // firefox
image/avif,image/webp,image/apng,image/svg+xml,image/*,*/*;q=0.8  // chrome + edge

But this doesn't solve the problem - it just explicitly checks for these headers. There are probably others that could be included too. And CSS typically doesn't even come through the application server in production. Same is for fonts, which cannot be even detected through accept headers. And .js is maybe the most difficult - the accept headers wary a lot for those resources and it is probably impossible to not serve a UI for them if they land to the BootstrapHandler.

[Legioth]

The Accept header is constructed based on the URL.

In which browser is that? I tested with Chrome, where it seems to only vary based on where the URL is used but not based on the contents of the URL.

No matter what URL prefix I use (I tested missing.html, missing.jpg and missing.js), I always get the same Accept header based on where I use the URL.

Usage type	Accept value
Address bar, clicking a link, <iframe>	text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.9
<img>	image/avif,image/webp,image/apng,image/svg+xml,image/*,*/*;q=0.8
<link rel="stylesheet">	text/css,*/*;q=0.1

Scripts and webfonts are indeed difficult to distinguish since at least my Chrome requests those as */*.

We don't need to have perfect logic for when to create an UI instance and when not, but the more cases we can avoid confusion, the better.

• 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.

[anssit]

Behaviour in V22 in its default bootstrap mode is different. Created a new issue for that (#12965). In legacy V14 bootstrap mode (-Dvaadin.useDeprecatedV14Bootstrapping=true) in V22, the behaviour is the same as described in this ticket.

[anssit]

For V23 this issue has been fixed in #12571

[anssit]

Fix for V14 legacy bootstrap mode for the upcoming V23.1 in #13242.

[vaadin-bot]

This ticket/PR has been released with Vaadin 23.0.2.