flow icon indicating copy to clipboard operation
flow copied to clipboard

Vaadin23: index.html rewritten causing vaadin-bundle-xxx.cache.js URL to be 404 not found

Open archiecobbs opened this issue 3 years ago • 9 comments

Description of the bug

The situation is described in this stackoverflow.com question:

I'm working on a Vaadin application running under tomcat in a WAR file.

I'm trying to simply upgrade it from Vaadin 22 to Vaadin 23.

It was loading reliably with Vaadin 22 but now it fails to load with Vaadin 23.

The WAR file is named pcom.war and within it web.xml contains:

<servlet-mapping>
    <servlet-name>PCOM</servlet-name>
    <url-pattern>/pcom/*</url-pattern>
</servlet-mapping>

Therefore, the URL I trying to load the web application from, and from which Vaadin is failing to load successfully, is: http://localhost:8080/pcom/pcom/.

The index.html generated by Vaadin in META-INF/VAADIN/webapp/index.html in pcom.war contains (line breaks added for clarity):

<!doctype html>
<html lang="en">
<head>
<meta charset="UTF-8"/>
<meta name="viewport" content="width=device-width,initial-scale=1"/>
<style>body, #outlet {
      height: 100vh;
      width: 100%;
      margin: 0;
    }</style>
<script defer="defer" src="VAADIN/build/vaadin-bundle-67fde5fb08b0f134e867.cache.js"></script>
</head>
<body>
<div id="outlet"></div>
</body>
</html>

Note the src="VAADIN/build/vaadin-bundle-... relative URL. This is correct, relative to index.html as it sits within the META-INF contents of pcom.war:

        0  07-29-2022 14:33   META-INF/
       79  07-29-2022 14:33   META-INF/MANIFEST.MF
        0  07-29-2022 14:33   META-INF/VAADIN/
        0  07-29-2022 14:33   META-INF/VAADIN/config/
     1844  07-29-2022 14:33   META-INF/VAADIN/config/flow-build-info.json
     1658  07-29-2022 14:33   META-INF/VAADIN/config/stats.json
        0  07-29-2022 14:33   META-INF/VAADIN/webapp/
        0  07-29-2022 14:33   META-INF/VAADIN/webapp/VAADIN/
        0  07-29-2022 14:33   META-INF/VAADIN/webapp/VAADIN/build/
  1051904  07-29-2022 14:33   META-INF/VAADIN/webapp/VAADIN/build/vaadin-1-cfc74d1f13c51748c7ba.cache.js
   259593  07-29-2022 14:33   META-INF/VAADIN/webapp/VAADIN/build/vaadin-1-cfc74d1f13c51748c7ba.cache.js.gz
   607425  07-29-2022 14:33   META-INF/VAADIN/webapp/VAADIN/build/vaadin-2-737c0b4498b03051df61.cache.js
   158040  07-29-2022 14:33   META-INF/VAADIN/webapp/VAADIN/build/vaadin-2-737c0b4498b03051df61.cache.js.gz
    67025  07-29-2022 14:33   META-INF/VAADIN/webapp/VAADIN/build/vaadin-3-460383d2eeb67dbda1eb.cache.js
    23477  07-29-2022 14:33   META-INF/VAADIN/webapp/VAADIN/build/vaadin-3-460383d2eeb67dbda1eb.cache.js.gz
    27991  07-29-2022 14:33   META-INF/VAADIN/webapp/VAADIN/build/vaadin-4-9ba06ea1fd5a6da385f4.cache.js
     8040  07-29-2022 14:33   META-INF/VAADIN/webapp/VAADIN/build/vaadin-4-9ba06ea1fd5a6da385f4.cache.js.gz
     3518  07-29-2022 14:33   META-INF/VAADIN/webapp/VAADIN/build/vaadin-5-a21e874ef30c99e3575f.cache.js
     1480  07-29-2022 14:33   META-INF/VAADIN/webapp/VAADIN/build/vaadin-5-a21e874ef30c99e3575f.cache.js.gz
   138893  07-29-2022 14:33   META-INF/VAADIN/webapp/VAADIN/build/vaadin-6-5710b84bf57453285225.cache.js
    46654  07-29-2022 14:33   META-INF/VAADIN/webapp/VAADIN/build/vaadin-6-5710b84bf57453285225.cache.js.gz
    86955  07-29-2022 14:33   META-INF/VAADIN/webapp/VAADIN/build/vaadin-bundle-67fde5fb08b0f134e867.cache.js
    26514  07-29-2022 14:33   META-INF/VAADIN/webapp/VAADIN/build/vaadin-bundle-67fde5fb08b0f134e867.cache.js.gz
      365  07-29-2022 14:33   META-INF/VAADIN/webapp/index.html
      273  07-29-2022 14:33   META-INF/VAADIN/webapp/index.html.gz

But when I load the application, FireFox inspector shows this error:

enter image description here

Note the reported error loading http://localhost:8080/pcom/VAADIN/build/vaadin-bundle-67fde5fb08b0f134e867.cache.js.

That URL is incorrect and generates a 404 error - I can verify this using curl(1).

However I can use curl(1) to successfully load the correct URL, which is http://localhost:8080/pcom/pcom/VAADIN/build/vaadin-bundle-67fde5fb08b0f134e867.cache.js.

Why does Vaadin 23 rewrite the URL in the index.html file so as to make it unloadable??

Expected behavior

Application should load.

Minimal reproducible example

None available yet. See description above for the basic setup.

Versions

  • Vaadin / Flow version: 23.1.3
  • Java version: 11.0.16
  • OS version: Mac OS X 12.5
  • Browser version (if applicable): Firefox 103.0
  • Application Server (if applicable): Tomcat 9.0.65
  • IDE (if applicable): vim

you can copy this from the debug window

(How do I show this?)

archiecobbs avatar Jul 29 '22 20:07 archiecobbs

Could be related to https://github.com/vaadin/flow/issues/13769

knoobie avatar Jul 30 '22 10:07 knoobie

Minimal reproducible example

OK I've created this.

$ git clone --branch issue-14239 [email protected]:archiecobbs/vaadin-flow-bugs.git
$ cd vaadin-flow-bugs/
$ mvn

Then try to load http://localhost:8090/demo/demo/ in your browser.

archiecobbs avatar Jul 31 '22 16:07 archiecobbs

Hi, thanks for reporting. Since Vaadin 23.1, when using a custom url mapping for Vaadin Servlet in a non Spring project, it is needed to add a mapping also for /VAADIN/* path. This was mentioned on the release notes, but unfortunately it seems that the documentation is not up to date. https://github.com/vaadin/flow/releases/tag/23.1.0

Documentation should be updated to explain how to setup such a scenario

mcollovati avatar Aug 01 '22 10:08 mcollovati

Static resources are always served from /VAADIN in the context path.

Well this explains the problem, but yuck - isn't that a layering violation? A WAR file is no longer self-contained.

For example, you can't run two independent Vaadin applications in the same instance of tomcat (because they might be running different versions of Vaadin).

Personally I don't ever do that but it seems like you wouldn't want to destroy this capability.

Quoting #13769:

In most cases, you have only one servlet deployed as /* in your application.

That observation may be true .. but it doesn't follow that we should only care about "most cases" and damn the others!

This is typical Vaadin behavior...

  • Assume everybody does things the way we do it (Spring Boot, Spring Security, etc.)
  • Only document how to do things our way
  • Include a bunch of hidden magic that breaks if you try to do things any other way

archiecobbs avatar Aug 01 '22 15:08 archiecobbs

Well this explains the problem, but yuck - isn't that a layering violation? A WAR file is no longer self-contained. For example, you can't run two independent Vaadin applications in the same instance of tomcat (because they might be running different versions of Vaadin).

/VAADIN/* mapping is per context, so it won't interfere with other deployed WAR

mcollovati avatar Aug 02 '22 05:08 mcollovati

The solution in the release notes does not work for OSGi environments. When defining the servlet using osgi.http.whiteboard.servlet.pattern=/myapp/*, it is just not possible to add another URL pattern.

Currently, the only way seems to be to define an additional resourse servlet and directly reference the folder with the vaadin-bundle-...cache.js like following:

"osgi.http.whiteboard.resource.pattern=/VAADIN/*", "osgi.http.whiteboard.resource.prefix=/META-INF/VAADIN/webapp/VAADIN"

To me, that looks more like a hack than a solution. It would be better if the Java Script file would be loaded from where it is served by default, which worked like that before 23.1. Even now, it is still reachable under /myapp/VAADIN//build/vaadin-bundle-...cache.js, but it is just no longer loaded from there.

Furthermore, if there is a proxy with access control filtering, it is now mandatory to also allow the sub path /VAADIN/build. When only access to /myapp is allowed, the Vaadin application doesn't work any longer since 23.1. So, after /VAADIN/push, another sub path to be opened (and another discussion with the cyber security expert why this is necessary).

Whichever the final solution will be, it should at least be documented.

jochenwalz avatar Aug 02 '22 05:08 jochenwalz

Reopening bug because it sounds like it's worth reconsidering this behavior.

archiecobbs avatar Aug 02 '22 13:08 archiecobbs

This is actually worse than I thought.

This bug also seems to be causing failure to load add-ons.

I'm using this:

        <dependency>
            <groupId>de.f0rce</groupId>
            <artifactId>ace</artifactId>
            <version>3.3.3</version>
        </dependency>

And the syntax highlighting is not working.

On the console I see these errors:

Screen Shot 2022-08-11 at 1 55 50 PM

I've updated the demonstration project at https://github.com/archiecobbs/vaadin-flow-bugs/tree/issue-14239 to show this as well, but note you have to edit src/main/webapp/WEB-INF/web.xml to apply the first workaround to get the app to load. Once you do that you can reproduce the problem described in this comment.

archiecobbs avatar Aug 11 '22 18:08 archiecobbs

I just realized this whole problem is trivial to fix.

The basic problem is that before we serve resources from the classpath, we have to strip off request.getServletPath() from the path we're looking for.

In the case there is no servlet path, then request.getServletPath() returns empty string, so that makes this easy:

diff --git a/flow-server/src/main/java/com/vaadin/flow/server/StaticFileServer.java b/flow-server/src/main/java/com/vaadin/flow/server/StaticFileServer.java
index 0ebe2cb4ad..17d32a3178 100644
--- a/flow-server/src/main/java/com/vaadin/flow/server/StaticFileServer.java
+++ b/flow-server/src/main/java/com/vaadin/flow/server/StaticFileServer.java
@@ -259,6 +259,9 @@ public class StaticFileServer implements StaticFileHandler {
                             + filenameWithPath.replaceFirst("^/", ""));
         }
 
+        // Strip off servlet path (if any) from file path so it corresponds to our resources as we know them
+        filenameWithPath = filenameWithPath.substring(request.getServletPath().length());
+
         if (resourceUrl == null) {
             resourceUrl = getStaticResource(filenameWithPath);
         }

I've tested this successfully all four of the combinations below:

Non-empty servlet context, non-empty servlet path

  • Jetty is configured with:
<webApp>
    <contextPath>/mywar</contextPath>
</webApp>
  • web.xml is configured with:
<servlet-mapping>
    <servlet-name>VaadinBugServlet</servlet-name>
    <url-pattern>/demo/*</url-pattern>
</servlet-mapping>
  • Servlet is loaded from http://localhost:8090/mywar/demo/

Empty servlet context, non-empty servlet path

  • Jetty is configured with no context path
  • web.xml is configured with:
<servlet-mapping>
    <servlet-name>VaadinBugServlet</servlet-name>
    <url-pattern>/demo/*</url-pattern>
</servlet-mapping>
  • Servlet is loaded from http://localhost:8090/demo/

Non-empty servlet context, Empty servlet path

  • Jetty is configured with:
<webApp>
    <contextPath>/mywar</contextPath>
</webApp>
  • web.xml is configured with:
<servlet-mapping>
    <servlet-name>VaadinBugServlet</servlet-name>
    <url-pattern>/*</url-pattern>
</servlet-mapping>
  • Servlet is loaded from http://localhost:8090/mywar/

Empty servlet context, empty servlet path

  • Jetty is configured with no context path
  • web.xml is configured with:
<servlet-mapping>
    <servlet-name>VaadinBugServlet</servlet-name>
    <url-pattern>/*</url-pattern>
</servlet-mapping>
  • Servlet is loaded from http://localhost:8090/

archiecobbs avatar Aug 12 '22 19:08 archiecobbs

We decided to revert the change for serving static resources from context root https://github.com/vaadin/flow/pull/14357 and fix Vite-related issue https://github.com/vaadin/flow/issues/13190 in another way.

mshabarov avatar Aug 16 '22 10:08 mshabarov

This ticket/PR has been released with Vaadin 23.2.0.beta3 and is also targeting the upcoming stable 23.2.0 version.

vaadin-bot avatar Aug 26 '22 09:08 vaadin-bot