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.8k stars 1.9k forks source link

MetaInfConfiguration scanning of TLDs should follow spec #10899

Closed joakime closed 8 months ago

joakime commented 8 months ago

Jetty version(s) 12.0.3

Jetty Environment All environments

Java version/vendor (use: java -version) All

OS type/version All

Description As pointed out in comment https://github.com/jetty/jetty.project/pull/10889#discussion_r1393121857

We are scanning for TLDs incorrectly. We should also be using the Resource API, not Path + Walk (if possible).

For Webapps themselves.

https://github.com/jakartaee/pages/blob/master/spec/src/main/asciidoc/ServerPages.adoc#731-identifying-tag-library-descriptors

It is a recursive scan for anything that ends in .tld (case insensitive) from root of webapp resource.

For JAR files (in webapp classpath).

https://github.com/jakartaee/pages/blob/master/spec/src/main/asciidoc/ServerPages.adoc#721-packaged-tag-libraries

It is a recursive scan for anything that ends in .tld (case insensitive) from the /META-INF/ directory in the JAR file. No exclusions or ignored rules for JAR files.

joakime commented 8 months ago

The relevant section in the Tomcat Jasper implementation that follows this spec too.

https://github.com/apache/tomcat/blob/10.1.16/java/org/apache/jasper/servlet/TldScanner.java#L220-L254

Our use of org.eclipse.jetty.ee10.apache.jsp.JettyTldPreScanned subverts this logic in Tomcat Jasper.

janbartel commented 8 months ago

I think the current behaviour in jetty is correct. We are only doing the scanning of container and WEB-INF/lib jars to find .tld files and supplying them to Apache Jasper. Apache Jasper is still taking care of .tlds in the other locations you mentioned.

Both Jetty and Apache Jasper when looking for .tlds inside a jar will only consider .tlds that are inside META-INF, as the spec requires:

Tag library descriptor files have names that use the extension .tld, and the extension indicates a tag library descriptor file. When deployed inside a JAR file, the tag library descriptor files must be in the META-INF directory, or a subdirectory of it.

joakime commented 8 months ago

I think the current behaviour in jetty is correct. We are only doing the scanning of container and WEB-INF/lib jars to find .tld files and supplying them to Apache Jasper. Apache Jasper is still taking care of .tlds in the other locations you mentioned.

... and WebAppContext.extraClasspath JARs (and directories)?

janbartel commented 8 months ago

Yep, we handle those too. MetaInfConfiguration.findJars() method is where we collect those jars, findAndFilterWebAppPaths is where we add them to the context's MetaData.webInfResources, then we iterate over those when scanning.

janbartel commented 8 months ago

I believe we're following the spec re scanning for tlds. If you can find an omission then please reopen.