7081804: Remove cause field from javax.xml.crypto.NoSuchMechnismException

[Sebastian Sickelmann]

Am 22.08.2011 17:28, schrieb Sean Mullan:
> (dropping core-libs-dev)
>
> On 8/22/11 9:03 AM, Sebastian Sickelmann wrote:
>> Hi,
>>
>> while making some change for using exception-chaining on RuntimeException in
>> more cases, i found that javax.xml.crypto.NoSuchMechnismException had a private
>> cause field that isn't necessary since throwable can handle it. But just
>> removing isn't that simple as Alan pointed out[1].
> The history behind this is that this API is part of JSR 105. JSR 105 was
> released independently of Java SE as a "standalone JSR" and later integrated
> into Java SE 6 as part of the platform JSR. Thus as I understand, all JSR 105
> API changes need to first go through a maintenance revision of JSR 105.
>
> JSR 105 was designed to be used by applications using JDK 1.2 and up, thus there
> are no API dependencies on any releases later than that. Therefore, this class
> (and other exception classes in JSR 105) included their own methods to capture
> the cause.
>
> I've been aware of this issue for a little while but I did not have the cycles
> to analyze it thoroughly so I am glad you are looking into it.
>
> I think you will find the same issue in the other Exception classes in the
> javax.xml.crypto package and its subpackages. Have you looked at those yet?
>
> I am not sure if removing the 3 overloaded methods qualifies as an API change.
> If so, it would first require it go through a maintenance JSR. I will need to
> ask someone else about that and get back to you.
>
>> First of all i thought just changing the serialversionUID was the right
>> solution, but Alan disagreed and i actually think that too. Unfortunatly Alan
>> doen't had the time to explain it in detail, but i think that we must search for
>> a solution that doen't break backward serialization compatability.
>>
>> I created a webrev for my suggested change here:
>> http://oss-patches.24.eu/openjdk8/NoSuchMechanismException/REBASED_ON_8018d541a7b2/
>>
>> In a following post Alan writes that this should best be discussed here. What is
>> the hg repository for patches in this mailinglist? I don't see any security-dev
>> related repository for openjdk8 in http://hg.openjdk.java.net/ the patch is
>> based on ***<http://hg.openjdk.java.net/jdk8/tl/jdk/>*jdk8/tl/jdk maybe i
>> should rebase it onto another repository.
>>
>> Another problem i have is testing. I think we should add some test for this. But
>> got no real clue how to do it right. I tested it manually with the source
>> here[2] and the files in here[3].
>>
>> The files are created with an openjdk7 build(Unfortunatly i think with an quite
>> old private build) and with an openjdk8(with my patches to
>> NoSuchMechanismException). I tested it manually with my openjdk 7 and 8 build
>> and in both cases the serialized objects could be loaded correctly in all cases.
>>
>> What is the best way to test this automatically. I thought about checking if the
>> newly created openjdk8 serialization is binary equal to the jdk7 output, but it
>> isn't because of some unavoidable changes (ex. jdk8 has the
>> custom-serialization-bit)
>>
>> Alan mentioned that it may be ok to embed some serialized-objects as byte[] in
>> testcode.
> Yes, that is probably the best solution. I remember I had created some tests for
> this a while back. I'll see if I can find them.
>
>> I will try to create a jtreg for this. Hope to be back on this soon. In the
>> meantime it would be great to have someone reviewed the webrev.
> I am pretty busy but will try to find some cycles to review it more carefully in
> the next couple of days.
>
> --Sean
>
Some discussions went on in this thread as i tried to apply this 
solution to some other classes.
These classes has adressed the new chaining support in Throwable quite 
different.
In the meanwhile i think it is done best the way it is done in 
java/lang/{UndeclaredThrowableException|InvocationTargetException|ClassNotFoundException}

I changed the webrev to mirror this pattern of supporting exception 
chain from java/lang.
The solution is far easier to understand and i think it needs no 
addtional regression-tests.
The solution to delete the cause field and patch serialisation is far 
more complex.
I like the deletion of this field much more like prohibiting initCause. 
But this change should be mirrored to many Exceptions out there, and 
would require many discussion.

Here is the updated webrev:
http://oss-patches.24.eu/openjdk8/NoSuchMechanismException/7011804_0/

Maybe the title of the bug isn't right anymore.
@Alan: Can we change it into something like prohibit unintentional 
initCause() after construction?

-- Sebastian
>> @core-libs-dev: I crosspost it to core-libs-dev because the thread started
>> there. So the interessted parties can move over to security-dev archive[4]
>>
>> -- Sebastian
>>
>> [1] http://mail.openjdk.java.net/pipermail/core-libs-dev/2011-August/007462.html
>> [2] http://oss-patches.24.eu/openjdk8/NoSuchMechanismException/SeriallizeTest.java
>> [3] http://oss-patches.24.eu/openjdk8/NoSuchMechanismException/
>> [4] http://mail.openjdk.java.net/pipermail/security-dev/2011-August/thread.html