Jakarta Mail erroneously assumes that classes can be loaded from Thread#getContextClassLoader

[basil]

Problem

When attempting to use Jakarta Mail API 2.1.1 within a Jenkins plugin, the following error occurs:

java.lang.IllegalStateException: Not provider of jakarta.mail.util.StreamProvider was found
	at jakarta.mail.util.FactoryFinder.find(FactoryFinder.java:64)
	at jakarta.mail.util.StreamProvider.provider(StreamProvider.java:186)
	at jakarta.mail.Session.<init>(Session.java:254)
	at jakarta.mail.Session.getInstance(Session.java:308)
	at hudson.tasks.Mailer$DescriptorImpl.createSession(Mailer.java:415)
	at hudson.tasks.Mailer$DescriptorImpl.doSendTestMail(Mailer.java:704)

Evaluation

The cause is explained in this page. The caller hudson.tasks.Mailer is a Jenkins plugin whose class loader is not Thread#getContextClassLoader but which can see the classes in the Jenkins Mailer plugin and its dependencies (including the Jenkins Jakarta Mail plugin). Thread#getContextClassLoader is set to the Jenkins core class loader, which cannot see any Jenkins plugins. Since we bundle Jakarta Activation and Jakarta Mail as Jenkins plugins, invoking

mail-api/api/src/main/java/jakarta/mail/util/FactoryFinder.java

Line 118 in e8622ce

ServiceLoader<T> sl = ServiceLoader.load(factory);

