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

[Alexey Bakhtin]

Gentle ping

> On 27 May 2021, at 20:11, Alexey Bakhtin <alexey at azul.com> wrote:
> 
> 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
>> 
>