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

getResourcePaths fails when a META-INF resource has reserved characters in its filename #9972

Closed
wilkinsona opened this issue Jun 26, 2023 · 8 comments · Fixed by #9974
Closed
Assignees
Labels
Bug For general bugs on Jetty side

Comments

@wilkinsona
Copy link
Contributor

wilkinsona commented Jun 26, 2023

Jetty version(s)
12.0.0.beta2

Java version/vendor (use: java -version)

openjdk version "17.0.7" 2023-04-18 LTS
OpenJDK Runtime Environment (build 17.0.7+7-LTS)
OpenJDK 64-Bit Server VM (build 17.0.7+7-LTS, mixed mode, sharing)

OS type/version

macOS 13.4

Description

I'm in the process of upgrading Spring Boot to Jetty 12 and have encountered a problem that's similar to #4033 and #7160. The setup is similar too. There's a jar file that contains a resource named META-INF/resources/nested-reserved-!#\\$%&()*+,:=?@[]-meta-inf-resource.txt and a servlet that's used for testing:

private static final class GetResourcePathsServlet extends HttpServlet {

	@Override
	protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
		collectResourcePaths("/").forEach(resp.getWriter()::println);
		resp.getWriter().flush();
	}

	private Set<String> collectResourcePaths(String path) {
		Set<String> allResourcePaths = new LinkedHashSet<>();
		Set<String> pathsForPath = getServletContext().getResourcePaths(path);
		if (pathsForPath != null) {
			for (String resourcePath : pathsForPath) {
				allResourcePaths.add(resourcePath);
				allResourcePaths.addAll(collectResourcePaths(resourcePath));
			}
		}
		return allResourcePaths;
	}

}

A request to this servlet fails:

