Vaadin 23.2.0: Add-on resources fail to load if servlet not mapped at the top level

[archiecobbs]

Description of the bug

Vaadin appears to still be making the unwarranted assumption that everybody in the world maps their Vaadin servlets at the root of their servlet context.

I want to map my Vaadin servlet not to the root of the servlet context, e.g. something like this:

    <servlet-mapping>
        <servlet-name>MyVaadinServlet</servlet-name>
        <url-pattern>/demo/*</url-pattern>
    </servlet-mapping>

But when you do this, add-ons resources fail to load, which causes add-ons to not work.

This might be a lingering variant of issue #14239? See that issue for some background.

Expected behavior

Add-ons should be able to load their resources and work properly no matter how the Vaadin servlet is mapped.

Minimal reproducible example

Please see https://github.com/archiecobbs/vaadin-flow-bugs/tree/issue-14239

When you run the demo, you will see that the Javascript highlighting add-on is not working.

In Firefox the inspector console will show the reason, which is that the add-on's resources are failing to load:

Loading failed for the <script> with source “http://localhost:8090/webapps/demo/ace-builds/src-min-noconflict/theme-chrome.js”.
Loading failed for the <script> with source “http://localhost:8090/webapps/demo/ace-builds/src-min-noconflict/mode-javascript.js”.

Versions

• Vaadin / Flow version: 23.2.0
• Java version: 11
• OS version: MacOS
• Browser version (if applicable): Firefox 104.0.2
• Application Server (if applicable): Tomcat 9
• IDE (if applicable): N/A

[mcollovati]

It seems like the problem is StaticFileServer not stripping servletPath before trying to load requested resource from classpath.

Perhaps with the latest changes for Vite support, StaticFileServer.getRequestFilename() may not need be needed anymore and we can just consider HttpServletRequest.getPathInfo. To be checked.

[archiecobbs]

@mcollovati,

It seems like the problem is StaticFileServer not stripping servletPath before trying to load requested resource from classpath.

Yes that makes sense... in fact, I already have a patch that should fix this. Please see https://github.com/vaadin/flow/issues/14239#issuecomment-1213435280

[archiecobbs]

FYI I also came up with a more complete patch at one point including updating unit tests: https://github.com/vaadin/flow/commit/57c5b24013b75439453baed85f800a0586aac604

[mcollovati]

Adding some context for a better understanding of current situation:

• StaticFileServer javadocs states that it handles Web content or META-INF/resources in the case that {@link VaadinServlet} is mapped using "/*"; with a different mapping, such resources are served directly by the servlet container. Some notes can be found also on documentation https://vaadin.com/docs/latest/advanced/loading-resources/#using-a-servlet-path
• Resources failing to load are stored in META-INF/resources of the Ace Editor add-on
• Ace Editor loads resource by adding <script> tags to head section of the page, prepending path with a configurable baseUrl setting whose default is ace-builds/src-min-noconflict/ (so relative to the current page)
• Ace Editor add-on has setBaseUrl method to set the configuration; setting it to ../ace-builds/src-min-noconflict/ fixes the issue (of course value should not be hard-coded, but computed on current servletPath or it could <context-path>/ace-builds/src-min-noconflict/). Setting baseUrl is also mentioned in this issue https://github.com/F0rce/ace/issues/33

[archiecobbs]

@mcollovati thanks for the additional info.

StaticFileServer javadocs states that it handles Web content or META-INF/resources in the case that {@link VaadinServlet} is mapped using "/*"; with a different mapping, such resources are served directly by the servlet container.

This doesn't really make sense to me, because it breaks context-mapping portability of Vaadin.

I understand however that some people like to map static files once, separately, but that particular deployment idea shouldn't be forced on everyone in my opinion. But I also realize my opinion is not the only one.

Maybe the best answer is to just make this configurable? That is, add a new Vaadin servlet property or other type of deployment configuration variable to configure which behavior we get?

Then we could simply have StaticFileServer give either the current behavior or (with my patch) the new "fully portable" behavior when this new deployment configuration variable is enabled.

[mcollovati]

This doesn't really make sense to me, because it breaks context-mapping portability of Vaadin

I don't see any problem with context mapping, since every deployed webapp is isolated, and resources are served per webapp. Maybe I didn't understood correctly your statement

[archiecobbs]

I don't see any problem with context mapping, since every deployed webapp is isolated, and resources are served per webapp. Maybe I didn't understood correctly your statement

Sorry, I'm probably mixing up terminology. Should have said "servlet-mapping portability" I guess.

What I mean is that whether any random servlet functions properly should not be affected by where I happen to locate it in my web server URL space. If I can't just remap a servlet without it breaking then it's not "portable".

E.g., I might have a AdminVaadinServlet that is for admins only. And suppose I've got some proxy/security setup that blocks unauthorized access to anything under https://www.example.com/webapps/private and my Tomcat context path is /webapps.

To get the admins-only protection I might then do this:

<servlet-mapping>
        <servlet-name>AdminVaadinServlet</servlet-name>
        <url-pattern>/private/admin/*</url-pattern>
    </servlet-mapping>

Hope I'm explaining this correctly.

[mcollovati]

@archiecobbs Thanks for clarifying

[Legioth]

The mentioned example with AceEditor seems like a special case since it uses a custom mechanism for loading static resources instead of relying on the mechanisms provided by Vaadin, such as the @JavaScript annotation or Page::addJavaScript. It would be tempting to say that the problem here isn't that Vaadin doesn't take servlet paths into account, but rather that this particular add-on doesn't take servlet paths into account. That line of reasoning would be somewhat lazy from Vaadin's side - we should preferably make it so easy to do the right thing that add-on authors would get it right by default.

Like discussed in https://github.com/vaadin/flow/pull/14341, I'm relatively sure it's not a proper solution to cause a static resource not related to Vaadin (foo.txt) to become available both as /foo.txt and as /demo/foo.txt. We might need to collect a better understanding of how this problem manifests itself so that we can come up with ideas on what a proper solution might look like. Are there other examples of add-ons that fail to load resources with a custom servlet path even after https://github.com/vaadin/flow/issues/13769 was reverted?

[archiecobbs]

Are there other examples of add-ons that fail to load resources with a custom servlet path even after https://github.com/vaadin/flow/issues/13769 was reverted?

I don't know off the top of my head because AceEditor is the only add-on I'm currently using.

It would be tempting to say that the problem here isn't that Vaadin doesn't take servlet paths into account, but rather that this particular add-on doesn't take servlet paths into account.

Understood. I'm also a little fuzzy on who is "at fault" myself here.

But to me this bit is suspect:

VaadinServlet handles static resource requests, if you have mapped it to /*. If not, the servlet container takes care of static resource requests.

Why should VaadinServlet behave differently depending on where it's mapped? As stated earlier, this violates some "principle of portability" (admittedly, I've made up this principle but it seems like it should exist):

What I mean is that whether any random servlet functions properly should not be affected by where I happen to locate it in my web server URL space. If I can't just remap a servlet without it breaking then it's not "portable".

Aside from any principles, it's obviously the case that the "If not, the servlet container takes care of static resource requests" part is not correct, because otherwise we wouldn't have this bug, right?

[mcollovati]

Aside from any principles, it's obviously the case that the "If not, the servlet container takes care of static resource requests" part is not correct, because otherwise we wouldn't have this bug, right?

I would say it works as described. Add-on resources are in META-INF/resources and if you try to access them in your demo application without the servlet mapping they are resolved correctly (e.g. http://localhost:8090/webapps/ace-builds/src-min-noconflict/theme-chrome.js). Also the add-on works if you configure setBaseUrl("../ace-builds/src-min-noconflict/") because the script is loaded by the page at http://localhost:8090/webapps/demo/, so it will point at http://localhost:8090/webapps/demo/../ace-builds/src-min-noconflict/ that is http://localhost:8090/webapps/ace-builds/src-min-noconflict/

[archiecobbs]

I would say it works as described.

Thanks for that explanation. So yes, now I believe you and agree it really is AceEditor's fault...

AceEditor seems to be assuming that StaticFileServer does what I would have wanted it to do, instead of what it actually does.

The problem is that AceEditor shouldn't be assuming anything because it can't know a priori what the right thing to do is. It should instead be using @JavaScript or Page::addJavaScript.

[Legioth]

From another point of view, the "problem" here is that Vaadin is a single-page application framework that uses the browser's History API (aka pushState) to manipulate browser URLs in a way that emulates a multi-page application. To make this easier to deal with, Vaadin uses another browser feature, "base URL", to make it so that relative URLs on the page (e.g. <a href="other-route">) work as expected regardless of whether the path in the current URL is / or /context/servlet/some-route/a-parameter.

This leads to a conflict since URLs for routes are expected to be relative to the servlet path whereas URLs for static resources are expected to be relative to the context path (as long as we refrain from exposing the same static resource on multiple URLs). After lots of deliberation, we decided to set the base URL based on the servlet mapping so that route URLs would be logical, at the expense of static resource URLs. For static resources, there is the virtual context:// protocol that can be used with e.g. @JavaScript to compensate, but this still assumes that Vaadin's resource loading mechanisms are used and that add-on authors also test with a custom servlet mapping.

But to me this bit is suspect:

VaadinServlet handles static resource requests, if you have mapped it to /*. If not, the servlet container takes care of static resource requests.

I think VaadinServlet always does the same - if it receives a request for which it doesn't have anything more specific to serve, then it checks to see if there's a static resource that it could serve for the requested URL. The difference with how it's mapped is thus about whether it receives those requests at all rather than how it would handle them. If there is a static resource at /demo/demo.txt and a Vaadin servlet mapped to /demo/*, then the servlet container would dispatch the request to the Vaadin servlet which would serve the static resource to emulate how it would be served otherwise.

[archiecobbs]

From another point of view, the "problem" here ...

That all makes sense and is clarifying - thanks. It might be worth adding some of that information to the online documentation. It's all a little confusing and this would help people (like me) who are unsure about what is the underlying "theory" for URLs and what is supposed to be happening and how.

I would also include in that a mention of Deployment.computeBaseURI() which is key for anyone needing to "reflect" URLs back to the browser because the URLs being used by the browser are in general unknowable to the server (without asking).