[jdk17u-dev] RFR: 8305972: Update XML Security for Java to 3.0.2 [v5]

[Martin Balao]

On Mon, 8 Jan 2024 11:51:26 GMT, Goetz Lindenmaier <goetz at openjdk.org> wrote:

>> Hi Goetz,
>> 
>> Thanks for proposing this backport.
>> 
>> One minor comment:
>> 
>>  * test/jdk/javax/xml/crypto/dsig/GenerationTests.java
>>    * The import of X509Certificate and the definition of x5ks are dead code.
>> 
>> Otherwise, looks good to me. I'll give my approval anyways, as this is a minor comment.
>> 
>> On a final note, I agree with your changes of removing EdDSA code and aligning to the approved CSR. However, I have to say that doing this for 17u does not only prevent users from the enhancement but also increases the maintenance cost as there will be more chances of updates not applying cleanly for the years to come. I would have treated 17u differently than previous releases.
>> 
>> Martin.-
>
> Hi @martinuy,
> do you think it would be better to keep all the code and only remove the two String and the change to the comment of DigestMethod?
> In the 21u change for the update to [3.0.3](https://github.com/openjdk/jdk21u-dev/pull/94) I have done it that way. Probably that's also the better solution here.

Hi @GoeLin ,

I assume that in 21u you kept all the (implementation) code except for the public members. If so, I understand the motivations but personally prefer what you proposed for 17u in this PR. It makes the code more clear in terms of what is supported. For example, it would be misleading for someone who looks for "EdDSA" references in the code, finds many —even beyond defines— and assumes that it is supported. This is, of course, at the expense of higher chances of non-clean updates. We have taken this approach in other libraries before such as when we removed the implementation of DTLS in the 8u backport of the TLS engine.

Martin.-

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

PR Comment: https://git.openjdk.org/jdk17u-dev/pull/2006#issuecomment-1881457955