(which, unlike ServiceLoader#load​(Class service, ClassLoader loader), uses Thread#getContextClassLoader) from a Jenkins plugin attempts to find the class in the Jenkins core class loader (which cannot see classes in Jenkins plugins) and fails.

At its core, the problem is that Jakarta Mail generally (and StreamProvider in particular) assumes that Thread#getContextClassLoader has access to the classes that the calling class has access to. This assumption is usually true, but it is not true for Jenkins and other modular applications with a plugin system implemented with a class loader hierarchy.

Workaround

We can successfully work around the problem with code like

Thread t = Thread.currentThread();
ClassLoader orig = t.getContextClassLoader();
t.setContextClassLoader(getClass().getClassLoader());
try {
  Session.getInstance([…]);
} finally {
  t.setContextClassLoader(orig);
}

at each call site in each Jenkins plugin, but there are hundreds of such call sites, making it prohibitively difficult to work around the problem in this way throughout the Jenkins ecosystem.

Suggested solution

Please modify the default behavior to load classes with getClass().getClassLoader() instead of Thread#getContextClassLoader by changing

mail-api/api/src/main/java/jakarta/mail/util/FactoryFinder.java

Line 118 in e8622ce

ServiceLoader<T> sl = ServiceLoader.load(factory);

to ServiceLoader.load(factory, getClass().getClassLoader()). Such a change might seem risky, but it was successfully done (at the request of the Jenkins core developers) after a Twitter discussion in Jackson in FasterXML/jackson-dataformat-xml@97fe9eb without causing regressions for end users.

Note

Note that a similar problem exists in Jakarta Activation in https://github.com/jakartaee/jaf-api/blob/b7fb44ae9d1872bf86b4098f8f1462e7f7b05a3c/api/src/main/java/jakarta/activation/ServiceLoaderUtil.java#L31, but prior to #579 I was able to work around the problem with gross hacks like https://github.com/jenkinsci/jakarta-mail-api-plugin/tree/32b196156bd5774402bd1d5ec6de8ff3665e4b9c/src/main/java/io/jenkins/plugins/jakarta/activation. As of #579 the same problem is also present in Jakarta Mail, and I see no workaround. The ask is for both Jakarta Mail and Jakarta Activation to load classes with getClass().getClassLoader() instead of Thread#getContextClassLoader.

[yersan]

Such a change might seem risky, but it was successfully done (at the request of the Jenkins core developers) after a Twitter discussion in Jackson in FasterXML/jackson-dataformat-xml@97fe9eb without causing regressions for end users.

For Jakarta Mail, such a change could have unwanted side effects since the users should continue being able to load custom providers, which in general are only visible to the application classloader: https://jakarta.ee/specifications/mail/2.1/jakarta-mail-spec-2.1.html#resource-files

[basil]

The application classloader generally has access to custom providers and Jakarta Mail. The risk is nonzero but should be acceptable given the benefits. The current status quo is completely infeasible for modular applications like Jenkins.

[jmehrens]

I would think that we could use a fallback strategy sort of like what is done in Session::getService but instead of applying it to services we apply it to providers.

[basil]

@jmehrens Would you accept a PR for this? If so, could you please go into a lot more detail about how you would want it to be implemented?

[jmehrens]

I would accept a PR for this. As the problem and the solution should be the similar to #144.

1. I think the StreamProvider.provider() signature is incorrect as it should include the ClassLoader as an argument. This would allow the given class loader to be passed to ServiceLoader::load(Class,ClassLoader). It also means that the caller would be forced to think about which class loader should be used. I'm unsure if this was defined in the spec so I just need to go read up on it. Boils down to deprecate old provider method and add new method StreamProvider.provider(ClassLoader). More work to be done to determine if this is viable. If it can't be modified then we can manipulate existing StreamProvider.provider() by changing the value of the context classloader before calling the StreamProvider and then changing it back inside of the Session class.
2. Completing change number 1 and then starting with Session::this if I were to port the getService classloader preference logic it would look something like:

private Session(Properties props, Authenticator authenticator) {
		this.props = props;
		this.authenticator = authenticator;
                debug = Boolean.parseBoolean(props.getProperty("mail.debug"));
	
		initLogger();
		logger.log(Level.CONFIG, "Jakarta Mail version {0}", Version.version);

		Class<?> cl = null;
                StreamProvider sp = null;
                try {
                        cl = getContextClassLoader();
                        if (cl != null)
                            sp =StreamProvider.class.cast(StreamProvider.provider(cl));
                } catch (Exception tryAuth) {
                    logger.log(Level.FINEST, "Unable to load stream provider", tryAuth);
                }
                
                if (sp == null) {
                    try {  // get the Class associated with the Authenticator
                        if (authenticator != null)
		            cl = authenticator.getClass();
                        if (cl != null)
                    	    sp = StreamProvider.class.cast(StreamProvider.provider(cl));
                    } catch (Exception tryCCL) {
                        logger.log(Level.FINEST, "Unable to load stream provider", tryCCL);
                    }
                }

                if (sp == null) {
                    try {
                            cl = StreamProvider.class.getClassLoader();
                            if (cl != null)
                                sp =StreamProvider.class.cast(StreamProvider.provider(cl));
                    } catch (Exception trySystem) {
                        logger.log(Level.FINEST, "Unable to load stream provider", trySystem);
                    }
                }

                if (sp == null) {
                    try {
                            cl = ClassLoader.getSystemClassLoader()
                            sp = StreamProvider.class.cast(StreamProvider.provider(cl));
                    } catch (Exception fail) {
                       logger.log(Level.FINEST, "Unable to load stream provider", fail);
                      //TODO: throw new IllegalStateException???
                    }
                }

                // Use implementation class, because that class loader has access to jakarta.mail module and implementation resources
                cl = sp.getClass().getClassLoader();
                this.streamProvider = sp;
//TODO maybe log the toString values of the provider and classloader??
		// load the resources
		loadProviders(cl);
		loadAddressMap(cl);
		q = new EventQueue((Executor)props.get("mail.event.executor"));
    }

I haven't tested or even compiled this is just a general idea on how session should traverse the classloader maze as it is the same logic that is used when creating Transport or a Store from the Session. My understanding is that this approach would satisfy compatibility concerns around this PR as it is building on an older precedent already established in the project.

[petulal]

Hi all - is there a fix for this? I am having the same issue with spring-boot-starter-mail 3.1.1, java 17

I found this issue from here https://groups.google.com/g/wildfly/c/dwVsQoui6Us which is the same issue I am having but not in wildfly

[petulal]

Sorry to comment again but this is a blocker for me - I have asked on stackoverflow but doesn't seem to have an answer
https://stackoverflow.com/questions/76601476/spring-boot-starter-mail-3-1-1-throws-not-provider-of-jakarta-mail-util-streamp

Any help much appreciated...

[jmehrens]

Sorry to comment again but this is a blocker for me - I have asked on stackoverflow but doesn't seem to have an answer https://stackoverflow.com/questions/76601476/spring-boot-starter-mail-3-1-1-throws-not-provider-of-jakarta-mail-util-streamp

Any help much appreciated...

I answered the SO question. For now you have to use the same workaround as described above in this ticket which is to set/reset the context classloader before use.

[basil]

We have hundreds of such call sites, making it prohibitively difficult to work around the problem in this way throughout the Jenkins ecosystem. Please fix this bug.

[basil]

Happy New Year! A friendly ping that we are still impacted by this bug and are looking forward to the release of a fix.

[jmehrens]

@basil Any chance you can test the code changes in #701?

[basil]

@jmehrens I have reproduced the issue interactively in Jenkins with the Gmail SMTP server at HEAD with commit 6c9ac50 and verified that the issue is resolved with #701.

[lukasj]

@basil @jmehrens - thank you both for the work and testing this, corresponding PR was merged