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

Creating a Resource for an entry in a nested jar file in Jetty 12 #9973

Closed
wilkinsona opened this issue Jun 26, 2023 · 43 comments · Fixed by #9995 or #10058
Closed

Creating a Resource for an entry in a nested jar file in Jetty 12 #9973

wilkinsona opened this issue Jun 26, 2023 · 43 comments · Fixed by #9995 or #10058
Assignees
Labels

Comments

@wilkinsona
Copy link
Contributor

Jetty version

Jetty 12.0.0.Beta2

Java version

17

Question

I'm working on upgrading Spring Boot to Jetty 12. Spring Boot supports executable jars and wars that can be run using java -jar. This results in URLs pointing to jars nested within a jar or war. For example, this URL taken from our test suite:

jar:file:/Users/awilkinson/dev/spring-projects/spring-boot/main/spring-boot-tests/spring-boot-integration-tests/spring-boot-server-tests/build/spring-boot-server-tests-app/build/libs/spring-boot-server-tests-app-jetty.war!/WEB-INF/lib/spring-boot-server-tests-app-resources.jar!/

Much of the path doesn't matter. A shorter example would be:

jar:file:/example.war!/WEB-INF/lib/spring-boot-server-tests-app-resources.jar!/

With Jetty 11 and earlier, I could create a Resource that points to the META-INF/resources entry within spring-boot-server-tests-app-resources.jar! by calling Resource.newResource(url + "META-INF/resources"). I am struggling to find the Jetty 12 equivalent of this code.

I have a WebAppContext to hand so I have tried calling ResourceFactory resourceFactory = ResourceFactory.of(handler) and calling ResourceFactory.newResource. This works for situations where spring-boot-server-tests-app-resources.jar is available directly on the file system but not for the case where it's nested within a war.

How can I get this working for nested jars using Jetty 12 please?

@joakime
Copy link
Contributor

joakime commented Jun 26, 2023

Quick answer / example:

Create/Open Resources via the ResourceFactory, but be aware of resource management.

Path exampleJar = Path.of("example.jar");

try (ResourceFactory.Closeable resourceFactory = ResourceFactory.closeable())
{
    // Create Resource from jar/zip, mounted to root of jar/zip
    Resource jarFileResource = resourceFactory.newJarFileResource(exampleJar.toUri());

    // access as Resource
    Resource manifestResource = jarFileResource.resolve("/META-INF/MANIFEST.MF");
    try (InputStream inputStream = manifestResource.newInputStream())
    {
        // read from input stream
    }

    // Create Resource from jar/zip URI
    URI uri = URI.create("jar:file:/example.jar!/");
    Resource uriResource = resourceFactory.newResource(uri);
    // access as Path object
    Path manifestPath = uriResource.getPath().resolve("/META-INF/MANIFEST.MF");

    // create new Resource from Path object
    PathResource resource = (PathResource)resourceFactory.newResource(manifestPath);

    try (InputStream inputStream = resource.newInputStream())
    {
        // read from input stream
    }

}

A ResourceFactory exists to track/manage of the resources is manages (eg: cleanup), it can be created in a few ways.

  • ResourceFactory.root() will create a ResourceFactory associated with the running JVM, and is never cleaned up.
  • ResourceFactory.closeable() will create a ResourceFactory that can be closed via normal JVM resource management techniques (eg: try-with-resources)
  • ResourceFactory.lifecycle() will create a ResourceFactory that has normal Jetty LifeCycle associated with it, suitable to manage via the normal Jetty lifecycle techniques.
  • ResourceFactory.of(Container) will create a ResourceFactory.lifecycle() and associated it properly with the Jetty Container lifecycle (eg: WebAppContext, Server, ContextHandler, etc), so that when that Container lifecycle is stopped, so are all of the resources associated with that ResourceFactory.

Once you have the ResourceFactory you can create a Resource using one of the techniques in ResourceFactory.new*() methods (see javadoc for details).

Now to get to details ...
In Jetty 12, the first level entry is a managed resource with a JVM FileSystem lifecycle. jar:<path>.
All access to content within that resource is done via the JVM FileSystem. jar:<path>!/<content>.

