Request for review Remove "private" cause in jdk exceptions
sebastian.sickelmann at gmx.de
Fri Aug 26 03:59:45 PDT 2011
Am 26.08.2011 08:32, schrieb Peter Jones:
> On Aug 25, 2011, at 8:00 PM, Sebastian Sickelmann wrote:
>> Am 26.08.2011 00:24, schrieb Sebastian Sickelmann:
>>> Am 26.08.2011 00:03, schrieb Sebastian Sickelmann:
>>>> I have found more places in jdk source where an Exception has a private cause field.
>>>> share/classes/javax/management/remote/JMXProviderException.java: private Throwable cause = null;
>>>> share/classes/javax/xml/crypto/KeySelectorException.java: private Throwable cause;
>>>> share/classes/javax/xml/crypto/NoSuchMechanismException.java: private Throwable cause;
>>>> share/classes/javax/xml/crypto/MarshalException.java: private Throwable cause;
>>>> share/classes/javax/xml/crypto/dsig/XMLSignatureException.java: private Throwable cause;
>>>> share/classes/javax/xml/crypto/dsig/TransformException.java: private Throwable cause;
>>>> share/classes/javax/xml/crypto/URIReferenceException.java: private Throwable cause;
>>>> 7081804 handles NoSuchMechanismException.
>>>> Is there a way to expand it to at least the xml/crypto/**/* Exceptions?
>>>> JMXProviderException should be fine too.
>>>> I would create a CR for the changes to me made to change and test this.
>>>> Would it be good to have some utility-code in Throwable to don't introduce to much code-duplication?
>>>> -- Sebastian
>>> After a very quick analysis i think i found more candidates for removing private causes.
>>> share/classes/javax/security/sasl/SaslException.java: private Throwable _exception;
>>> share/classes/java/lang/reflect/UndeclaredThrowableException.java: private Throwable undeclaredThrowable;
>>> share/classes/java/lang/reflect/InvocationTargetException.java: private Throwable target;
>>> share/classes/java/lang/ClassNotFoundException.java: private Throwable ex;
>>> share/classes/com/sun/java/browser/dom/DOMAccessException.java: private Throwable ex;
>>> share/classes/com/sun/java/browser/dom/DOMUnsupportedException.java: private Throwable ex;
>>> share/classes/javax/naming/NamingException.java: protected Throwable rootException = null;
>>> share/classes/java/rmi/RemoteException.java: public Throwable detail;
>>> share/classes/java/rmi/activation/ActivationException.java: public Throwable detail;
>>> Some of them need deeper inspection. Some of them are the same as the above noted Exceptions in xml/crypto package.
>>> - Sebastian
>> OK. Webrev is there: http://oss-patches.24.eu/openjdk8/NoSuchMechanismException/REBASED_ON_8018d541a7b2_2/
>> Can someone review this?
> Public fields like RemoteException.detail, ill-advised as they may have been, cannot be removed (would break binary compatibility).
Sorry for that. It was more a reflex (remove "evil" public fields) than
a real problem with this. Breaking this would breaking binary
compatibility but can this be a real show-stopper for not fixing this?
But you are right. Changes with binary incompatibility should be done
> This change proposes additions to the public API (subclass interface) of java.lang.Throwable, which would need additional approval process. Without getting into detailed design critique, the proposed protected methods seem to add subtle complexity to the subclass interface of this central class, which leads me to:
> Is there a particular problem that these changes are attempting to address? Many of these exception classes had cause-like fields prior to the addition of Throwable.cause in 1.4, and as described in their class docs, for 1.4 they were each retrofitted, with some care, to work well with the general cause APIs added to Throwable. (Also see the doc for Throwable.getCause, which describes how subclasses can override it to take responsibility for their causes.)
> These changes seem to be about implementing an alternate approach to that retrofitting, with considerably higher complexity (serialized form compatibility code) and risk, and I don't understand why-- I don't see a motivation discussed earlier on core-libs-dev?
Discussion started at core-libs-dev in another threads. The
main-reason was to chain more Exceptions together where possible. I
changed the NoSuchMechanismException in javax.xml.crypto while try to
chain more Exceptions that extends RuntimeException.
I just removed the field, Alan noted that this would change to
compatibility and said that this CR should be discussed at security-dev.
Additionally he created a bug "7081804: Remove cause field from
The changes for chained-exceptions in Throwable made to
javax/xml/crypto/*Exceptions are not done so well as in the other cases
here. So i tried to fix it like here.
Alternatively i could fix javax/xml/crypto by adopting the solution used
in the other cases.
The question i have is what is the best solution that should used.
Changing the serialization thought custom serialization or through using
the solution used in the other cases? I think we should only have one
solution to address this.
> -- Peter
More information about the security-dev