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

MetaInfConfiguration scanning of TLDs should follow spec #10899

Closed
joakime opened this issue Nov 15, 2023 · 5 comments
Closed

MetaInfConfiguration scanning of TLDs should follow spec #10899

joakime opened this issue Nov 15, 2023 · 5 comments
Labels
Bug For general bugs on Jetty side Specification For all industry Specifications (IETF / Servlet / etc)

Comments

@joakime
Copy link
Contributor

joakime commented Nov 15, 2023

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 #10889 (comment)

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.

  • ignore if found in /WEB-INF/classes/
  • ignore if found in /WEB-INF/lib/
  • ignore if ends in / (ie: it's a directory called foo.tld/)
  • ignore if found in /WEB-INF/tags/ (with exception for implicit.tld)
  • all other hits are valid in the webapp.

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 joakime added Bug For general bugs on Jetty side Specification For all industry Specifications (IETF / Servlet / etc) labels Nov 15, 2023
@joakime
Copy link
Contributor Author

joakime commented Nov 15, 2023

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
Copy link
Contributor

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
Copy link
Contributor Author

joakime commented Nov 15, 2023

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
Copy link
Contributor

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
Copy link
Contributor

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

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 Specification For all industry Specifications (IETF / Servlet / etc)
Projects
No open projects
Status: ✅ Done
Development

No branches or pull requests

2 participants