XML Transformer factory is being set by System.setProperty, impacting entire JVM, breaking Weblogic 9.x

I guess you're right we shouldn't be tied to any specific xml api impl.
Setting the expected transformer factory impl via System.setProperty is there since the early days of geoserver, and since the reason is not documented I don't know if there is a specific need to expect that one, though as said, we shouldn't be doing that. That to say it is pretty much possible that the guilty for that line of code being there can well be myself... Or at least I remember letting it there because it scared me to remove it...

Now, I know there are at least one more place where a specific xml impl is referenced, this time through direct invocation, so it becomes a hard dependency over xerces. Guess it is the xml Encoder in geotools which directly calls xerces' XMLSerializer.

So the question is two fold:

• does anybody knows if it is safe to remove the call to System.setProperty?
• should we require no hard dependencies over a xml impl library?

This bug is a big problem and repeatably kills other servlets. Everything seems to work at first, until GeoServer receives a WFS request with outputformat=GML2 or version=1.0.0, which triggers the system property change, and causes any other servlet relying on the platform TransformerFactory and not having a xalan jar to fail.

The fault is now at line 181-182 in org.geoserver.wfs.xml.GML2OutputFormat.prepare() on trunk.

        System.setProperty("javax.xml.transform.TransformerFactory",
            "org.apache.xalan.processor.TransformerFactoryImpl");

You just cannot do this sort of thing in a servlet and expect to coexist with other servlets. It kills deegree running in the same Tomcat stone dead, because deegree was expecting to use the JDK 1.5 xalan (Sun's repackaged copy), and GeoServer just told everyone to use xalan 2.7.0, which it has, but nobody else has on their classpath. Anyone servlet using another TransformerFactory, such as Sun's repackaged copy of Xalan, or Saxon, is stuffed.

The workaround is to copy GeoServer's xalan jar into every other TransformerFactory-using servlet, so they can survive the property change.

There is one sane alternative: use SPI:
http://java.sun.com/j2se/1.4.2/docs/api/javax/xml/transform/TransformerFactory.html#newInstance%28%29
The third option is to use an SPI resource:
META-INF/services/javax.xml.transform.TransformerFactory
This looks like the only servlet-friendly option.

And GeoServer 1.5.4 also kills GeoNetwork running in the same context. The workaround was to run two separate Tomcat 5.5 instances on a production machine.

If I remove the System.setProperty(), all unit tests pass (on trunk).

That line was introduced eons ago while GeoTools was still targeting java 1.4, which afaik did not include xlst support built in. I think it's safe to remove but I'd like to hear from the xml experts before doing any change

Attached patch removes the offending lines. I do not know which implementation is used (Sun JDK or xalan.jar). If this concerns you, add an SPI entry.

Andrea, I am sure you are right. What is more, the packaging changed over time as Sun did their usual copy-freeze-retreat-from-community-outrage (see also xerces, crimson ...).

It would be nice to get this into 1.7.6. I have tagged it thus. Feel free to untag if you are happy to make another GeoServer release that kills GeoNetwork or deegree running in the same Tomcat instance.

Ben, GeoNetwork has been bundling GeoServer into their releases for at least two years. I guess they must be doing something to prevent the problem? (maybe they just use Jetty, don't know).

As for the 1.7.x series, I would go for the spi thing on 2.5.x, seems safer, and just kill the setProperty without any other change on trunk?

Andrea, the GeoNetwork known to be affected was the Bluenet MEST version. It also bundled GeoServer. It would only have been affected if it used GeoServer for WFS GML2. WMS would not have been affected. If Jetty contains the xalan jars, they would also have been OK. It is very sensitive to what is in the bundle.

I have no objection to including SPI on both 1.7.x and trunk. I am testing trunk now to see tests pass with SPI.

Updated patch adds SPI for xalan TransformerFactory implementation.

Andrea, I can confirm that all GeoServer trunk tests pass when the System.setProperty() statement is removed and an SPI entry for the xalan jar is added. wfs has a transitive dependency via main on xalan, so the jar is available at test runtime.

Put back the original patch without SPI.

Andrea, I removed the SPI from the patch. An SPI entry in wfs is not needed because the required SPI entry is included in xalan-2.7.0.jar, which is on the classpath.

I checked GeoServer 1.7.5 and it is also bundled with xalan-2.7.0.jar. Although I have not tested it with the change, this makes me suspect very strongly that 1.7.x should be fixed by the same patch used for trunk, without any additional SPI.

Subject: Re: [Geoserver-devel] Xalan TransformerFactory system property change
Date: Thu, 23 Jul 2009 10:08:45 +0800
From: Justin Deoliveira
To: Caradoc-Davies, Ben (E&M, Kensington)
CC: Geoserver-devel, Andrea Aime
References: <4A66BDE1.8000607@csiro.au>

Thankfully this time something XML related is not my fault . I am not
sure why that line is there... but +1 on removing it. It would be good
if we could verify that it does not break anything, testing cite in a
servlet container and in jetty.

Ben Caradoc-Davies wrote:
> Justin,
>
> is there any reason why we should not get rid of a troublesome Xalan
> System.setProperty in GML2OutputFormat?
> http://jira.codehaus.org/browse/GEOS-1094
>
> Kind regards,
>

–
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.

With Justin we agreed that the change is too close to the release and thus we won't apply it for 1.7.6
But we'll apply it on trunk and see how it plays.

Could you apply it on 1.7.x branch please ?

Bouiaw, the 1.7.x series is dead, we won't make releases unless there is specific funding to do so. But you can still checkout the soruces, apply the patch youself and build GeoServer

Note that this patch was applied on trunk in r12942 by Andrea, fixing 2.0.x and later.