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

Vaadin OSGi applications should not expose relevant classpath content as static resources #50

Closed
mstahv opened this issue Mar 8, 2021 · 5 comments · Fixed by #51
Closed

Comments

@mstahv
Copy link
Member

mstahv commented Mar 8, 2021

In our OSGi examples (https://github.com/vaadin/base-starter-flow-osgi & https://github.com/vaadin/base-starter-flow-karaf) class files and other resources can be accessed from the main bundles. This is unexpected behaviour from the end users point of view and can be considered a bad practise.

We should either change the implementation of our integration code so that generic content from bundle(s) is not served or change the examples so that only minimal and security wise safe content is exposed.

If we only change our examples, it should be documented that all resources (including class files) from the bundle that is registering the servlet are exposed.

@denis-anisimov
Copy link

denis-anisimov commented Mar 9, 2021

This is partially an issue in overall OSGi HTTP whiteboard implementation (not sure about specification) and Vaadin:

  • all the resources located inside the bundle/jar are available via a ServletContext for the Servlet registered in the bundle. This is behavior of HTTP whiteboard.
  • any resource available in the Servlet context is exposed via HTTP by the VaadinServlet (namely StaticFileServer).

Vaadin behavior is the same regardless servlet container. But the first behavior is quite specific for OSGi: one has to do an explicit action/configuration to include resources available via ServletContext.

Unfortunately there is no easy way to fix this: if we disable serving resources available via ServletContext by the static file server many existing WAR applications will be broken since they rely on this existing functionality.
But this is what should be done for the OSGi case: prevent serving resources available via ServletContext (at least partially).

In fact there is already existing solution for this (even though it's not obvious that it's always "required" with Vaadin): a custom ServletContextHelper.
A custom ServletContextHelper may be registered as a service and it may decide which resources should be in the ServletContext and which should not be. Then the servlet should be registered using a specific property to select the specific context.

So there is already a solution for this but it requires additional class and complicates the servlet registration: every servlet should be registered using some ServletContextHelper (and we can't do this automatically since we don't know which context path to use).

On the other hand it's quite easy to expose any classpath resource via Resource service, so there is no need to expose automatically everything available via ServletContext by the VaadinServlet.
So I think we can disable in OSGi case exposing resources from the ServletContext.

@denis-anisimov
Copy link

denis-anisimov commented Mar 9, 2021

you have to explicit register an Service that uses special service properties.

I agree with you.

But by sentence says totally different thing.
It says that the resources in the Jar becomes available as a resource in the ServletContext which means if you use method ServletContext::getResource(String) then it will return a URL for any resource inside bundle/jar (except ....).
And yes, to be able to access to the resource via WEB one has to register a resource service.
But one thing is access to the resource via WEB and another thing is access to the resource via ServletContext.

denis-anisimov pushed a commit to vaadin/flow that referenced this issue Mar 10, 2021
refactor: use StaticFileHandler as a service

Part of vaadin/osgi#50
denis-anisimov pushed a commit to vaadin/flow that referenced this issue Mar 10, 2021
refactor: use StaticFileHandler as a service

Part of vaadin/osgi#50
denis-anisimov pushed a commit to vaadin/flow that referenced this issue Mar 10, 2021
* refactor: use StaticFileHandler as a service (#10229)

Part of vaadin/osgi#50
Java Framework bugs & maintenance (Vaadin 10+) automation moved this from WIP to Closed Mar 10, 2021
denis-anisimov pushed a commit that referenced this issue Mar 10, 2021
* fix: implement static file handler factory for OSGi

fixes #50
denis-anisimov pushed a commit to vaadin/vaadin-flow-karaf-example that referenced this issue Mar 11, 2021
ServletContext resources are not available anymore via HTTP after
vaadin/osgi#50. So they should be exposed explicitly via resource
service
@pleku
Copy link
Contributor

pleku commented Mar 19, 2021

all the resources located inside the bundle/jar (except specific paths like META-INF ) are available via a ServletContext for the Servlet registered in the bundle. This is behavior of HTTP whiteboard.

@denis-anisimov it seems that the META-INF/MANIFEST.MD can be accessed via the browser, so could you clarify what you meant with the except specific paths like META-INF part ?

@denis-anisimov
Copy link

The sentence targets the resources availability via ServletContext and META-INF part was just a note.
I've tried to access to some resource inside META-INF and wasn't able to access.

@pleku
Copy link
Contributor

pleku commented Mar 22, 2021

Ok. Both me and Artem were able to access the manifest file. I'll edit the comment so nobody has any misunderstanding.

mstahv pushed a commit to vaadin/vaadin-flow-karaf-example that referenced this issue Apr 12, 2021
ServletContext resources are not available anymore via HTTP after
vaadin/osgi#50. So they should be exposed explicitly via resource
service
denis-anisimov pushed a commit to vaadin/vaadin-flow-karaf-example that referenced this issue Apr 12, 2021
ServletContext resources are not available anymore via HTTP after
vaadin/osgi#50. So they should be exposed explicitly via resource
service

Co-authored-by: Denis Anisimov <denis@vaadin.com>
mstahv added a commit to vaadin/vaadin-flow-karaf-example that referenced this issue May 5, 2021
* A new default branch with a stable version

* Update repository tag in pom (#16)

* fixed gitignore file for the project (#22)

Co-authored-by: Matti Tahvonen <matti@vaadin.com>

* version update (#23)

* Update Vaadin to 19.0.4 (#30)

* Fix regression with static resources (#31)

* Revert "Fix regression with static resources (#31)" (#32)

This reverts commit 93b9647.

* chore: expose static resources explicitly (#34)

ServletContext resources are not available anymore via HTTP after
vaadin/osgi#50. So they should be exposed explicitly via resource
service

Co-authored-by: Denis Anisimov <denis@vaadin.com>

* Add issue template (#29) (#38)

* Update Vaadin to 19.0.5 (#39)

* Update to use Java 11

* Separate module for greetservice api

* Update to use Java 11

* Avoid issues with Java 11 modules

* Proper export package hint for bdn

* Make the project use Java 11 (#41)

* Update to use Java 11

* Avoid issues with Java 11 modules

* Validation build for the project for all PRs (#42)

* Update to use Java 11

* Avoid issues with Java 11 modules

* Added a GitHub actions based test that build passes

* Added a missing pom.xml

Co-authored-by: Zhe Sun <31067185+ZheSun88@users.noreply.github.com>
Co-authored-by: Vaadin Bot <vaadin-bot@users.noreply.github.com>
Co-authored-by: Denis <denis@vaadin.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

4 participants