WebAppClassLoader should protect ServiceLoader access of META-INF/services and getResource() lookups

[gregw]

If a webapp uses the ServiceLoader, then the WebAppClassLoader should handle server classes by:

• not finding a resource /META-INF/service/org.some.Service if org.some.Service is a server class
• filtering the content of any /META-INF/service/org.some.Service file so that any implementations listed within the file that are server classes are also removed

[stale]

This issue has been automatically marked as stale because it has been a full year without activity. It will be closed if no further activity occurs. Thank you for your contributions.

[gregw]

I think we should still try to fix this one. #6112 was a problem caused by not having the filter on services files. @janbartel came up with a good way of work around by adding a location specific pattern for the server classes, which apparently does exclude the services files as well. But I don't think that is general enough for all circumstances

[gregw]

@lorban want to put this on your todo list?

[lorban]

This isn't as easy as it sounds.

Editing out /META-INF/service/... files on the fly would require handing out URLs that point to the modified files. Should we hand out URLs pointing to temporary files on disk? In that case, we must be careful with duplicate service filenames and if/when to cleanup those files. Or should we keep the changes in-memory and hand out a new kind of URL? That would require creating a new URL handler, which could be quite substantial.

This statement also sounds odd to me:

• not finding a resource /META-INF/service/org.some.Service if org.some.Service is a server class

is that case possible at all, knowing that the ServiceLoader API takes a Class instance as its argument to build the /META-INF/service/... resource name?

[gregw]

Taking the second question first. The case is possible because a webapplication can itself contain jetty classes within it's WEB-INF/lib. So for example, there could be code within a webapp using the service loader to find implementations of org.eclipse.jetty.http.HttpFieldPreEncoder - but that is the webapps classloaders version of that class and not the servers version of it. Thus the service loader needs to not find any /META-INF/service/org.eclipse.jetty.http.HttpFieldPreEncoder files in jars that are on the server classpath and not the webapps classpath.

For the second part, yes this is difficult. Temp files are one solution... or potentially we could use a custom URLStreamHandlerFactory (but are they also discovered by the ServiceLoader, in which case we have turtles all the way down!). But I think a solution would be useful as ServiceLoaders are being used more and there a real use-cases for having jetty classes within the scope of a webapp.

[sbordet]

Also, remember that service loading is very different with JPMS, where JPMS modules declare the service class and what are the implementations, etc.

Any solution would need to be tested with JPMS as well.

[github-actions]

This issue has been automatically marked as stale because it has been a
full year without activity. It will be closed if no further activity occurs.
Thank you for your contributions.

[gregw]

kicking the can along the road....

[github-actions]

This issue has been automatically marked as stale because it has been a
full year without activity. It will be closed if no further activity occurs.
Thank you for your contributions.

[sbordet]

@gregw how's the status of this issue?

[gregw]

@sbordet it is not done. It is still a nice to have (or nice to investigate), but we can proceed without it.

[joakime]

This should be expanded to protect ClassLoader.getResource(String) usages too.

Eg: a logging implementation in the webapp, should not be able to find a hit when using ClassLoader.getResource("logging.xml") that points to something that only exists on the server classloader, and doesn't exist in the webapp classloader.

[github-actions]

This issue has been automatically marked as stale because it has been a
full year without activity. It will be closed if no further activity occurs.
Thank you for your contributions.