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

Different behaviour of DefaultServlet in Jetty 12 #10296

Closed
rakeshk15 opened this issue Aug 11, 2023 · 7 comments
Closed

Different behaviour of DefaultServlet in Jetty 12 #10296

rakeshk15 opened this issue Aug 11, 2023 · 7 comments
Assignees
Labels
Bug For general bugs on Jetty side
Milestone

Comments

@rakeshk15
Copy link

rakeshk15 commented Aug 11, 2023

Jetty version(s)
Jetty 12.0.0

Java version/vendor
Java 17

OS type/version
macOS Ventura 13.5

Description
I was using Jetty 11.0.15 in embedded mode and mounted the DefaultServlet at /static/* with resource base to a directory webapp in one of the jar at the classpath

So the full base path becomes jar:file:/<jar-full-path>!/webapp

And this webapp directory contains the static folder which has the images, css, js etc.

image

When a static resources such as http://localhost:8080/static/img/logo.png is requested by browser then it results in a 404.

The same path is successfully served by Jetty 11

It appears that the servlet path was taken into account in Jetty 11 while looking for the resource which is appended to the resource base path.

To circumvent this I had to change the resource base path to /webapp/static and now Jetty 12 is also serving the resources.

Can someone please tell why there is a deviation in logic from Jetty 11 to 12?

@rakeshk15 rakeshk15 added the Bug For general bugs on Jetty side label Aug 11, 2023
@rakeshk15 rakeshk15 changed the title DefaultServlet inconsistent behaviour in Jetty 12 Different behaviour of DefaultServlet in Jetty 12 Aug 11, 2023
@janbartel
Copy link
Contributor

@rakeshk15 can you please report:

  • which ee version you are deploying into on jetty-12: is it ee9 or ee10?
  • do you have pathInfoOnly set on your DefaultServlet configuration?
  • the code where you configure the DefaultServlet, including the resourceBase configuration?

@janbartel janbartel self-assigned this Aug 11, 2023
@gregw
Copy link
Contributor

gregw commented Aug 11, 2023

In Jetty <12 the DefaultServlet has a pathInfoOnly to allow only the pathInfo to be used to find a resource, so if the servlet is mapped to /static/* then a request to /static/foo would result in a resource lookup of resourceBase + "/foo".
We found that there were very few uses the DefaultServlet that were not mapped to "/" that didn't have the pathInfoOnly option set. It was also very complex to deal efficiently with all the variants of that setting when doing welcome files and welcome servlets and welcome files that are actually JSP servlets.

So we ended up going for a much simpler solution without an optional parameter that uses the pathInfoOnly if the servlet is not mapped to the default path. This is now implemented in the new method protected String getEncodedPathInContext(HttpServletRequest req, String includedServletPath), so if you want something different you can extend the DefaultServlet and implement whatever mapper you want in this method.

@rakeshk15
Copy link
Author

thanks @gregw for the explanation, I'll check the method you mentioned above.

@janbartel here is the information you asked.

  • which ee version you are deploying into on jetty-12: is it ee9 or ee10? - ee10
  • do you have pathInfoOnly set on your DefaultServlet configuration? - no, i did not set that
  • the code where you configure the DefaultServlet, including the resourceBase configuration? - please see these GH links

Jetty 12

https://github.com/AdeptJ/adeptj-runtime/blob/92e5b8842a715a9e9ccbe0d8ecb1a4f669501363/adapters/jetty/src/main/java/com/adeptj/runtime/jetty/JettyServer.java#L129C26-L129C26

https://github.com/AdeptJ/adeptj-runtime/blob/92e5b8842a715a9e9ccbe0d8ecb1a4f669501363/adapters/jetty/src/main/resources/reference.conf#L45

Jetty 11

https://github.com/AdeptJ/adeptj-runtime/blob/8ba4192a9a950b924ae3b83e34e51a231f384e9c/adapters/jetty/src/main/java/com/adeptj/runtime/jetty/JettyServer.java#L136

https://github.com/AdeptJ/adeptj-runtime/blob/8ba4192a9a950b924ae3b83e34e51a231f384e9c/adapters/jetty/src/main/resources/reference.conf#L45

@grgrzybek
Copy link
Contributor

See some discussion here: #9910

@joakime joakime added this to the 12.0.x milestone Sep 20, 2023
@joakime
Copy link
Contributor

joakime commented Sep 20, 2023

@rakeshk15 do you allow users to add arbitrary servlets and filters to your ServletContextHandler?
Potentially even ones from other common libraries that support Servlets?

@rakeshk15
Copy link
Author

@joakime only the servlets and filters from the adeptj-runtime are considered, since these are already part of the adeptj-runtime project so it is safe to add to the Jetty ServletContextHandler.

@janbartel
Copy link
Contributor

@rakeshk15 as @gregw said, the behaviour changed a bit in jetty-12 due to the complexities of the DefaultServlet and the way people are mapping it. Your options are to override the getEncodedPathInContext() method, or the isDefaultMapping() method. Or, if you've only got a single DefaultServlet then you could name it default and you won't have to override anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
No open projects
Status: ✅ Done
Development

No branches or pull requests

5 participants