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

[Alexey Bakhtin]

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