java.lang.NumberFormatException: !hex &
	at org.eclipse.jetty.util.TypeUtil.convertHexDigit(TypeUtil.java:467) ~[jetty-util-12.0.0.beta2.jar:12.0.0.beta2]
	at org.eclipse.jetty.util.URIUtil.canonicalPath(URIUtil.java:729) ~[jetty-util-12.0.0.beta2.jar:12.0.0.beta2]
	at org.eclipse.jetty.util.URIUtil.canonicalPath(URIUtil.java:667) ~[jetty-util-12.0.0.beta2.jar:12.0.0.beta2]
	at org.eclipse.jetty.ee10.servlet.ServletContextHandler$ServletContextApi.getResourcePaths(ServletContextHandler.java:2827) ~[jetty-ee10-servlet-12.0.0.beta2.jar:12.0.0.beta2]
	at com.example.ResourceHandlingApplication$GetResourcePathsServlet.collectResourcePaths(ResourceHandlingApplication.java:81) ~[classes/:na]
	at com.example.ResourceHandlingApplication$GetResourcePathsServlet.collectResourcePaths(ResourceHandlingApplication.java:85) ~[classes/:na]
	at com.example.ResourceHandlingApplication$GetResourcePathsServlet.doGet(ResourceHandlingApplication.java:75) ~[classes/:na]
	at jakarta.servlet.http.HttpServlet.service(HttpServlet.java:527) ~[jakarta.servlet-api-6.0.0.jar:6.0.0]
	at jakarta.servlet.http.HttpServlet.service(HttpServlet.java:614) ~[jakarta.servlet-api-6.0.0.jar:6.0.0]
	at org.eclipse.jetty.ee10.servlet.ServletHolder.handle(ServletHolder.java:739) ~[jetty-ee10-servlet-12.0.0.beta2.jar:12.0.0.beta2]
	at org.eclipse.jetty.ee10.servlet.ServletHandler$ChainEnd.doFilter(ServletHandler.java:1601) ~[jetty-ee10-servlet-12.0.0.beta2.jar:12.0.0.beta2]
	at org.springframework.web.filter.CharacterEncodingFilter.doFilterInternal(CharacterEncodingFilter.java:201) ~[spring-web-6.1.0-M1.jar:6.1.0-M1]
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:116) ~[spring-web-6.1.0-M1.jar:6.1.0-M1]
	at org.eclipse.jetty.ee10.servlet.FilterHolder.doFilter(FilterHolder.java:205) ~[jetty-ee10-servlet-12.0.0.beta2.jar:12.0.0.beta2]
	at org.eclipse.jetty.ee10.servlet.ServletHandler$Chain.doFilter(ServletHandler.java:1573) ~[jetty-ee10-servlet-12.0.0.beta2.jar:12.0.0.beta2]
	at org.eclipse.jetty.ee10.websocket.servlet.WebSocketUpgradeFilter.doFilter(WebSocketUpgradeFilter.java:195) ~[jetty-ee10-websocket-servlet-12.0.0.beta2.jar:12.0.0.beta2]
	at org.eclipse.jetty.ee10.servlet.FilterHolder.doFilter(FilterHolder.java:205) ~[jetty-ee10-servlet-12.0.0.beta2.jar:12.0.0.beta2]
	at org.eclipse.jetty.ee10.servlet.ServletHandler$Chain.doFilter(ServletHandler.java:1573) ~[jetty-ee10-servlet-12.0.0.beta2.jar:12.0.0.beta2]
	at org.eclipse.jetty.ee10.servlet.ServletHandler$MappedServlet.handle(ServletHandler.java:1534) ~[jetty-ee10-servlet-12.0.0.beta2.jar:12.0.0.beta2]
	at org.eclipse.jetty.ee10.servlet.ServletChannel.lambda$handle$0(ServletChannel.java:427) ~[jetty-ee10-servlet-12.0.0.beta2.jar:12.0.0.beta2]
	at org.eclipse.jetty.ee10.servlet.ServletChannel.dispatch(ServletChannel.java:684) ~[jetty-ee10-servlet-12.0.0.beta2.jar:12.0.0.beta2]
	at org.eclipse.jetty.ee10.servlet.ServletChannel.handle(ServletChannel.java:418) ~[jetty-ee10-servlet-12.0.0.beta2.jar:12.0.0.beta2]
	at org.eclipse.jetty.ee10.servlet.ServletHandler.handle(ServletHandler.java:458) ~[jetty-ee10-servlet-12.0.0.beta2.jar:12.0.0.beta2]
	at org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:504) ~[jetty-security-12.0.0.beta2.jar:12.0.0.beta2]
	at org.eclipse.jetty.ee10.servlet.SessionHandler.handle(SessionHandler.java:663) ~[jetty-ee10-servlet-12.0.0.beta2.jar:12.0.0.beta2]
	at org.eclipse.jetty.server.handler.ContextHandler.handle(ContextHandler.java:798) ~[jetty-server-12.0.0.beta2.jar:12.0.0.beta2]
	at org.eclipse.jetty.server.Handler$Wrapper.handle(Handler.java:611) ~[jetty-server-12.0.0.beta2.jar:12.0.0.beta2]
	at org.eclipse.jetty.server.Server.handle(Server.java:177) ~[jetty-server-12.0.0.beta2.jar:12.0.0.beta2]
	at org.eclipse.jetty.server.internal.HttpChannelState$HandlerInvoker.run(HttpChannelState.java:617) ~[jetty-server-12.0.0.beta2.jar:12.0.0.beta2]
	at org.eclipse.jetty.server.internal.HttpConnection.onFillable(HttpConnection.java:480) ~[jetty-server-12.0.0.beta2.jar:12.0.0.beta2]
	at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:322) ~[jetty-io-12.0.0.beta2.jar:12.0.0.beta2]
	at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:100) ~[jetty-io-12.0.0.beta2.jar:12.0.0.beta2]
	at org.eclipse.jetty.io.SelectableChannelEndPoint$1.run(SelectableChannelEndPoint.java:53) ~[jetty-io-12.0.0.beta2.jar:12.0.0.beta2]
	at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.runTask(AdaptiveExecutionStrategy.java:473) ~[jetty-util-12.0.0.beta2.jar:12.0.0.beta2]
	at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.consumeTask(AdaptiveExecutionStrategy.java:436) ~[jetty-util-12.0.0.beta2.jar:12.0.0.beta2]
	at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.tryProduce(AdaptiveExecutionStrategy.java:288) ~[jetty-util-12.0.0.beta2.jar:12.0.0.beta2]
	at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.produce(AdaptiveExecutionStrategy.java:196) ~[jetty-util-12.0.0.beta2.jar:12.0.0.beta2]
	at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:969) ~[jetty-util-12.0.0.beta2.jar:12.0.0.beta2]
	at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.doRunJob(QueuedThreadPool.java:1194) ~[jetty-util-12.0.0.beta2.jar:12.0.0.beta2]
	at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1149) ~[jetty-util-12.0.0.beta2.jar:12.0.0.beta2]
	at java.base/java.lang.Thread.run(Thread.java:833) ~[na:na]

