[jdk17u-dev] RFR: 8302111: Serialization considerations [v2]

[Richard Reingruber]

On Thu, 20 Feb 2025 08:59:41 GMT, Goetz Lindenmaier <goetz at openjdk.org> wrote:

>> I backport this to match 17.0.13-oracle based on the commit to 21.
>> 
>> I had to resolve several files, two of them considerably:
>> 
>> src/java.base/share/classes/com/sun/crypto/provider/DHPrivateKey.java
>>   Resolved larger chunk.
>> src/java.base/share/classes/com/sun/crypto/provider/DHPublicKey.java
>>   Resolved some code.
>> 
>> The new code in DHPublic/PrivateKey depends on the removal
>> of IOExceptions in [JDK-8297065](https://bugs.openjdk.org/browse/JDK-8297065) "DerOutputStream operations should not throw IOExceptions". 
>> This change is in 21 but not in 17. Thus code backported from 21 calls functions of 
>> DerOutputStream that do not throw an exception in 21, but do so in 17. 
>> JDK-8297065 makes the point that these exceptions can never be thrown, which also
>> holds for 17.  So I just catch and ignore them.
>> 
>> Also, I had to adapt code because an exception constructor with cause for
>> InvalidObjectException is missing in 17.
>> This was added by [JDK-8282696](https://bugs.openjdk.org/browse/JDK-8282696) "Add constructors taking a cause
>> to InvalidObjectException and InvalidClassException" in 19.
>> I added a call to initCause() to store the causing exception.
>> 
>> I double-checked that [JDK-8297065](https://bugs.openjdk.org/browse/JDK-8297065) was not backported
>> by Oracle. Backporting this change would simplify matters considerably.
>> 
>> The remaining files only needed trivial resolves:
>> 
>> src/java.base/share/classes/java/security/Permissions.java
>> src/java.base/share/classes/java/security/SignedObject.java
>>   Only Copyright.
>> src/java.base/share/classes/java/security/Timestamp.java
>>   Copyright and import.
>> src/java.base/share/classes/java/security/UnresolvedPermissionCollection.java
>> src/java.base/share/classes/java/security/cert/CertificateRevokedException.java
>> src/java.base/share/classes/sun/security/provider/DRBG.java
>> src/java.base/share/classes/sun/security/util/ObjectIdentifier.java
>> src/java.base/share/classes/sun/security/x509/AlgIdDSA.java
>>   Only Copyright.
>> src/java.security.jgss/share/classes/sun/security/jgss/krb5/Krb5Context.java
>>   Resolved imports.
>> src/java.security.jgss/share/classes/sun/security/jgss/krb5/Krb5InitCredential.java
>> src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11SecureRandom.java
>>   Only Copyright.
>> 
>> I manually ran these tests on linux x86_64:
>> 
>> test/jdk/com/sun/security
>> test/jdk/java/awt/security
>> test/jdk/java/net/httpclient/security
>> test/jdk/java/net/httpclient/we...
>
> Goetz Lindenmaier has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Remove mentioning of ProviderException

Changes requested by rrich (Reviewer).

src/java.base/share/classes/com/sun/crypto/provider/DHPublicKey.java line 229:

> 227:         } catch (IOException e) {
> 228:             throw new ProviderException("Cannot produce ASN.1 encoding", e);
> 229:         }

You should remove the try-catch. `encode` doesn't throw IOE. This also removes the diff to jdk 21.

src/java.base/share/classes/com/sun/crypto/provider/DHPublicKey.java line 384:

> 382:             if (ioe != null) {
> 383:                 ioe.initCause(e);
> 384:             }

The null check is redundant.
Suggestion:

            ioe.initCause(e);

-------------

PR Review: https://git.openjdk.org/jdk17u-dev/pull/3278#pullrequestreview-2630339484
PR Review Comment: https://git.openjdk.org/jdk17u-dev/pull/3278#discussion_r1963842849
PR Review Comment: https://git.openjdk.org/jdk17u-dev/pull/3278#discussion_r1963850223