Re: Patch: FYI: disable XML service files

[Andrew Haley <aph at redhat dot com>]

Gary Benson writes:
 > Tom Tromey wrote:
 > > I'm checking this in on the trunk and the RH 4.1 branch.
 > > 
 > > This removes the XML service files from libgcj.so.  This lets us
 > > override things properly again.
 > > 
 > > Andrew, I tried your test case from ifoox, but I can only see the
 > > failure if I back out some of the latest XML bug fixes.  However a
 > > simpler test case shows which thing we're actually loading:
 > > 
 > >     import javax.xml.parsers.*;
 > >     public class q {
 > >       public static void main(String[] args) throws Throwable{
 > >         DocumentBuilderFactory tf = DocumentBuilderFactory.newInstance();
 > >         System.out.println(tf.getClass());
 > >       }
 > >     }
 > > 
 > > Eg, here's the bug in action:
 > > 
 > > opsy. gij -Djava.class.path=/usr/share/java/xerces-j2.jar:. q
 > > class gnu.xml.dom.DomDocumentBuilderFactory
 > 
 > This is actually the correct behaviour: xerces should only be loaded
 > if it is endorsed, and merely being in the classpath should have no
 > effect.  Check this test case with Sun or IBM java:
 > 
 >   import javax.xml.parsers.DocumentBuilderFactory;
 >   public class Test {
 >     public static void main(String[] args) throws Throwable{
 >       DocumentBuilderFactory f = DocumentBuilderFactory.newInstance();
 >       f.setAttribute("bad_attribute", "hello");
 >     }
 >   }

IMO that's the wrong test.  What matters is this not the
DocumentBuilderFactory returned, but the DocumentBuilder, and here
what we do is clearly wrong:

import javax.xml.parsers.DocumentBuilderFactory;
public class Test1 {
  public static void main(String[] args) throws Throwable{
    DocumentBuilderFactory f = DocumentBuilderFactory.newInstance();
    System.out.println(f.newDocumentBuilder());
    }
}

 $ ~/jdk1.5.0_09/bin/java Test1
com.sun.org.apache.xerces.internal.jaxp.DocumentBuilderImpl@18a47e0

 $ ~/jdk1.5.0_09/bin/java -classpath /usr/share/java/xerces-j2.jar:. Test1
org.apache.xerces.jaxp.DocumentBuilderImpl@7d8483


The old gcj did The Right Thing:

 $ gij Test1
gnu.xml.dom.DomDocumentBuilder@3181dc15

 $ gij -classpath /usr/share/java/xerces-j2.jar:. Test1
org.apache.xerces.jaxp.DocumentBuilderImpl@2cfe98d5


Without tromey's patch the new gcj does the Wrong Thing:

 $ ~/gcc/trunk/install/bin/gij Test1
gnu.xml.dom.DomDocumentBuilder@318eaa95
 $ ~/gcc/trunk/install/bin/gij -classpath /usr/share/java/xerces-j2.jar:. Test1
gnu.xml.dom.DomDocumentBuilder@31992c35


And with his patch, the Right Thing:

 $ ~/gcc/trunk/install/bin/gij Test1
gnu.xml.dom.DomDocumentBuilder@318ebff5

 $ ~/gcc/trunk/install/bin/gij -classpath /usr/share/java/xerces-j2.jar:. Test1
org.apache.xerces.jaxp.DocumentBuilderImpl@31918a55


We don't use Xerces so we can't be compatible with all behaviour in
every possible case, but this is far more like to affect
user-observable behaviour.

What we had before did the right thing for the wrong reason.
	    
 > You can't just print the DocumentBuilderFactory here because Sun and
 > IBM both use xerces internally, but causing an exception lets you see
 > the line numbers and figure out where it's loading from:
 > 
 >   mambo:[~]$ ibmjava Test
 >   Exception in thread "main" java.lang.IllegalArgumentException: bad_attribute
 >           at org.apache.xerces.jaxp.DocumentBuilderFactoryImpl.setAttribute(Unknown Source)
 >           at Test.main(Test.java:7)
 > 
 >   mambo:[~]$ ibmjava -cp /usr/share/java/xerces-j2.jar:. Test
 >   Exception in thread "main" java.lang.IllegalArgumentException: bad_attribute
 >           at org.apache.xerces.jaxp.DocumentBuilderFactoryImpl.setAttribute(Unknown Source)
 >           at Test.main(Test.java:7)
 > 
 >   mambo:[~]$ ibmjava -Djava.endorsed.dirs=/var/lib/tomcat5/common/endorsed Test
 >   Exception in thread "main" java.lang.IllegalArgumentException: Property 'bad_attribute' is not recognized.
 >           at org.apache.xerces.jaxp.DocumentBuilderFactoryImpl.setAttribute(DocumentBuilderFactoryImpl.java:112)
 >           at Test.main(Test.java:7)
 > 
 > Just being in the classpath has the same behaviour as not being there
 > at all.  gcj had the correct behaviour before the service files were
 > removed.

I don't think this is true.  

The logic in GNU Classpath's DocumentBuilderFactory.newInstance() that
searches for an available parser can't possibly return anything other
than the built-in one if the XML property files are included, because
the class loader logic will always find the built-in one first.

With IBM, Sun, etc, all that a user has to do to use their own version
of Xerces is put it into the classpath, and that's what people do.  It
isn't necessary to put the it in endorsed, although that would be good
practice.

Andrew.