jetty / jetty.project

Eclipse Jetty® - Web Container & Clients - supports HTTP/2, HTTP/1.1, HTTP/1.0, websocket, servlets, and more
https://eclipse.dev/jetty
Other
3.86k stars 1.91k forks source link

Different behaviour of DefaultServlet in Jetty 12 #10296

Closed rakeshk15 closed 1 year ago

rakeshk15 commented 1 year ago

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?

janbartel commented 1 year ago

@rakeshk15 can you please report:

gregw commented 1 year ago

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 commented 1 year ago

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

@janbartel here is the information you asked.

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 commented 1 year ago

See some discussion here: #9910

joakime commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

@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.