[8u] RFR: 8206189: sun/security/pkcs12/EmptyPassword.java fails with Sequence tag error

[Alexey Bakhtin]

Hi Andrew,

I have updated the webrev with patches generated by "hg diff” :
11u patch for JDK-8206189: http://cr.openjdk.java.net/~abakhtin/8206189/webrev.v0/8206189.11u.patch
8u patch for JDK-8206189: http://cr.openjdk.java.net/~abakhtin/8206189/webrev.v0/8206189.8u.patch
diff between patches: http://cr.openjdk.java.net/~abakhtin/8206189/webrev.v0/11uto8u.diff

Regards
Alexey

> On 18 May 2021, at 16:39, Alexey Bakhtin <alexey at azul.com> wrote:
> 
> Hi Andrew,
> 
> Thank you for review.
> It looks like an issue of the diff generated by webrev tool
> The difference between 11u and 8u patches generated by hg diff command is minimal and expected:
> 
> < diff -r 53b0d5ad71db -r 038688fa32d0 src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java
> < --- a/src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java	Wed Jul 11 14:54:35 2018 -0700
> < +++ b/src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java	Thu Jul 12 08:44:39 2018 +0800
> < @@ -296,6 +296,9 @@
> ---
>> diff -r c44b864509e2 src/share/classes/sun/security/pkcs12/PKCS12KeyStore.java
>> --- a/src/share/classes/sun/security/pkcs12/PKCS12KeyStore.java	Thu May 06 19:00:51 2021 +0300
>> +++ b/src/share/classes/sun/security/pkcs12/PKCS12KeyStore.java	Mon May 17 21:19:06 2021 +0300
>> @@ -380,68 +383,72 @@
> 47a48,50
>> +                if (value.length < 1 || value.length > 2) {
>> +                    throw new IOException("Invalid length for AlgorithmIdentifier");
>> +                }
> 99a103,105
>> -            if (value.length < 1 || value.length > 2) {
>> -                throw new IOException("Invalid length for AlgorithmIdentifier");
>> -            }
>> @@ -1992,14 +1998,13 @@
> 156c162
> <              if (contentType.equals(ContentInfo.DATA_OID)) {
> ---
>>             if (contentType.equals((Object)ContentInfo.DATA_OID)) {
> 164c170
> <              } else if (contentType.equals(ContentInfo.ENCRYPTED_DATA_OID)) {
> ---
>>             } else if (contentType.equals((Object)ContentInfo.ENCRYPTED_DATA_OID)) {
> 
> Regards
> Alexey
> 
>> On 17 May 2021, at 17:11, Andrew Hughes <gnu.andrew at redhat.com> wrote:
>> 
>> On 10:15 Mon 17 May     , Alexey Bakhtin wrote:
>>> Hello,
>>> 
>>> Please review the backport of DK-8206189 to 8u
>>> This is the second backport in the series required for JDK-8076190 [1]
>>> 
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8206189
>>> Original patch: http://hg.openjdk.java.net/jdk/jdk11/rev/038688fa32d0
>>> 8u webrev: http://cr.openjdk.java.net/~abakhtin/8206189/webrev.v0/
>>> 
>>> Original patch applied almost clean except for two differences:
>>> - one of the blocks is not applied cleanly because JDK-8234042 [1] already applied and JDK8 already verifies certValues length
>>> - JDK8 has a strict class cast to Object which is not necessary for JDK11
>>> 
>>> The initial review has been posted on [2]
>>> 
>>> [1] https://hg.openjdk.java.net/jdk8u/jdk8u-dev/jdk/rev/3054a00b5333
>>> [2] https://mail.openjdk.java.net/pipermail/jdk8u-dev/2021-May/013799.html
>>> 
>>> Regards
>>> Alexey
>> 
>> There seems to be something odd going on with the whitespace in this patch,
>> regarding the block that didn't apply cleanly.
>> 
>> The 11u patch has a single addition block and a single removal block for
>> the large hunk beginning "Parse the key algorithm and then use a JCA key factory".
>> 
>> The 8u patch seems to be broken up between the two, which makes them hard
>> to compare. In particular, there is the removal:
>> 
>> -            }, password);
>> 
>> and then subsequent re-addition of:
>> 
>> +            }, password);
>> 
>> Do the 11u & 8u versions of the file look the same after patching?
>> 
>> The 8u-only block from JDK-8234042 is:
>> 
>> +                if (value.length < 1 || value.length > 2) {
>> +                    throw new IOException("Invalid length for AlgorithmIdentifier");
>> +                }
>> 
>> I determined this by comparing the two patches line by line, but
>> it should really be clearer from the patch.
>> 
>> Thanks,
>> --
>> Andrew :)
>> 
>> Senior Free Java Software Engineer
>> OpenJDK Package Owner
>> Red Hat, Inc. (http://www.redhat.com)
>> 
>> PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
>> Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222
>