The same test succeeds with Jetty 11.

@wilkinsona wilkinsona added the Bug For general bugs on Jetty side label Jun 26, 2023
lorban added a commit that referenced this issue Jun 26, 2023
Signed-off-by: Ludovic Orban <[email protected]>
lorban added a commit that referenced this issue Jun 26, 2023
Signed-off-by: Ludovic Orban <[email protected]>
@joakime
Copy link
Contributor

joakime commented Jun 26, 2023

Draft PR #9974 opened for this issue.

lorban added a commit that referenced this issue Jun 27, 2023
Signed-off-by: Ludovic Orban <[email protected]>
@lorban
Copy link
Contributor

lorban commented Jun 27, 2023

@wilkinsona thanks for the report, this turns out to be a very interesting test case as it uncovered a bug in the Jetty code base, as well as another one in the JDK itself!

#9974 is the PR with the Jetty fix, it will be reviewed and merged very soon and should be enough for your example servlet to work as expected.

#9978 is a merged PR that adds a disabled test to the Jetty codebase, referring to the JDK bug we filed and reproducing it.

Let us know if you need more details.

@lorban lorban moved this to 👀 In review in Jetty 12.0.0.beta3 Jun 27, 2023
@joakime
Copy link
Contributor

joakime commented Jun 27, 2023

@wilkinsona the JDK bug is that ZIP/JAR resources accessed via string are normalized, an internal process that forces all backslashes \ to be converted to forward slashes / before being resolved against the zip/jar file listing and contents.

This makes the use of \ in a zip/jar filename impossible to use. (it's interpreted as another path separator)

The other characters in your input string META-INF/resources/nested-reserved-!#\\$%&()*+,:=?@[]-meta-inf-resource.txt are fine and produce no issues.

This is not a problem if the filename nested-reserved-!#\\$%&()*+,:=?@[]-meta-inf-resource.txt is used on disk, only a problem when used in a zip/jar.

lorban added a commit that referenced this issue Jun 28, 2023
#9972 Fix ServletContextApi.getResource* path normalization

Signed-off-by: Ludovic Orban <[email protected]>
@lorban
Copy link
Contributor

lorban commented Jun 28, 2023

@wilkinsona a fix got merged to the jetty-12.0.x branch. We'd be glad to hear back from you and get your opinion about it.

Thanks for the report, btw!

@lorban lorban closed this as completed Jun 28, 2023
@lorban lorban moved this from 👀 In review to ✅ Done in Jetty 12.0.0.beta3 Jun 28, 2023
@wilkinsona
Copy link
Contributor Author

@lorban Thanks for fixing this so quickly. What's the best way for me to try a 12.0.0 snapshot? Are they published anywhere or would I need to build from source?

@lorban
Copy link
Contributor

lorban commented Jun 28, 2023

@wilkinsona our nightly builds are published here: https://oss.sonatype.org/content/repositories/jetty-snapshots/
You'll have to wait until tomorrow for the next snapshot to contain the changes, as those were merged this morning.

@olamy
Copy link
Member

olamy commented Jun 28, 2023

I just forced a build. It would be best if you waited the end of this Jenkins build https://jenkins.webtide.net/job/nightlies/job/jetty-12.0.x-snapshots-deploy/279/

@wilkinsona
Copy link
Contributor Author

Thanks, both. I've just tested 12.0.0-SNAPSHOT and the java.lang.NumberFormatException no longer occurs.

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
None yet
Development

Successfully merging a pull request may close this issue.

4 participants