Skip to content
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

Always serve static files from VAADIN in the context root #13769

Closed
Artur- opened this issue May 13, 2022 · 4 comments · Fixed by #13795
Closed

Always serve static files from VAADIN in the context root #13769

Artur- opened this issue May 13, 2022 · 4 comments · Fixed by #13795

Comments

@Artur-
Copy link
Member

Artur- commented May 13, 2022

Description of the bug

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

In cases when you have other web frameworks in use, REST endpoints etc you might map your application to /myapp/* instead.

In the rare cases where you have multiple Vaadin servlets in the same application, you might have /myapp/* and /admin/* mappings.

Right now the bundle files are loaded from

  • /VAADIN in the /* mapping case
  • /myapp/VAADIN in the /myapp/* mapping case
  • /myapp/VAADIN AND /admin/VAADIN in the multi mapping case

Expected behavior

In all cases, the static files (JS bundle files, vaadinPush script etc) should be loaded from VAADIN inside the context root. Thus is you have multiple servlet mappings, you still load e.g. web components only once from /VAADIN and not from both /myapp/VAADIN and /admin/VAADIN.

Any dynamic content related to the servlet should still be served through /myapp/VAADIN/dynamic.

Minimal reproducible example

Versions

Vaadin: 23.0.9
Flow: 23.0.7
Java: Homebrew 17.0.1
OS: aarch64 Mac OS X 12.3.1
Browser: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/101.0.4951.54 Safari/537.36

@Artur- Artur- self-assigned this May 13, 2022
Artur- added a commit that referenced this issue May 18, 2022
Artur- added a commit that referenced this issue May 18, 2022
Artur- added a commit that referenced this issue May 18, 2022
Artur- added a commit that referenced this issue May 19, 2022
Artur- added a commit that referenced this issue May 20, 2022
Artur- added a commit that referenced this issue May 20, 2022
mshabarov pushed a commit that referenced this issue May 23, 2022
mshabarov added a commit that referenced this issue May 24, 2022
Add a /VAADIN/* servlet mapping for OSGi test servlet, since it's mandatory for non-root mappings.

Part-of #13769
vaadin-bot pushed a commit that referenced this issue May 24, 2022
Add a /VAADIN/* servlet mapping for OSGi test servlet, since it's mandatory for non-root mappings.

Part-of #13769
mshabarov added a commit that referenced this issue May 24, 2022
Add a /VAADIN/* servlet mapping for OSGi test servlet, since it's mandatory for non-root mappings.

Part-of #13769

Co-authored-by: Mikhail Shabarov <61410877+mshabarov@users.noreply.github.com>
@vaadin-bot
Copy link
Collaborator

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

@knoobie
Copy link
Contributor

knoobie commented Jul 1, 2022

@Artur- Just for clarification, which is probably important for a lot of people:

If I deploy a spring boot application with a configuration like this:

server:
  servlet:
    context-path: /web

Vaadin's static ressources are still served with /web/VAADIN, right?

@Artur-
Copy link
Member Author

Artur- commented Jul 1, 2022

Yes, the only affect this change should have had is if you map your servlet with a separate path inside the context path. Then earlier it was using /contextpath/servletpath/VAADIN for static files and now it is using /contextpath/VAADIN

@archiecobbs
Copy link
Contributor

Yes, the only affect this change should have had is if you map your servlet with a separate path inside the context path. Then earlier it was using /contextpath/servletpath/VAADIN for static files and now it is using /contextpath/VAADIN

Um, no.

When a non-empty servlet path is in use, this change:

  • Breaks almost all Vaadin add-ons
  • Creates a security issue

Pleas see #14341 for details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants