search

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

Only use ServletContainerInitializers from server path for web.xml < 3.0 #1466

Closed janbartel closed 7 years ago

janbartel commented 7 years ago

ServletContainerInitializers are currently loaded from both the server path and the webapp's classpath. If a webapp includes a jar that happens to contain a ServletContainerInitializer, it will be applied, even if the webapp itself has a web.xml version < 3.0. This might be considered unexpected behaviour. We could apply ServletContainerInitializers only from the server's classpath in the case where web.xml < 3.0.

joakime commented 7 years ago

Since this would be a Server path only fix. Perhaps we could have an annotation @ServletSupportLevel(3.0) that can help trigger these in appropriate places (such as Servlet 2.5 and JSR356 websockets).

janbartel commented 7 years ago

Not sure I follow @joakime. The point here would be to not do any annotation processing at all, if web.xml <= 2.5.

joakime commented 7 years ago

Of the 2 SCIs in Websocket, the JSR356 one should never execute (even with a blank class Set) on a webapp declared as Servlet 2.5.

But the other one (native Jetty Websocket) is fine on Servlet 2.5.

joakime commented 7 years ago

Are you proposing that all SCI's on the Server side do not run when the webapp is Servlet 2.5?

janbartel commented 7 years ago

@joakime, no I'm proposing that ONLY SCIs that are on the server path run when web.xml < 3.

joakime commented 7 years ago

Perhaps only run server SCIs that have no @HandlesTypes present?

janbartel commented 7 years ago

@joakime I think that if the SCIs are present on the server side then they've a) been selected for use by the user and b) necessary for the server to deliver services, so I would run all server SCIs, and if they happen to have an @HandlesTypes, then so be it.

Where are the SCIs for websocket, on the server path or on the webapp path?

joakime commented 7 years ago

There's 2, both on the server side.

/javax-websocket-server-impl - WebSocketServerContainerInitializer.java

JSR356 was introduced as a part of JEE7 / Servlet 3.1, this one cannot function on older Servlet specs and APIs, it should never be executed for Servlet 2.4/2.5/3.0 webapps.

/websocket-server - NativeWebSocketServletContainerInitializer.java

This one should work find on Servlet 2.4+

janbartel commented 7 years ago

Change committed. Users who had previously never updated their web.xml past version 2.5 but had newer 3.1 jars with SCIs etc in WEB-INF/lib will need to either:

  1. update the web.xml or
  2. use WebAppContext.setConfigurationDiscovered(true)
jmcc0nn3ll commented 7 years ago

@WalkerWatch should make sure there is a NOTE on this in the docs under deployment somewhere, probably near a metadata-complete=true comment

ThrawnCA commented 5 years ago

@jmcc0nn3ll Yeah, I don't think that this ever reached the docs; if it did, I couldn't find it. We're trying to update from Jetty 9.2 to 9.4 and were getting 404s on everything because our web.xml still declared webapp version 2.4.