RFR [11] (JAXP): 6857903: SAXException.initCause() does not correctly set Exception

Jason Mehrens jason_mehrens at hotmail.com
Tue Feb 6 15:25:51 UTC 2018


I missed this before but, it looks like we are calling the public getException method in writeObject.  We should create a private method to service both the public getException method and writeObject to guard against subclass tampering.

Everything else looks good.

From: Aleks Efimov <aleksej.efimov at oracle.com>
Sent: Monday, February 5, 2018 8:46 PM
To: Jason Mehrens
Cc: core-libs-dev
Subject: Re: RFR [11] (JAXP): 6857903: SAXException.initCause() does not correctly set Exception

Thank you for your comments, Jason!
New webrev can be found at this location:

The list of changes:
- this.initCause() -> super.initCause()
- 'readObject' has been modified to convert IllegalStateException to
InvalidClassException. The test case that triggers IllegalStateException
in SAXException::readObject has been added to
'testReadObjectIllegalStateException' test method.

Best Regards,

On 02/05/2018 05:09 PM, Jason Mehrens wrote:
> Aleksei,
> Looks good.  We should avoid this.initCause in readObject and prefer super.initCause so subclasses can't alter the deserialization behavior.
> While I can't think of a case off the top of my head, I would prefer that we trap and convert IllegalStateException into a InvalidClassException in readObject.
> That keeps the readObject contract intact in case something malicious is going on.  That is just my preference though.
> Jason
> ________________________________________
> From: Aleks Efimov <aleksej.efimov at oracle.com>
> Sent: Monday, February 5, 2018 10:51 AM
> To: Jason Mehrens; 'Roger Riggs'
> Cc: core-libs-dev
> Subject: Re: RFR [11] (JAXP): 6857903: SAXException.initCause() does not correctly set Exception
> Hi Roger, Jason,
> I've tried to avoid doing the same stuff as I did for XPathException to
> minimize the changes to the SAXException class. But I was naive to think so:
> Tried to keep the exception field in place to avoid modifications
> required to keep serial form intact (similar as it was done in
> JDK-8009581 bug mentioned by Jason), but I missed the 'initCause' method
> (spotted by Roger, thank you!) that can make cause/exception values
> inconsistent. To avoid the API modification and for the sake of clarity
> I proceeded with removal of the 'exception' field. The new version of
> the fix:
> - Removes 'exception' field
> - Maintains the serial form of the SAXException class
> - Handles the difference in SAXException:exception and Throwable:cause
> types 'SAXException::getException', i.e. SAXException:exception is
> Exception typed and Throwable:cause is Throwable typed.
> - Avoids any changes to API and serial form of the class, but
> maintaining it similar to JDK-8009581)
> - There is a copy of SAXException class in java.base module that is
> required for j.u.Properties::loadFromXML, it has been update with the
> same changes.
> New regression test has been added to test:
> - Serial compatibility with legacy JDKs (similar to JDK-8009581)
> - usages of SAXException::initCause/getException
> Webrev with the fix and new regression:
> http://cr.openjdk.java.net/~aefimov/6857903/dev/01/
> Testing shows no JCK/regression tests failures with the proposed fix.
> Best Regards,
> Aleksei
> On 12/21/2017 07:00 PM, Jason Mehrens wrote:
>> Aleksei,
>> You have to override all of the constructors to always disable initCause.  Actually a similar issue was covered in:
>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-May/016908.html
>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-June/017594.html
>> Pretty much everything was hashed out in those threads.
>> Here is the final webrev for that:
>> http://cr.openjdk.java.net/~dmeetry/8009581/webrev.5/jaxp/src/javax/xml/xpath/XPathException.java.udiff.html
>> That should be a good cookbook for changes and tests required for this issue.  Note that it gets rid of the Exception field and maintains serial compatibility.
>> Looking back on that change, I would have changed it so the InvalidClassException added the double cause as suppressed exceptions.
>> Jason
>> ________________________________________
>> From: core-libs-dev <core-libs-dev-bounces at openjdk.java.net> on behalf of Aleks Efimov <aleksej.efimov at oracle.com>
>> Sent: Thursday, December 21, 2017 11:27 AM
>> To: core-libs-dev
>> Subject: RFR [11] (JAXP): 6857903: SAXException.initCause() does not correctly set Exception
>> Hello,
>> Please, help to review the fix for jaxp bug that fixes SAXException to
>> correctly set exception cause with 'setCause' method:
>> http://cr.openjdk.java.net/~aefimov/6857903/dev/00/
>> I've tried to keep the fix miminal with respect to serial form of this
>> API class, i.e. kept private 'exception' field.
>> The new test case was added to IssueTracker56Test unit test. Testing
>> showed no related JCK/JTREG failures.
>> With Best Regards,
>> Aleksei

More information about the core-libs-dev mailing list