[DS-2437] Stop relying on alphabetical loading of jars in WEB-INF/lib

[dspace-bot]

Imported from JIRA [DS-2437] created by bram

This bug affects people who using the practice of keeping customizations to modules such as DSpace-API in the /modules/additions folder according to the docs at:
https://wiki.duraspace.org/display/DSDOC5x/Advanced+Customisation

These customizations will be packaged as additions.jar and end up in the WEB-INF/lib directory together with the standard dspace-api and other JARS.

Right now, there is no custom classloader or other mechanism that guarantees that the servlet container will use the classes in additions.jar instead of the other ones. Tomcat v5-v7 happen to load the jars alphabetically, and because additions.jar comes first, these classes get loaded.

This is not part of the servlet spec, nor documented in Tomcat:
http://stackoverflow.com/questions/5474765/order-of-loading-jar-files-from-lib-directory (answer with 18 pluses).

This bug will not manifest for people using Tomcat v5-v7 but WILL result in problems for people on Tomcat 8 according to this report:
https://issues.apache.org/bugzilla/show_bug.cgi?id=57129
(Won't fix - Tomcat says applications, like DSpace should deal with it, and not Tomcat itself)

[dspace-bot]

bram said:

Bumping this issue again because of the anticipated problems with Tomcat 8

[dspace-bot]

mwood said:

We may want to re-examine the way we provide additions. There may be another way around this.

[dspace-bot]

bram said:

Bumping this again to raise the profile on this issue. We will need to deal with this for DSpace 6 as people will start using Tomcat 8.

[dspace-bot]

tdonohue said:

Just a thought here, in the WEB-INF, the documented order of precedence is:

• First 'WEB-INF/classes'
• Then 'WEB-INF/lib'

See http://tomcat.apache.org/tomcat-8.0-doc/class-loader-howto.html

So, it seems like, one route may be to stop packaging up an "additions.jar" and simply compile those custom classes and add them to 'WEB-INF/classes' (which is guaranteed loaded first). The only "catch" would be finding a way to also ensure those classes are made available to commandline tools (which use [dspace]/lib instead of WEB-INF/lib)...but it seems like we should be able to find a way to make that work as well.

[dspace-bot]

mwood said:

Searching WEB-INF/classes before WEB-INF/lib is part of the Servlet specification. For example, in Servlet 3.0, it is in section 10.5:

The Web application class loader must load classes from the WEB-INF/ classes directory first, and then from library JARs in the WEB-INF/lib directory.

So, if that is sufficient to our needs, we should be able to rely on this behavior regardless the brand of container used by a site.

[dspace-bot]

cwilper said:

I would prefer to ultimately see a plugin/extensibility strategy that does not depend on classpath ordering, or class "overlays" at all. Better interfaces/extensibility points, the maven shade plugin, and use of more formalized frameworks like OSGi could help in the long run.

But each of those are probably a long way off, if they'll ever happen at all. In the meantime, Tim's suggestion seems reasonable as long as the command line script(s) can follow suit by specifying classes before the lib dir.

To support cases where multiple overriding jars are used (custom modules that aren't distributed under additions.jar, but other jar names, like acme-module2.jar) it would be nice if one could easily configure their build do this with those as well. Say there's some built in utility to "explode" additions.jar by default into /classes. It would be nice to be able to add to the ordered list of maven GAVs that the maven and/or ant build uses as input to that process.

[dspace-bot]

mwood said:

We need to be clear about what belongs in "additions". Local code to be merged into a specific webapp. belongs in modules/xmlui, modules/rdf, or the like. So it seems that modules/additions is specifically for extending/overlaying dspace-api, yes?

If that is the case, then we should do it in a fashion similar to that used in the webapp.s. That is: modules/additions should not result in a separate additions.jar, but be merged with dspace-api code to yield a single jar. Then code from both projects will be under a single classpath element and there is no ordering issue.

[dspace-bot]

bram said:

shamelessly bumping this again without adding any new information.

Would be a pity if we're releasing DSpace 6 without Tomcat 8 support.

[dspace-bot]

tdonohue said:

Thanks Bram Luyten (Atmire). I agree, we need to get a volunteer for this ASAP, as it's important to fix in 6.0. Any volunteers out there?

Based on the brainstorms above, the two "quick fix" options seem to be:

1. Find some way to simply compile "additions" into the WEB-INF/classes, where it will be loaded prior to any JARs. However, that'd imply having to copy/overlay the additions code into every WAR. Plus also finding a way to make it available to commandline tools via [dspace]/lib
2. OR, we find a way to directly "overlay" the dspace-api.jar (as suggested by Mark H. Wood above). Unfortunately, though, by default Maven only has a WAR overlay concept. We'd have to find a Maven plugin out there that could perform a similar overlay concept at the JAR level. Perhaps the maven-shade-plugin? https://maven.apache.org/plugins/maven-shade-plugin/

[dspace-bot]

tdonohue said:

I just created a PR which goes with Option #1 (WEB-INF/classes approach). Needs further testing, but it seems to work for the basics:
https://github.com/DSpace/DSpace/pull/1331

[dspace-bot]

tom.desair said:

There is also a "tomcat 8 only" solution. There is a configuration option in Tomcat 8 which allows you to give priority to certain JARs when constructing the classpath (https://tomcat.apache.org/tomcat-8.0-doc/config/resources.html#Nested_Components). In our case, adding the following to your webapp context XML file does the trick:

"org.apache.catalina.webresources.StandardRoot">
    "org.apache.catalina.webresources.JarResourceSet"
      base="/path/to/dspace/lib/additions-5.4.jar"
      internalPath="/"
      webAppMount="/WEB-INF/classes" />

This dynamically (and virtually) puts the additions.jar (or any other jar if you want) in the WEB-INF/classes folder of the webapp. I tested this and it seems to work. But I think it's best that someone else also tries this and confirms my findings.

The advantage over copying the addition class files for every webapp is that this solution saves some space and also works for older DSpace versions. The disadvantage is that it will only work for Tomcat (8) and no other Servlet Container Server.

[dspace-bot]

rdillen said:

https://github.com/DSpace/DSpace/pull/1333
Does what Tim Donohue did in #1331 but will allow you to compile against additions' additions. I am pretty sure that no additional work is required to completely remove additions from WEB-INF/lib as the Servlet Spec specifically states
According to servlet spec v3 section 10.5

"The Web application class loader must load classes from the WEB-INF/classes directory first, and then from library JARs in the WEB-INF/lib directory."

[dspace-bot]

tom.desair said:

I would indeed remove additions from the WEB-INF/lib directory (as it will never be used if its classes are in WEB-INF/classes).

Since we're fixing the classpath here, should we also take a look at the DSpace command line interface? I think the DSpace bin now also relies on the alphabetical loading of jars: https://github.com/DSpace/DSpace/blob/master/dspace/bin/dspace#L59

I think it would be better to explicitly put the additions jar at the beginning of the classpath. Can we add this to the PR?

[dspace-bot]

rdillen said:

I was pondering the same issue. We may introduce subtle inconsistencies here between the command line and the webapps. I will look into having maven determine what classes are loaded.

[dspace-bot]

tdonohue said:

I've merged Roeland Dillen's PR and mine into one... https://github.com/DSpace/DSpace/pull/1331 There were a few minor changes to apply to Roeland Dillen's PR.

This PR was merged, and resolves the issue of alphabetic loading of JARS in WEB-INF/lib. Now all "additions", are automatically extracted into the WEB-INF/classes/.

(Regarding the discussions here about alphabetic loading of JARs from commandline scripts. At this point in time, there is no known issue with that, so that has been left unresolved. If an issue is found, a new ticket should be opened.)

Closing this as resolved in 6.0.

[dspace-bot]

tom.desair said:

I've been testing the "tomcat 8 only" configuration solution with different DSpace versions (3.2, 4.4, 5.2 and 5.4) that each have different customizations and I did not encounter a problem yet. All customizations in the additions.jar look to be working fine.

Do I extend these wiki pages on how to configure Tomcat 8?

• https://wiki.duraspace.org/display/DSDOC5x/Installing+DSpace#InstallingDSpace-ServletEngine%28ApacheTomcat7orlater,Jetty,CauchoResinorequivalent%29
• https://wiki.duraspace.org/display/DSDOC5x/Advanced+Customisation

[dspace-bot]

tdonohue said:

Tom Desair : You are welcome to add an additional bullet to the DSpace 5x Installation Documentation about how to configure Tomcat 8 + DSpace 5. We already have a few Tomcat 8 notes there already, and it would be good to enhance those docs with this other known Tomcat 8 issue.

In DSpace 6, however, the "Tomcat 8 only" workaround will no longer be necessary (because of PR#1331, which fixes this).

[dspace-bot]

mwood said:

The other day I ran into a similar problem with a third-party add-on that uses our overlay approach.  Such products will need to be extracted to WEB-INF/classes too, if they are to work reliably on Tomcat 8+ (and perhaps in other containers as well).

[dspace-bot]

Is related to: DS-3092

[dspace-bot]

Addresses / Fixes: DS-2615