test integration -> Re: [Bug 100017] XML encoder can cause a StackOverflowError

[Joe Wang]

Joe,

The patch, contributed by Andrew, was integrated into OpenJDK7, which 
Andrew confirmed. According to Kelly, he applied the same source tarball 
to OpenJDK6. So we need to find out why jdk6-jaxp-2009_10_27.zip did not 
have this fix.

About tests, we do have unit test for every fix in jaxp, and for this 
particular bug, we've used the testcase Andrew contributed. Bill had 
asked me to upload all of the unit tests along with source tarball. 
According to Bill, his team will take over from there and integrate them 
into the jdk workspace.

Thanks,
Joe

Joe Darcy wrote:
> Hello.
>
> Andrew John Hughes wrote:
>> On 09:50 Fri 15 Jan     , Joe Wang wrote:
>>> Thanks Alan. Yes, I did receive the bugzilla notice.
>>>
>>> Kelly told me that he applied the same jaxp source tarball to 
>>> OpenJDK6. Kelly, could you tell Andrew how to include the jaxp 
>>> source tarball when building OpenJDK6?
>>>
>>
>> The tarball jdk6-jaxp-2009_10_27.zip used by the OpenJDK6 build does not
>> include the fix.  We are still having to apply the patch:
>>
>> diff -Nru openjdk.orig/jaxp/build.properties 
>> openjdk/jaxp/build.properties
>> --- openjdk.orig/jaxp/build.properties         2009-12-08 
>> 17:42:33.000000000 +0000
>> +++ openjdk/jaxp/build.properties         2009-12-08 
>> 17:43:03.000000000 +0000
>> @@ -73,6 +73,9 @@
>>  # Where patches to drop bundle sources live
>>  patches.dir=patches
>>  
>> +# Patches to apply
>> +jaxp_src.patch.list=xml-encodinginfo.patch
>> +
>>  # Sanity information
>>  sanity.info= Sanity Settings:${line.separator}\
>>    ant.home=${ant.home}${line.separator}\
>> diff -Nru openjdk.orig/jaxp/patches/jaxp_src/xml-encodinginfo.patch 
>> openjdk/jaxp/patches/jaxp_src/xml-encodinginfo.patch
>> --- openjdk.orig/jaxp/patches/jaxp_src/xml-encodinginfo.patch        
>> 1970-01-01 01:00:00.000000000 +0100
>> +++ openjdk/jaxp/patches/jaxp_src/xml-encodinginfo.patch        
>> 2009-12-08 17:41:58.000000000 +0000
>> @@ -0,0 +1,18 @@
>> +diff -Nru 
>> src/com/sun/org/apache/xml/internal/serializer/EncodingInfo.java 
>> src.new/com/sun/org/apache/xml/internal/serializer/EncodingInfo.java
>> +--- 
>> src/com/sun/org/apache/xml/internal/serializer/EncodingInfo.java        
>> 2009-10-27 21:54:16.000000000 +0000
>> ++++ 
>> src.new/com/sun/org/apache/xml/internal/serializer/EncodingInfo.java   
>> 2009-12-08 17:40:14.000000000 +0000
>> +@@ -326,9 +326,11 @@
>> +             m_last = last;
>> + +             // Set the range of unicode values that this object
>> +-            // explicitly manages
>> +-            m_explFirst = codePoint;
>> +-            m_explLast = codePoint + (RANGE-1);
>> ++            // explicitly manages. Align the explicitly managed values
>> ++            // to RANGE so multiple EncodingImpl objects dont 
>> manage the same ++            // values.
>> ++            m_explFirst = codePoint / RANGE * RANGE;
>> ++            m_explLast = m_explFirst + (RANGE-1);
>> + +             m_encoding = encoding;
>> + 
>
> Joe (Wang), I'll let you decide on the appropriateness of the fix for 
> jaxp.  If the fix is good, a new jaxp bundle for OpenJDK 6 would be 
> fine by me.
>
>> BTW, we have a testcase for this.  Would it be possible to add this to
>> the JDK tree?  The complete patch included in the icedtea6-hg tree
>> (http://icedtea.classpath.org/people/andrew/icedtea6-hg), including
>> test case, is attached.
>
> Tests are good of course and since the jaxp repo doesn't currently 
> have any regression tests, putting the test into OpenJDK 6 in the jdk 
> repo seems like the right home for it, as the patch currently does.
>
> Note that at the moment the license header of the test files says 
> "Copyright 2009 Red Hat" but later states "Please contact Sun 
> Microsystems ... if you need additional information or have any 
> questions."  If this goes upstream, the copyright holder should be 
> changed to Sun.
>
> Thanks,
>
> -Joe