In Jetty 11 and earlier, this was all done via the java.net.URL and associated protocol/stream handing in the JVM.
That meant each time you accesses the deep resource content jar:<path>!/<content> a new URL object was created, and the jar opened, and the resource was accessed, and depending on the JVM implementation the jar is closed/cached for later access.
Now that all of the public URL constructors are deprecated in new JVMs, and the variations of zip-slip bugs is increasing, the Jetty project moved to using the JVM built-in zipfs support on the java.nio.file package (which also protects against zip-slip without additional effort on the project using it).

One behavior of this decision is at the JVM level, namely this means once a jar is opened via a java.nio.file.FileSystem the JVM tracks this open jar with a specific FileSystem object. Other code that wants to open/use that same jar have to use the same FileSystem object. But once all usages are done, then the FileSystem.close() can be used.
Internally, Jetty tracks this usage via the FileSystemPool class.

Lets take your example URL of jar:file:/example.war!/WEB-INF/lib/spring-boot-server-tests-app-resources.jar!/
That's a bit more complex.
What the WebAppContext will do is ...

  • Load file:/example.war as a Resource managed by a ResourceFactory that is on the WebAppContext lifecycle, resulting in an entry for jar:file:/example.war!/ (this also becomes the base resource for static content not served via the ClassLoader)
  • Find all of the details of the war (following servlet spec) that are needed to build up a proper description/descriptor of the webapp to start.
  • The jars in WEB-INF/lib/ are unpacked to the ServletContext temp directory (see: ServletContext.TEMPDIR)
  • Then each unpacked jar is loaded as a managed resource within the WebAppContext / WebAppClassloader.
  • Access to the contents of WEB-INF/lib and WEB-INF/classes is done through traditional ClassLoader behavior.
  • Further content that needs it's own entry in a combined/compound base resource (eg: jar:file:/<servlet-context-temp-dir>/foo.jar!META-INF/resources/) are made available.
  • WebAppContext is started.

@joakime
Copy link
Contributor

joakime commented Jun 26, 2023

Note also, that the Resource obtained via Resource.new*() methods can be deep references.
Useful for declaring deep directories in jars to be the "base resource" for a webapp.

Example:

try (ResourceFactory.Closeable resourceFactory = ResourceFactory.closeable())
{
    URI uri = URI.create("jar:file:/example.jar!/webapp/base/");
    Resource baseResource = resourceFactory.newResource(uri);
    // this will result in a null
    Resource manifestResource = baseResource.resolve("../../META-INF/MANIFEST.MF");
    // this will work if that content exists in the `webapp/base/` directory
    Resource resource = resourceFactory.resolve("index.html");

    try (InputStream inputStream = resource.newInputStream())
    {
        // read from input stream
    }
}

@joakime
Copy link
Contributor

joakime commented Jun 26, 2023

Also of note, if you have a WebAppContext object instance, even if it's not started, you can access it's own ResourceFactory with ...

WebAppContext webappContext = new WebAppContext();
URI baseURI = URI.create("jar:file:/example.war!/");
Resource baseResource = webappContext.getResourceFactory().newResource(baseURI);
webappContext.setWarResource(baseResource);

This will let the lifecycle of the WebAppContext manage the lifecycle of the resources you use.

@wilkinsona
Copy link
Contributor Author

Thanks very much, @joakime.

It looks the WebAppContext behavior that you have described is (largely) performed by WebInfConfiguration which we haven't been using in Spring Boot thus far. We leave everything nested inside the war file and register resources for META-INF/resources ourselves. This relies on a URL-based approach where a custom URLStreamHandler plugs in our support for nested jars.

the Jetty project moved to using the JVM built-in zipfs support on the java.nio.file package (which also protects against zip-slip without additional effort on the project using it).

Does this move to the JVM's built-in zipfs support prevent the old URL-based approach from being used?

