RFR 8247960: jarsigner says "signer errors" for some normal warnings when -strict is set

[Hai-May Chao]

> On Jul 24, 2020, at 6:54 AM, Weijun Wang <weijun.wang at oracle.com> wrote:
> 
> I'd suggest changing
> 
> +        result = isSigning ? rb.getString("jar.signed.") : rb.getString("jar.verified.");
> +        if ((strict) && (!errors.isEmpty())) {
> +            result = isSigning ? rb.getString("jar.signed.with.signer.errors.")
> +                     : rb.getString("jar.verified.with.signer.errors.");
> +        }
> 
> to
> 
> +        if ((strict) && (!errors.isEmpty())) {
> +            result = isSigning ? rb.getString("jar.signed.with.signer.errors.")
> +                     : rb.getString("jar.verified.with.signer.errors.");
> +        } else {
> +            result = isSigning ? rb.getString("jar.signed.") : rb.getString("jar.verified.");
> +        }
> 

Webrev updated as suggested.

> Everything else looks fine.
> 
> Also, I remember you meant to fix 2 bugs with a single changeset. What should the full commit message be?

Fix in a single changeset, so use this bug as the commit message please.

Thanks,
Hai-May

> 
> Thanks,
> Max
> 
>> On Jul 24, 2020, at 4:25 PM, Hai-May Chao <hai-may.chao at oracle.com> wrote:
>> 
>> 
>> 
>>> On Jul 15, 2020, at 12:16 AM, Weijun Wang <weijun.wang at oracle.com> wrote:
>>> 
>>> The following lines are useless now:
>>> 
>> 
>> Ok, this is a separate problem from the original bug to be addressed. Cleanup/refactoring of the checking on flags is also included in this changeset.
>> 
>>> 1053         if (badKeyUsage || badExtendedKeyUsage || badNetscapeCertType ||
>>> 1054                 notYetValidCert || chainNotValidated || hasExpiredCert ||
>>> 1055                 hasUnsignedEntry || signerSelfSigned || (legacyAlg != 0) ||
>>> 1056                 (disabledAlg != 0) || aliasNotInStore || notSignedByAlias ||
>>> 1057                 tsaChainNotValidated ||
>>> 1058                 (hasExpiredTsaCert && !signerNotExpired)) {
>>> 
>>> 1198         }
>>> 
>>> 1205         if (hasExpiringCert ||
>>> 1206                 (hasExpiringTsaCert  && expireDate != null) ||
>>> 1207                 (noTimestamp && expireDate != null) ||
>>> 1208                 (hasExpiredTsaCert && signerNotExpired)) {
>>> 
>>> 1245         }
>>> 
>>> I would even suggest you remove the "result" variable and move the "System.out.println(result)" line into branches of the if-else block on lines 1254-1272.
>> 
>> Current change has the checking for sign and verify. Keep it as-is that you agreed.
>> 
>> https://cr.openjdk.java.net/~hchao/8247960/webrev.01/
>> 
>> Thanks,
>> Hai-May
>> 
>> 
>>> 
>>> No other comments.
>>> 
>>> Thanks,
>>> Max
>>> 
>>> 
>>> 
>>>> On Jul 15, 2020, at 4:09 AM, Hai-May Chao <hai-may.chao at oracle.com> wrote:
>>>> 
>>>> Hi,
>>>> 
>>>> I’d like to request a review for:
>>>> 
>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8247960
>>>> Webrev: https://cr.openjdk.java.net/~hchao/8247960/webrev.00/
>>>> 
>>>> Jarsigner is changed to emit “with signer errors” only when there are errors detected during sign and verify with -strict specified.
>>>> 
>>>> Thanks,
>>>> Hai-May
>