Bug 61366

Summary:	"Unable to find [comp]" when attempting to use JNDI datasource for JDBCStore session store
Product:	Tomcat 8	Reporter:	Jonathan Horowitz <jhorowitz>
Component:	Catalina	Assignee:	Tomcat Developers Mailing List <dev>
Status:	RESOLVED FIXED
Severity:	major	CC:	jhorowitz
Priority:	P2
Version:	8.5.16
Target Milestone:	----
Hardware:	All
OS:	All
Attachments:	Properly set the TCCL before performing JNDI datasource lookups Properly set the TCCL before performing JNDI datasource lookups Set TCCL to the webapp classloader if localDataSource == true

Description Jonathan Horowitz 2017-08-01 00:05:18 UTC

Created attachment 35191 [details]
Properly set the TCCL before performing JNDI datasource lookups

When attempting to utilize a JNDI datasource defined at the Context level, JDBCStore uses the wrong TCCL and the lookup fails.

The fix for this (patch attached against current 8.5.x-trunk (revision 1803599) is similar to the changes introduced by revisions 1514281 and 1726925:  the JDBCStore session store implementation correctly sets the TCCL to that of the webapp context before attempting to load a session from the database.

It should do the same before attempting the JNDI datasource lookup.

Comment 1 Jonathan Horowitz 2017-08-01 00:47:04 UTC

Created PR at https://github.com/apache/tomcat/pull/71

Updating patch to reflect the PR changes, backported to 8.5.x-trunk.

Comment 2 Jonathan Horowitz 2017-08-01 00:47:46 UTC

Created attachment 35192 [details]
Properly set the TCCL before performing JNDI datasource lookups

Comment 3 Mark Thomas 2017-08-01 12:57:57 UTC

The patch forces a switch from a global DataSource to a local one. That is going to break configurations for anyone currently using this Store.

I think what is required is something similar to the DataSourceRealm's localDataSource attribute that allows either to be used.

Comment 4 Jonathan Horowitz 2017-08-01 17:39:55 UTC

Created attachment 35194 [details]
Set TCCL to the webapp classloader if localDataSource == true

Comment 5 Jonathan Horowitz 2017-08-01 17:40:20 UTC

Thanks Mark, the patch and PR have been modified to reflect your recommendation.

Comment 6 Mark Thomas 2017-08-02 15:39:05 UTC

Many thanks.

I added some docs and a change log entry and applied it.

Fixed in:
- trunk for 9.0.0.M26 onwards
- 8.5.x for 8.5.20 onwards
- 8.0.x for 8.0.46 onwards

I didn't back-port to 7.0.x because the back-port was not clean and it seems unlikely that anyone already using 7.0.x is going to need this option.