Our nested, executable jars don't support NIO FileSystems at the moment and I suspect that's a bigger piece of work than we have time for right now. Two alternatives are to continue with a URL-based approach (if that's possible in Jetty 12) or perhaps to start using WebInfConfiguration and rely on the contents of the war being extracted so that there's no nesting to deal with.

@joakime
Copy link
Contributor

joakime commented Jun 27, 2023

Thanks very much, @joakime.

It looks the WebAppContext behavior that you have described is (largely) performed by WebInfConfiguration which we haven't been using in Spring Boot thus far. We leave everything nested inside the war file and register resources for META-INF/resources ourselves. This relies on a URL-based approach where a custom URLStreamHandler plugs in our support for nested jars.

Note that URLStreamHandler is also impacted by the great URL deprecation effort underway on the JDK.

The default URLStreamHandler for jar protocol has not handled nested artifacts properly.
Does your custom version handle zip-slip properly?

Note that you don't need the nested jar in a jar in a jar ... to have an executable war.
Our https://github.com/jetty-project/embedded-jetty-live-war demonstrates that, all the way back to Jetty 9/10/11.
And it includes protection to not expose those jar resources as web resources as well.

the Jetty project moved to using the JVM built-in zipfs support on the java.nio.file package (which also protects against zip-slip without additional effort on the project using it).

Does this move to the JVM's built-in zipfs support prevent the old URL-based approach from being used?

We have a URL based Resource implementation available.

ResourceFactory.registerResourceFactory("jar", new URLResourceFactory());

This will incur a heavy performance hit (open / access / close for every resource request), and introduce all of the known URL bugs and vulnerabilities that exist in the JVM you are using.

Our nested, executable jars don't support NIO FileSystems at the moment and I suspect that's a bigger piece of work than we have time for right now. Two alternatives are to continue with a URL-based approach (if that's possible in Jetty 12) or perhaps to start using WebInfConfiguration and rely on the contents of the war being extracted so that there's no nesting to deal with.

I would strongly encourage you to investigate that spring bug sooner than later, as many projects and libraries are using zipfs and FileSystem now. Most making the changes due the mass deprecation of various URL classes/methods in the JDK due to all of the various bugs in the URL class that cannot be fixed there, but are already fixed in existing URI and java.nio.file classes.
We've even been fixing our implementation to make it compatible with graalvm.

@lorban
Copy link
Contributor

lorban commented Jun 28, 2023

@wilkinsona did @joakime's explanation answered your question or do you need more help?

@wilkinsona
Copy link
Contributor Author

I'm still stuck at the moment. #9984 is at least part of that. Looking at the Jetty 11 code and how we called it, I think what we're missing is a Resource implementation that's equivalent to the old JarFileResource:

https://github.com/eclipse/jetty.project/blob/4413c2b8a01cda05b26e760a2d26d395fdd2749f/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java#L157-L160

@lorban
Copy link
Contributor

lorban commented Jun 28, 2023

ResourceFactory should be able to transparently do that out of the box with a simple resourceFactory.newResource() call. You shouldn't have to filter out your URIs by checking their scheme, you should directly give the URI to resourceFactory.newResource() and it should do all the necessary magic to open the jar if needed.

If you want to, we can arrange a meeting so I can help you live review all the changes you made to upgrade to Jetty 12, advise you on how to best use our new API, and help you with any difficulty you may have.

@wilkinsona
Copy link
Contributor Author

wilkinsona commented Jun 28, 2023

ResourceFactory should be able to transparently do that out of the box with a simple resourceFactory.newResource() call

That hasn't appeared to be the case in my testing of Spring Boot's executable jars that support nesting one jar within another.

Unfortunately, like the JDK's JarFile and related classes, the JDK's FileSystem infrastructure does not support nested jar files. Code in our spring-boot-loader module works around this for JarFile but does not yet do so for FileSystem. As far as I can tell, Jetty 12's resource support leans very heavily on Path and FileSystem. This makes our custom JarFile and Handler useless and breaks Spring Boot executable jars and wars with Jetty.

While the Path-based resource support is obviously preferred, it would be hugely beneficial to Spring Boot if Jetty 9, 10, and 11's URL-based resource support continued to work in Jetty 12.

@lorban
Copy link
Contributor

lorban commented Jun 28, 2023

Ok, got it. Let's just hope URLResourceFactory's limited functionality will suffice. You could try replacing the ResourceFactory handling jar files with a URLResourceFactory this way:

ResourceFactory.registerResourceFactory("jar", new URLResourceFactory());

and see how it works for you.

@lorban
Copy link
Contributor

lorban commented Jun 28, 2023

The PR about the NPE in URLResourceFactory just got merged, and I've triggered a new build to publish fresh artifacts to the snapshot repo to help you test the above solution.

Please keep us posted.

@wilkinsona
Copy link
Contributor Author

Thanks for the NPE fix. Unfortunately, it's not sufficient to reach parity with Jetty 11. Things now get a little further but the lack of support for detecting if the URL points to a directory still leaves us blocked. This is the failure that I'm seeing:

Caused by: java.lang.IllegalArgumentException: Non-Directory not allowed: URLResource@265FDED2(jar:file:/[…]/build/libs/spring-boot-server-tests-app-jetty.jar!/BOOT-INF/lib/spring-boot-server-tests-app-resources.jar!/META-INF/resources)
	at org.eclipse.jetty.util.resource.CombinedResource.gatherUniqueFlatResourceList(CombinedResource.java:87) ~[jetty-util-12.0.0-SNAPSHOT.jar!/:12.0.0-SNAPSHOT]
	at org.eclipse.jetty.util.resource.CombinedResource.combine(CombinedResource.java:44) ~[jetty-util-12.0.0-SNAPSHOT.jar!/:12.0.0-SNAPSHOT]
	at org.eclipse.jetty.util.resource.ResourceFactory.combine(ResourceFactory.java:46) ~[jetty-util-12.0.0-SNAPSHOT.jar!/:12.0.0-SNAPSHOT]
	at org.springframework.boot.web.embedded.jetty.JettyServletWebServerFactory.configureDocumentRoot(JettyServletWebServerFactory.java:327) ~[spring-boot-3.2.0-SNAPSHOT.jar!/:3.2.0-SNAPSHOT]
	... 19 common frames omitted

If we overcome that, I suspect that listing not being supported will be the next problem.

@joakime
Copy link
Contributor

joakime commented Jun 28, 2023

That a META-INF/resources that doesn't have a trailing slash.
Walking URLs will result in that kind of nonsense.
As there's no way of knowing, from the URL itself, if the last entry /foo is a file or directory.
You only know that by requesting/accessing the content, which would put a huge burden on the URL.

So lets see where this could be coming from.
Are you using MetaInfConfiguration or are you populating the base resource yourself (in configureDocumentRoot)?

@joakime
Copy link
Contributor

joakime commented Jun 28, 2023

@wilkinsona looking at ...

https://github.com/spring-projects/spring-boot/blob/main/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyServletWebServerFactory.java#L297-L316

... the URLs from ...

	protected final List<URL> getUrlsOfJarsWithMetaInfResources() {
		return this.staticResourceJars.getUrls();
	}

All need to end with a slash to give the right hint that they are a directory, not a resource.

@joakime
Copy link
Contributor

joakime commented Jun 28, 2023

Also ...

https://github.com/spring-projects/spring-boot/blob/main/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyServletWebServerFactory.java#L318-L326

That is wrong as well.

	private Resource createResource(URL url) throws Exception {
		if ("file".equals(url.getProtocol())) {
			File file = new File(url.toURI());
			if (file.isFile()) {
				return Resource.newResource("jar:" + url + "!/META-INF/resources");
			}
		}
		return Resource.newResource(url + "META-INF/resources");
	}

Each of those "META-INF/resources" strings should end in a slash (ala "META-INF/resources/")

@wilkinsona
Copy link
Contributor Author

wilkinsona commented Jun 28, 2023

We're populating things. We have some common code that we use with Jetty, Tomcat, and Undertow to find all the jars with META-INF/resources. We have URLs for those locations and now need to figure out how to create a Jetty Resource for META-INF/resources within each (potentially nested) jar. I can append a / quite easily and that improved the situation. Thanks for the pointer and apologies – I should have looked more closely at the implementation of exists().

I now see a different failure that can be reproduced with this code:

new URLResourceFactory().newResource("jar:file:example.war!/WEB-INF/lib/resources.jar!/META-INF/resources/").resolve("nested-resource.txt");

It fails with java.lang.IllegalArgumentException: URI is not absolute because URLResource creates a URI from nested-resource.txt and it isn't absolute. It then fails when trying to turn that URI into a URL.

lorban added a commit that referenced this issue Jun 28, 2023
@lorban
Copy link
Contributor

lorban commented Jun 28, 2023

I've created a new JarURLResourceFactory as an attempt to revive Jetty 11's JarURLResource: #9991

We're probably not going to include it in the official Jetty 12 codebase, but you should be able to take this class and import it into your codebase.

Could you please see if it helps, eventually adapting it?

@joakime
Copy link
Contributor

joakime commented Jun 28, 2023

That JarURLResourceFactory cannot handle the use case presented in this issue (nested jars) and introduces a bunch of bugs against our own code and the JVM.

@lorban
Copy link
Contributor

lorban commented Jun 29, 2023

@joakime I agree JarURLResourceFactory is far from ideal, as we've run away from it for good reasons.

But if the Spring team is willing to take ownership of this resource factory until they come up with a java.nio.file.FileSystem implementation which supports their nested jars requirements, I'd say why not. We've made ResourceFactory very flexible and pluggable, so let's take advantage of it if we can.

@wilkinsona
Copy link
Contributor Author

until they come up with a java.nio.file.FileSystem implementation which supports their nested jars requirements

I spent a bit of time looking at this yesterday, and I don't think it's going to be possible. To be able to plug in our support transparently (as we would require and as we can do with URL), we need to be able to replace the JDK's FileSystem for jar: URIs. Unfortunately, we cannot do so as the built-in ZipFileSystemProvider always takes precedence. Unless I've missed something, that leaves us stuck with our existing URLStreamHandler-based approach where we can transparently plug in our custom implementation.

I think we're stuck in a bit of a corner. On one side, we have to stick with a URL- and URLStreamHandler-based approach as FileSystem isn't sufficiently pluggable. On the other side, we have to move to FileSystem and Path as Jetty 12 no longer supports a URL-based approach out of the box. We can't satisfy both of those requirements at the same time.

It would be much appreciated if you would reconsider the removal of the URL-based Resource support that has served Spring Boot so well since Jetty 8.

@joakime
Copy link
Contributor

joakime commented Jun 29, 2023

It would be much appreciated if you would reconsider the removal of the URL-based Resource support that has served Spring Boot so well since Jetty 8.

We have one, it is the URLResourceFactory, lets make that work for you.
Right now, the ball is in your court, the last issue, around META-INF/resources is a bug in your code (the URL class instance created in your code is missing the trailing slash).

@joakime
Copy link
Contributor

joakime commented Jun 29, 2023

I think we're stuck in a bit of a corner. On one side, we have to stick with a URL- and URLStreamHandler-based approach as FileSystem isn't sufficiently pluggable. On the other side, we have to move to FileSystem and Path as Jetty 12 no longer supports a URL-based approach out of the box. We can't satisfy both of those requirements at the same time.

This is possible, but you need to implement your own scheme/protocol, you can't override jar, so make something new, like nested: or even spring-boot: for your work.

There are a host of bugs you are tickling with your choice of using jar for your own custom URLStreamHandler.
Example: We experimented with nested jar support a long time ago (Jetty 6 IIRC), but found assumptions (correct assumptions) in other libraries that would do things like url.indexOf("!/") to find the split in in jar:file: URLs. (which would break the nested approach you have, with 2 !/ entries).

@wilkinsona
Copy link
Contributor Author

Thanks, @joakime.

Right now, the ball is in your court, the last issue, around META-INF/resources is a bug in your code (the URL class instance created in your code is missing the trailing slash).

I've fixed that but hit another problem. Please see this comment above for details. From that comment, a single-line reproducer:

new URLResourceFactory().newResource("jar:file:example.war!/WEB-INF/lib/resources.jar!/META-INF/resources/").resolve("nested-resource.txt");

This throws java.lang.IllegalArgumentException: URI is not absolute.

@wilkinsona
Copy link
Contributor Author

This is possible, but you need to implement your own scheme/protocol, you can't override jar, so make something new, like nested: or even spring-boot: for your work.

This is what I was getting at when I said we need to be able to plug in our support transparently. Requiring a custom scheme doesn't meet that requirement. If someone's working with a jar: URL, they get Spring Boot's nested jar support without having to write any code that's specific to Spring Boot. This is important as there's a huge amount of third-party code that relies upon it. For FileSystem support to be truly useful, we would want the same for jar: URIs such that you could use the URI as-is to get a fully functioning Path.

@joakime
Copy link
Contributor

joakime commented Jun 29, 2023

Merged PR #9995 into jetty-12.0.x

@lorban
Copy link
Contributor

lorban commented Jul 3, 2023

@wilkinsona Have you had the opportunity to test Jetty 12 again after #9995 got merged? What's your current status and plan w.r.t the Spring / Jetty 12 integration? Is there anything else we can do to help?

@gregw
Copy link
Contributor

gregw commented Jul 3, 2023

We have a URL based Resource implementation available.

ResourceFactory.registerResourceFactory("jar", new URLResourceFactory());

This will incur a heavy performance hit (open / access / close for every resource request), and introduce all of the known URL bugs and vulnerabilities that exist in the JVM you are using.

I think we need to look at improving the performance of URLResource to be equivalent to what we had in jetty<12, where the connection was cached in the resource and a new one was only created if necessary. I known URL and URLConnection are deprecated, but there are going to be custom URL schemes that need support for sometime.

I've open issue #10057

@gregw
Copy link
Contributor

gregw commented Jul 3, 2023

We have one, it is the URLResourceFactory, lets make that work for you.
Right now, the ball is in your court, the last issue, around META-INF/resources is a bug in your code (the URL class instance created in your code is missing the trailing slash).

@joakime @lorban I think the ball is a bit in our court. I've long argued that we need good URL support in jetty-12 and this issue is a prime example of why. We need to support URLs in 12, just as well as we did in 11. By all means let us provide a better alternative, but until the zipfs can meet all the use-cases currently being used by real code, it is on us to make sure we do not regress is functionality.

@wilkinsona
Copy link
Contributor Author

Have you had the opportunity to test Jetty 12 again after #9995 got merged?

Thanks for asking, @lorban. #9995 hasn't really helped, unfortunately. We end up with a URLResource like this:

URLResource@629F4C27(jar:file:///Users/awilkinson/dev/spring-projects/spring-boot/main/spring-boot-tests/spring-boot-integration-tests/spring-boot-server-tests/build/spring-boot-server-tests-app/build/libs/spring-boot-server-tests-app-jetty.jar!/BOOT-INF/lib/spring-boot-server-tests-app-resources.jar!/META-INF/resources/)

Jetty tries to resolve /nested-meta-inf-resource.txt against it due to an HTTP request being made for /nested-meta-inf-resource.txt. The resolution results in the URI jar:file:/nested-meta-inf-resource.txt which is incorrect from our perspective.

I've long argued that we need good URL support in jetty-12 and this issue is a prime example of why. We need to support URLs in 12, just as well as we did in 11.

Thanks, @gregw. You probably won't be surprised to hear that I agree wholeheartedly with this.

In a previous comment, you also said this:

I known URL and URLConnection are deprecated, but there are going to be custom URL schemes that need support for sometime.

I don't want to split hairs, but, as far as I know, neither URL or URLConnection is deprecated. The constructors in URL are deprecated in Java 20 but I believe that's it. As long as you're creating a URL by some other means, I don't think you'll touch any deprecated code. The javadoc on the constructor's deprecation even mentions opening a connection as a reason for creating a URL from a URI:

In cases where an instance of java.net.URL is needed to open a connection, URI can be used to construct or parse the URL string

Given this situation in the JDK, from my admittedly biased perspective, I think Jetty 12 really needs to continue to support URLs as it did in earlier versions.

@lorban
Copy link
Contributor

lorban commented Jul 3, 2023

@wilkinsona you should not resolve paths starting with a "/", unless you want the whole uri to be replaced with your resolved path. This is akin to how Path and more generally the filesystem works:

Path path = Path.of("/some/path/");
Path resolvedWithSlash = path.resolve("/nested-meta-inf-resource.txt");
Path resolvedWithoutSlash = path.resolve("nested-meta-inf-resource.txt");

resolvedWithSlash will point to /nested-meta-inf-resource.txt while resolvedWithoutSlash will point to /some/path/nested-meta-inf-resource.txt.

It is also how URI works:

URI uri = URI.create("file:/some/path/spring-boot-server-tests-app-jetty.jar!/BOOT-INF/lib/spring-boot-server-tests-app-resources.jar!/META-INF/resources/");
URI resolvedWithSlash = uri.resolve("/nested-meta-inf-resource.txt");
URI resolvedWithoutSlash = uri.resolve("nested-meta-inf-resource.txt");

resolvedWithSlash will point to file:/nested-meta-inf-resource.txt while resolvedWithoutSlash will point to file:/some/path/spring-boot-server-tests-app-jetty.jar!/BOOT-INF/lib/spring-boot-server-tests-app-resources.jar!/META-INF/resources/nested-meta-inf-resource.txt.

Jetty 11 was very lenient about the effects of missing and extra slashes, but Jetty 12 is not anymore; as you noticed earlier when @joakime told you META-INF/resources needs a trailing slash and you're getting bitten again now.

If you could give me some directions about what tests to run in which branch of Spring Boot and how to run them, we could avoid these inefficient back-and-forth messages. As @gregw pointed out, we're committed to making sure Spring works at least as well as it used to without too much effort from your side.

@wilkinsona
Copy link
Contributor Author

you should not resolve paths starting with a "/", unless you want the whole uri to be replaced

It's not my code that's doing it, it's Jetty's. Here's the stack:

Thread [qtp596470015-43] (Suspended (breakpoint at line 144 in URLResourceFactory$URLResource))	
	URLResourceFactory$URLResource.resolve(String) line: 144	
	CombinedResource.resolve(String) line: 147	
	ResourceFactory$1.newResource(String) line: 397	
	ResourceHttpContentFactory.getContent(String) line: 48	
	VirtualHttpContentFactory.getContent(String) line: 57	
	PreCompressedHttpContentFactory.getContent(String) line: 48	
	DefaultServlet$ServletResourceService(ResourceService).getContent(String, Request) line: 86	
	DefaultServlet.doGet(HttpServletRequest, HttpServletResponse) line: 470	
	DefaultServlet(HttpServlet).service(HttpServletRequest, HttpServletResponse) line: 527	
	DefaultServlet(HttpServlet).service(ServletRequest, ServletResponse) line: 614	
	ServletHolder.handle(ServletRequest, ServletResponse) line: 736	
	ServletHandler$ChainEnd.doFilter(ServletRequest, ServletResponse) line: 1606	
	OrderedCharacterEncodingFilter(CharacterEncodingFilter).doFilterInternal(HttpServletRequest, HttpServletResponse, FilterChain) line: 201	
	OrderedCharacterEncodingFilter(OncePerRequestFilter).doFilter(ServletRequest, ServletResponse, FilterChain) line: 116	
	FilterHolder.doFilter(ServletRequest, ServletResponse, FilterChain) line: 205	
	ServletHandler$Chain.doFilter(ServletRequest, ServletResponse) line: 1578	
	WebSocketUpgradeFilter.doFilter(ServletRequest, ServletResponse, FilterChain) line: 195	
	FilterHolder.doFilter(ServletRequest, ServletResponse, FilterChain) line: 205	
	ServletHandler$Chain.doFilter(ServletRequest, ServletResponse) line: 1578	
	ServletHandler$MappedServlet.handle(ServletHandler, String, HttpServletRequest, HttpServletResponse) line: 1539	
	ServletChannel.lambda$handle$0() line: 488	
	0x0000000800f81160.dispatch() line: not available	
	ServletChannel.dispatch(ServletChannel$Dispatchable) line: 742	
	ServletChannel.handle() line: 479	
	JettyEmbeddedWebAppContext$JettyEmbeddedServletHandler(ServletHandler).handle(Request, Response, Callback) line: 463	
	ConstraintSecurityHandler(SecurityHandler).handle(Request, Response, Callback) line: 510	
	SessionHandler.handle(Request, Response, Callback) line: 663	
	JettyEmbeddedWebAppContext(ContextHandler).handle(Request, Response, Callback) line: 801	
	Server(Handler$Wrapper).handle(Request, Response, Callback) line: 596	
	Server.handle(Request, Response, Callback) line: 177	
	HttpChannelState$HandlerInvoker.run() line: 615	
	HttpConnection.onFillable() line: 480	
	AbstractConnection$ReadCallback.succeeded() line: 322	
	AbstractEndPoint$1(FillInterest).fillable() line: 100	
	SelectableChannelEndPoint$1.run() line: 53	
	QueuedThreadPool.runJob(Runnable) line: 969	
	QueuedThreadPool$Runner.doRunJob(Runnable) line: 1194	
	QueuedThreadPool$Runner.run() line: 1149	
	Thread.run() line: 833	

This was the result of an HTTP GET requests for /nested-meta-inf-resource.txt.

My work on upgrading to Jetty 12 is in this branch. Running ./gradlew spring-boot-tests:spring-boot-integration-tests:spring-boot-server-tests:build should reproduce the failures.

@gregw
Copy link
Contributor

gregw commented Jul 3, 2023

My work on upgrading to Jetty 12 is in this branch. Running ./gradlew spring-boot-tests:spring-boot-integration-tests:spring-boot-server-tests:build should reproduce the failures.

@wilkinsona (edit: oops) for the sake of clarity, can you confirm what branch we should run the same test on to see how it is currently working for jetty-11?

@wilkinsona
Copy link
Contributor Author

Assuming you meant me, any of 3.0.x, 3.1.x, or main will use Jetty 11.

@joakime
Copy link
Contributor

joakime commented Jul 3, 2023

Opened PR #10058 to address URLResource.resolve("/path/that/starts/with/slash.txt") issue.

@gregw
Copy link
Contributor

gregw commented Jul 3, 2023

Note that I ran the tests against the springboot 3.1.x branch and I can see that our JarFileResource is being using, but that it's connection class is org.springframework.boot.loader.jar.JarURLConnection:jar:file:/home/gregw/src/spring-boot/spring-boot-tests/spring-boot-integration-tests/spring-boot-server-tests/build/spring-boot-server-tests-app/build/libs/spring-boot-server-tests-app-jetty.jar!/BOOT-INF/lib/spring-boot-server-tests-app-resources.jar!/META-INF/resources/nested-meta-inf-resource.txt, so that definitely looks like the spring URLStreamHandler is being invoked underneath it.

joakime added a commit that referenced this issue Jul 3, 2023
joakime added a commit that referenced this issue Jul 3, 2023
joakime added a commit that referenced this issue Jul 3, 2023
lorban added a commit that referenced this issue Jul 3, 2023
lorban added a commit that referenced this issue Jul 3, 2023
joakime added a commit that referenced this issue Jul 3, 2023
joakime added a commit that referenced this issue Jul 3, 2023
joakime added a commit that referenced this issue Jul 3, 2023
* use URIUtil.addPaths() instead of URI.resolve()
* Better Connection / InputStream locks
* Removing URLResource.close()
* Adding URLResourceFactory.setReadTimeout()
* restore existence check in isDirectory
* Simplify URLResource.resolve

---------

Signed-off-by: Ludovic Orban <[email protected]>
Signed-off-by: Joakim Erdfelt <[email protected]>
Co-authored-by: Ludovic Orban <[email protected]>
@joakime
Copy link
Contributor

joakime commented Jul 6, 2023

@wilkinsona With the merge of PR #10058 and the release of Jetty 12.0.0.beta3 you should be able to get much further.

I've been able to build + test that set of spring-boot test cases you referenced with a fork of your jetty-12 branch.
See changes at https://github.com/joakime/spring-boot-jetty-12/tree/jetty-12-joakim for details.

@joakime
Copy link
Contributor

joakime commented Jul 6, 2023

@wilkinsona see also the PR at wilkinsona/spring-boot#7

@joakime
Copy link
Contributor

joakime commented Jul 7, 2023

@wilkinsona I think we can close this issue, open new issues for any new concerns with your spring-boot effort

@joakime joakime closed this as completed Jul 7, 2023
@wilkinsona
Copy link
Contributor Author

👍 thanks for all the assistance thus far.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment