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

Jetty logging service file leaking to web applications #6112

Closed
sbordet opened this issue Mar 29, 2021 · 17 comments · Fixed by #6336
Closed

Jetty logging service file leaking to web applications #6112

sbordet opened this issue Mar 29, 2021 · 17 comments · Fixed by #6336
Assignees

Comments

@sbordet
Copy link
Contributor

sbordet commented Mar 29, 2021

Jetty version
10.0.x

Description
Deploying a simple *.war file containing slf4j-api-2.0.0-alpha1.jar and slf4j-simple-2.0.0-alpha1.jar, with a class that triggers SLF4J (e.g. Jetty's CrossOriginFilter), the following happens:

2021-03-29 16:01:22.730:WARN :oejw.WebAppContext:main: Failed startup of context o.e.j.w.WebAppContext@1dac5ef{cometd,/cometd,file:///tmp/jetty-0_0_0_0-8080-cometd_war-_cometd-any-2932914642059191478/webapp/,UNAVAILABLE}{/tmp/jetty.base/webapps/cometd.war}
java.util.ServiceConfigurationError: org.slf4j.spi.SLF4JServiceProvider: Provider org.eclipse.jetty.logging.JettyLoggingServiceProvider not found
	at java.base/java.util.ServiceLoader.fail(ServiceLoader.java:589)
	at java.base/java.util.ServiceLoader$LazyClassPathLookupIterator.nextProviderClass(ServiceLoader.java:1211)
	at java.base/java.util.ServiceLoader$LazyClassPathLookupIterator.hasNextService(ServiceLoader.java:1220)
	at java.base/java.util.ServiceLoader$LazyClassPathLookupIterator.hasNext(ServiceLoader.java:1264)
	at java.base/java.util.ServiceLoader$2.hasNext(ServiceLoader.java:1299)
	at java.base/java.util.ServiceLoader$3.hasNext(ServiceLoader.java:1383)
	at org.slf4j.LoggerFactory.findServiceProviders(LoggerFactory.java:104)
	at org.slf4j.LoggerFactory.bind(LoggerFactory.java:147)
	at org.slf4j.LoggerFactory.performInitialization(LoggerFactory.java:139)
	at org.slf4j.LoggerFactory.getProvider(LoggerFactory.java:418)
	at org.slf4j.LoggerFactory.getILoggerFactory(LoggerFactory.java:404)
	at org.slf4j.LoggerFactory.getLogger(LoggerFactory.java:353)
	at org.slf4j.LoggerFactory.getLogger(LoggerFactory.java:379)
	at org.eclipse.jetty.servlets.CrossOriginFilter.<clinit>(CrossOriginFilter.java:122)

The *.war file does not contain at all any reference to JettyLoggingServiceProvider, so it must be leaking from the server class-loader as a resource; trying to load it as a class fails because the class is a server class and it's hidden to applications.

@sbordet
Copy link
Contributor Author

sbordet commented Mar 29, 2021

Originally reported as cometd/cometd#1013.

@joakime
Copy link
Contributor

joakime commented Mar 29, 2021

So the WAR / WebAppClassloader allows the resource META-INF/services/org.slf4j.spi.SLF4JServiceProvider to be found and returned to the ServiceLoader, but the class it points to is still filtered out by the classloader isolation. Ugh.

Wonder how hard it would be to just remove the logging from CrossOriginFilter?

@joakime
Copy link
Contributor

joakime commented Mar 29, 2021

Or, do we want to expose the jetty logger in the WebAppClassLoader if it's used?

But what happens if someone uses CrossOriginFilter and say logback on the server?

@sbordet
Copy link
Contributor Author

sbordet commented Mar 29, 2021

So the WAR / WebAppClassloader allows the resource META-INF/services/org.slf4j.spi.SLF4JServiceProvider to be found and returned to the ServiceLoader, but the class it points to is still filtered out by the classloader isolation.

That's the hypothesis, needs to be verified.

@sbordet
Copy link
Contributor Author

sbordet commented Mar 29, 2021

Perhaps we should do the same in Jetty 10+?

I don't think so... I think the WebAppClassLoader should hide resources too, not only classes, IFF the hypothesis is correct.

@jmcc0nn3ll
Copy link
Contributor

I am with Simone on that, by default should be excluded but we could certainly have a property with the mod file for logging that you can edit in your base to expose if you so desire.

@gregw
Copy link
Contributor

gregw commented Mar 30, 2021

Is the CrossOriginFilter being loaded from a jar in the webapp, or has it been exposed with jetty-servlets and thus loaded from the container classloader?

Eitherway, I think if we are to expose jetty-servlets from the server classpath, then we should remove all logging from them as a needless dependency complication.

@andrascsibi
Copy link

I'm getting the same exception but even with a simple war without any slf4j dependenies.

I'm using jetty runner 11.0.2.

java -jar jetty-runner-11.0.2.jar --port 8081 build/libs/nagyarbi.war

Please excuse posting everything here but then this is the entirety of my setup.

project structure

src/
└── main
    ├── java
    │   └── nagyarbi
    │       ├── HealthCheck.java
    │       └── JerseyApplication.java
    ├── resources
    │   └── META-INF
    └── webapp
        └── WEB-INF
            └── web.xml

build.gradle.kts

plugins {
    war
    eclipse
}

repositories {
    mavenCentral()
    maven {
        url = uri("https://jitpack.io")
    }
}

dependencies {

    implementation("org.glassfish.jersey.containers:jersey-container-jetty-http:3.0.1")
    implementation("org.glassfish.jersey.core:jersey-server:3.0.1")
    implementation("org.glassfish.jersey.media:jersey-media-json-jackson:3.0.1")
    implementation("com.fasterxml.jackson.jaxrs:jackson-jaxrs-json-provider:2.12.3")

    implementation("javax:javaee-web-api:8.0")

    implementation("org.eclipse.jetty:jetty-util:11.0.2")
    implementation("org.eclipse.jetty:jetty-servlet:11.0.2")
    implementation("org.eclipse.jetty:jetty-server:11.0.2")
}

web.xml

<web-app xmlns="http://java.sun.com/xml/ns/j2ee" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://java.sun.com/xml/ns/j2ee http://java.sun.com/xml/ns/j2ee/web-app_2_4.xsd"
version="2.4">

  <display-name>NAGYARBI</display-name>

  <servlet>
    <servlet-name>nagyarbi</servlet-name>
    <servlet-class>org.glassfish.jersey.servlet.ServletContainer</servlet-class>
    <init-param>
        <param-name>javax.ws.rs.Application</param-name>
        <param-value>nagyarbi.JerseyApplication</param-value>
    </init-param>
    <init-param>
      <param-name>jersey.config.server.provider.packages</param-name>
      <param-value>nagyarbi</param-value>
    </init-param>
    <load-on-startup>1</load-on-startup>
  </servlet>
  <servlet-mapping>
    <servlet-name>nagyarbi</servlet-name>
    <url-pattern>/*</url-pattern>
  </servlet-mapping>

</web-app>

HealthCheck.java

package nagyarbi;

import javax.ws.rs.GET;
import javax.ws.rs.Path;
import javax.ws.rs.Produces;
import javax.ws.rs.core.MediaType;


@Path("is_alive")
public class HealthCheck {


	@Path("{is_alive}")
	@GET
	@Produces(MediaType.TEXT_PLAIN)
	public String isAlive() {
		return "possibly";
	}
}

JerseyApplication.java

package nagyarbi;

import org.glassfish.jersey.jackson.JacksonFeature;
import org.glassfish.jersey.server.ResourceConfig;

public class JerseyApplication extends ResourceConfig {
    public JerseyApplication() {
        register(JacksonFeature.class);
    }
}

Thanks.

@joakime
Copy link
Contributor

joakime commented Apr 26, 2021

@andrascsibi check your built WAR file WEB-INF/lib/*.jar for jars that need slf4j.
Jetty 10+ jars will, and so will other libs.

@andrascsibi
Copy link

OK, thanks. So jetty-*:11.0.2 seem to be the culprit. So is there a workaround for using the jetty libs together with the jetty runner? Thanks.

@joakime
Copy link
Contributor

joakime commented Apr 27, 2021

   implementation("org.eclipse.jetty:jetty-util:11.0.2")
   implementation("org.eclipse.jetty:jetty-servlet:11.0.2")
   implementation("org.eclipse.jetty:jetty-server:11.0.2")

OK, thanks. So jetty-*:11.0.2 seem to be the culprit. So is there a workaround for using the jetty libs together with the jetty runner? Thanks.

Those (specific) Jetty jars shouldn't be in your WEB-INF/lib/, as they come from jetty-runner.

@janbartel janbartel self-assigned this May 31, 2021
@janbartel
Copy link
Contributor

@gregw we can mostly replace all of the log.info() and log.warn() messages with ServletContext.log() methods instead, but what about log.debug()?

@gregw
Copy link
Contributor

gregw commented May 31, 2021

@janbartel I think we should just remove the debug.
Ultimately I think we should deprecate all of these servlets and find some other way of providing them other than exposing them through the classloader.

@vmassol
Copy link

vmassol commented Jan 12, 2024

Hello,

I'm facing this issue in XWiki (we're trying to upgrade to Jetty 12). In our WAR we have the slf4j-api-2.0.10.jar JAR and when starting jetty 12.0.5, we get:

SLF4J(E): A service provider failed to instantiate:
org.slf4j.spi.SLF4JServiceProvider: Provider org.eclipse.jetty.logging.JettyLoggingServiceProvider not found

If I remove this JAR, Jetty starts without this message.

Also note that we have the following JARs too (drawn by some of our 3rd party deps), but removing them doesn't change the message displayed:

jetty-alpn-client-9.4.53.v20231009.jar
jetty-client-9.4.53.v20231009.jar
jetty-io-9.4.53.v20231009.jar
jetty-alpn-java-client-9.4.53.v20231009.jar
jetty-http-9.4.53.v20231009.jar
jetty-util-9.4.53.v20231009.jar

Some more info on our logging setup:

Thanks a lot!

@joakime
Copy link
Contributor

joakime commented Jan 12, 2024

@vmassol I think Issue #4652 would need to be addressed to help you.

@vmassol
Copy link

vmassol commented Jan 14, 2024

@vmassol I think Issue #4652 would need to be addressed to help you.

Thanks for the link! I'm not sure I understand fully #4652 . Isn't it about jetty classes being in WEB-INF/lib? When I remove these classes to test, I still get the problem. The only thing that makes the warning disappear is when I remove slf4j-api-2.0.10.jar from our WEB-INF/lib.

I would expect that a webapp having the slf4j jar in WEB-INF/lib is a pretty common use case, and you'd get complaints all the time about it if it wasn't working, so maybe there's something more in our cases that I don't see.

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants