RFR 8191137: keytool fails to format resource strings for keys for some languages after JDK-8171319

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR 8191137: keytool fails to format resource strings for keys for some languages after JDK-8171319

Weijun Wang
keytool contains a printf("%d-bit %s key", 1024, "RSA") call but when it's translated into French the call becomes printf("Clave %s de %d bits", 1024, "RSA") and %s does not match 1024.

The fix adds position parameters to printf resources strings when there are multiple arguments. The translations should follow.

diff --git a/src/java.base/share/classes/sun/security/tools/keytool/Resources.java b/src/java.base/share/classes/sun/security/tools/keytool/Resources.java
--- a/src/java.base/share/classes/sun/security/tools/keytool/Resources.java
+++ b/src/java.base/share/classes/sun/security/tools/keytool/Resources.java
@@ -456,21 +456,21 @@
         {"the.tsa.certificate", "The TSA certificate"},
         {"the.input", "The input"},
         {"reply", "Reply"},
-        {"one.in.many", "%s #%d of %d"},
+        {"one.in.many", "%1$s #%2$d of %3$d"},
         {"alias.in.cacerts", "Issuer <%s> in cacerts"},
         {"alias.in.keystore", "Issuer <%s>"},
         {"with.weak", "%s (weak)"},
-        {"key.bit", "%d-bit %s key"},
-        {"key.bit.weak", "%d-bit %s key (weak)"},
+        {"key.bit", "%1$d-bit %2$s key"},
+        {"key.bit.weak", "%1$d-bit %2$s key (weak)"},
         {"unknown.size.1", "unknown size %s key"},
         {".PATTERN.printX509Cert.with.weak",
                 "Owner: {0}\nIssuer: {1}\nSerial number: {2}\nValid from: {3} until: {4}\nCertificate fingerprints:\n\t SHA1: {5}\n\t SHA256: {6}\nSignature algorithm name: {7}\nSubject Public Key Algorithm: {8}\nVersion: {9}"},
         {"PKCS.10.with.weak",
                 "PKCS #10 Certificate Request (Version 1.0)\n" +
-                        "Subject: %s\nFormat: %s\nPublic Key: %s\nSignature algorithm: %s\n"},
-        {"verified.by.s.in.s.weak", "Verified by %s in %s with a %s"},
-        {"whose.sigalg.risk", "%s uses the %s signature algorithm which is considered a security risk."},
-        {"whose.key.risk", "%s uses a %s which is considered a security risk."},
+                        "Subject: %1$s\nFormat: %2$s\nPublic Key: %3$s\nSignature algorithm: %4$s\n"},
+        {"verified.by.s.in.s.weak", "Verified by %1$s in %2$s with a %3$s"},
+        {"whose.sigalg.risk", "%1$s uses the %2$s signature algorithm which is considered a security risk."},
+        {"whose.key.risk", "%1$s uses a %2$s which is considered a security risk."},
         {"jks.storetype.warning", "The %1$s keystore uses a proprietary format. It is recommended to migrate to PKCS12 which is an industry standard format using \"keytool -importkeystore -srckeystore %2$s -destkeystore %2$s -deststoretype pkcs12\"."},
         {"migrate.keystore.warning", "Migrated \"%1$s\" to %4$s. The %2$s keystore is backed up as \"%3$s\"."},
         {"backup.keystore.warning", "The original keystore \"%1$s\" is backed up as \"%3$s\"..."},
diff --git a/src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Resources.java b/src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Resources.java
--- a/src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Resources.java
+++ b/src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Resources.java
@@ -270,7 +270,7 @@
         {"The.1.algorithm.specified.for.the.2.option.is.considered.a.security.risk.",
                 "The %1$s algorithm specified for the %2$s option is considered a security risk."},
         {"The.1.signing.key.has.a.keysize.of.2.which.is.considered.a.security.risk.",
-                "The %s signing key has a keysize of %d which is considered a security risk."},
+                "The %1$s signing key has a keysize of %2$d which is considered a security risk."},
         {"This.jar.contains.entries.whose.certificate.chain.is.invalid.reason.1",
                  "This jar contains entries whose certificate chain is invalid. Reason: %s"},
         {"This.jar.contains.entries.whose.tsa.certificate.chain.is.invalid.reason.1",

After this there is neither /%.*%[^0-9]/ nor /%[^0-9].*%/ patterns.
Thanks
Max
Reply | Threaded
Open this post in threaded view
|

Re: RFR 8191137: keytool fails to format resource strings for keys for some languages after JDK-8171319

Severin Gehwolf
Hi,

On Tue, 2017-11-14 at 12:20 +0800, Weijun Wang wrote:

> keytool contains a printf("%d-bit %s key", 1024, "RSA") call but when it's translated into French the call becomes printf("Clave %s de %d bits", 1024, "RSA") and %s does not match 1024.
>
> The fix adds position parameters to printf resources strings when there are multiple arguments. The translations should follow.
>
> diff --git a/src/java.base/share/classes/sun/security/tools/keytool/Resources.java b/src/java.base/share/classes/sun/security/tools/keytool/Resources.java
> --- a/src/java.base/share/classes/sun/security/tools/keytool/Resources.java
> +++ b/src/java.base/share/classes/sun/security/tools/keytool/Resources.java
> @@ -456,21 +456,21 @@
>          {"the.tsa.certificate", "The TSA certificate"},
>          {"the.input", "The input"},
>          {"reply", "Reply"},
> -        {"one.in.many", "%s #%d of %d"},
> +        {"one.in.many", "%1$s #%2$d of %3$d"},
>          {"alias.in.cacerts", "Issuer <%s> in cacerts"},
>          {"alias.in.keystore", "Issuer <%s>"},
>          {"with.weak", "%s (weak)"},
> -        {"key.bit", "%d-bit %s key"},
> -        {"key.bit.weak", "%d-bit %s key (weak)"},
> +        {"key.bit", "%1$d-bit %2$s key"},
> +        {"key.bit.weak", "%1$d-bit %2$s key (weak)"},
>          {"unknown.size.1", "unknown size %s key"},
>          {".PATTERN.printX509Cert.with.weak",
>                  "Owner: {0}\nIssuer: {1}\nSerial number: {2}\nValid from: {3} until: {4}\nCertificate fingerprints:\n\t SHA1: {5}\n\t SHA256: {6}\nSignature algorithm name: {7}\nSubject Public Key Algorithm: {8}\nVersion: {9}"},
>          {"PKCS.10.with.weak",
>                  "PKCS #10 Certificate Request (Version 1.0)\n" +
> -                        "Subject: %s\nFormat: %s\nPublic Key: %s\nSignature algorithm: %s\n"},
> -        {"verified.by.s.in.s.weak", "Verified by %s in %s with a %s"},
> -        {"whose.sigalg.risk", "%s uses the %s signature algorithm which is considered a security risk."},
> -        {"whose.key.risk", "%s uses a %s which is considered a security risk."},
> +                        "Subject: %1$s\nFormat: %2$s\nPublic Key: %3$s\nSignature algorithm: %4$s\n"},
> +        {"verified.by.s.in.s.weak", "Verified by %1$s in %2$s with a %3$s"},
> +        {"whose.sigalg.risk", "%1$s uses the %2$s signature algorithm which is considered a security risk."},
> +        {"whose.key.risk", "%1$s uses a %2$s which is considered a security risk."},
>          {"jks.storetype.warning", "The %1$s keystore uses a proprietary format. It is recommended to migrate to PKCS12 which is an industry standard format using \"keytool -importkeystore -srckeystore %2$s -destkeystore %2$s -deststoretype pkcs12\"."},
>          {"migrate.keystore.warning", "Migrated \"%1$s\" to %4$s. The %2$s keystore is backed up as \"%3$s\"."},
>          {"backup.keystore.warning", "The original keystore \"%1$s\" is backed up as \"%3$s\"..."},
> diff --git a/src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Resources.java b/src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Resources.java
> --- a/src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Resources.java
> +++ b/src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Resources.java
> @@ -270,7 +270,7 @@
>          {"The.1.algorithm.specified.for.the.2.option.is.considered.a.security.risk.",
>                  "The %1$s algorithm specified for the %2$s option is considered a security risk."},
>          {"The.1.signing.key.has.a.keysize.of.2.which.is.considered.a.security.risk.",
> -                "The %s signing key has a keysize of %d which is considered a security risk."},
> +                "The %1$s signing key has a keysize of %2$d which is considered a security risk."},
>          {"This.jar.contains.entries.whose.certificate.chain.is.invalid.reason.1",
>                   "This jar contains entries whose certificate chain is invalid. Reason: %s"},
>          {"This.jar.contains.entries.whose.tsa.certificate.chain.is.invalid.reason.1",
>
> After this there is neither /%.*%[^0-9]/ nor /%[^0-9].*%/ patterns.

This looks fine, but I wonder if a regression test would be in order.
E.g. test/sun/security/tools/keytool/WeakAlg.java with
-Duser.language=fr or some such.

Thanks,
Severin
Reply | Threaded
Open this post in threaded view
|

Re: RFR 8191137: keytool fails to format resource strings for keys for some languages after JDK-8171319

Weijun Wang


> 在 2017年11月14日,18:02,Severin Gehwolf <[hidden email]> 写道:
>
> This looks fine, but I wonder if a regression test would be in order.
> E.g. test/sun/security/tools/keytool/WeakAlg.java with
> -Duser.language=fr or some such.

But my change has not really fixed anything. It’s just a hint on how to correctly translate the strings. The test you suggested would still fail after this change. It might be useful when the French translation is updated.

Thanks
Max

>
> Thanks,
> Severin

Reply | Threaded
Open this post in threaded view
|

Re: RFR 8191137: keytool fails to format resource strings for keys for some languages after JDK-8171319

Severin Gehwolf
On Tue, 2017-11-14 at 18:47 +0800, Wang Weijun wrote:

> > 在 2017年11月14日,18:02,Severin Gehwolf <[hidden email]> 写道:
> >
> > This looks fine, but I wonder if a regression test would be in
> > order.
> > E.g. test/sun/security/tools/keytool/WeakAlg.java with
> > -Duser.language=fr or some such.
>
> But my change has not really fixed anything. It’s just a hint on how
> to correctly translate the strings. The test you suggested would
> still fail after this change. It might be useful when the French
> translation is updated. 

Why can't this change update Resources_fr.java to include the correct
position and then the test updated to also work in French? It's not
really a localization change, but introduction of the correct position
of the parameter.

A follow-up bug could then move all other resources to a model similar
to Resources_fr.java?

Thanks,
Severin
Reply | Threaded
Open this post in threaded view
|

Re: RFR 8191137: keytool fails to format resource strings for keys for some languages after JDK-8171319

Weijun Wang


> On Nov 14, 2017, at 7:40 PM, Severin Gehwolf <[hidden email]> wrote:
>
> On Tue, 2017-11-14 at 18:47 +0800, Wang Weijun wrote:
>>> 在 2017年11月14日,18:02,Severin Gehwolf <[hidden email]> 写道:
>>>
>>> This looks fine, but I wonder if a regression test would be in
>>> order.
>>> E.g. test/sun/security/tools/keytool/WeakAlg.java with
>>> -Duser.language=fr or some such.
>>
>> But my change has not really fixed anything. It’s just a hint on how
>> to correctly translate the strings. The test you suggested would
>> still fail after this change. It might be useful when the French
>> translation is updated.
>
> Why can't this change update Resources_fr.java to include the correct
> position and then the test updated to also work in French? It's not
> really a localization change, but introduction of the correct position
> of the parameter.

I cannot be sure about the change, and there could be other places where similar change is needed. I'll leave it to the globalization team. This won't waste much time and they will be able to fix the problem in all releases at the same time.

In fact, I initially assign this bug to globalization directly, but they suggested me to add position labels into the English resources first so they can follow.

--<ax

>
> A follow-up bug could then move all other resources to a model similar
> to Resources_fr.java?
>
> Thanks,
> Severin

Reply | Threaded
Open this post in threaded view
|

Re: RFR 8191137: keytool fails to format resource strings for keys for some languages after JDK-8171319

Sean Mullan
In reply to this post by Weijun Wang
Fix looks good. Do you also need to add a label or subtask to the bug so
that the localization team knows that they still need to fix the
resource files?

--Sean

On 11/13/17 11:20 PM, Weijun Wang wrote:

> keytool contains a printf("%d-bit %s key", 1024, "RSA") call but when
> it's translated into French the call becomes printf("Clave %s de %d
> bits", 1024, "RSA") and %s does not match 1024.
>
> The fix adds position parameters to printf resources strings when there
> are multiple arguments. The translations should follow.
>
> *diff --git
> a/src/java.base/share/classes/sun/security/tools/keytool/Resources.java
> b/src/java.base/share/classes/sun/security/tools/keytool/Resources.java*
> *---
> a/src/java.base/share/classes/sun/security/tools/keytool/Resources.java*
> *+++
> b/src/java.base/share/classes/sun/security/tools/keytool/Resources.java*
> @@ -456,21 +456,21 @@
>           {"the.tsa.certificate", "The TSA certificate"},
>           {"the.input", "The input"},
>           {"reply", "Reply"},
> -        {"one.in.many", "%s #%d of %d"},
> +        {"one.in.many", "%1$s #%2$d of %3$d"},
>           {"alias.in.cacerts", "Issuer <%s> in cacerts"},
>           {"alias.in.keystore", "Issuer <%s>"},
>           {"with.weak", "%s (weak)"},
> -        {"key.bit", "%d-bit %s key"},
> -        {"key.bit.weak", "%d-bit %s key (weak)"},
> +        {"key.bit", "%1$d-bit %2$s key"},
> +        {"key.bit.weak", "%1$d-bit %2$s key (weak)"},
>           {"unknown.size.1", "unknown size %s key"},
>           {".PATTERN.printX509Cert.with.weak",
>                   "Owner: {0}\nIssuer: {1}\nSerial number: {2}\nValid
> from: {3} until: {4}\nCertificate fingerprints:\n\t SHA1: {5}\n\t
> SHA256: {6}\nSignature algorithm name: {7}\nSubject Public Key
> Algorithm: {8}\nVersion: {9}"},
>           {"PKCS.10.with.weak",
>                   "PKCS #10 Certificate Request (Version 1.0)\n" +
> -                        "Subject: %s\nFormat: %s\nPublic Key:
> %s\nSignature algorithm: %s\n"},
> -        {"verified.by.s.in.s.weak", "Verified by %s in %s with a %s"},
> -        {"whose.sigalg.risk", "%s uses the %s signature algorithm which
> is considered a security risk."},
> -        {"whose.key.risk", "%s uses a %s which is considered a security
> risk."},
> +                        "Subject: %1$s\nFormat: %2$s\nPublic Key:
> %3$s\nSignature algorithm: %4$s\n"},
> +        {"verified.by.s.in.s.weak", "Verified by %1$s in %2$s with a
> %3$s"},
> +        {"whose.sigalg.risk", "%1$s uses the %2$s signature algorithm
> which is considered a security risk."},
> +        {"whose.key.risk", "%1$s uses a %2$s which is considered a
> security risk."},
>           {"jks.storetype.warning", "The %1$s keystore uses a
> proprietary format. It is recommended to migrate to PKCS12 which is an
> industry standard format using \"keytool -importkeystore -srckeystore
> %2$s -destkeystore %2$s -deststoretype pkcs12\"."},
>           {"migrate.keystore.warning", "Migrated \"%1$s\" to %4$s. The
> %2$s keystore is backed up as \"%3$s\"."},
>           {"backup.keystore.warning", "The original keystore \"%1$s\" is
> backed up as \"%3$s\"..."},
> *diff --git
> a/src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Resources.java
> b/src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Resources.java*
> *---
> a/src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Resources.java*
> *+++
> b/src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Resources.java*
> @@ -270,7 +270,7 @@
>          
> {"The.1.algorithm.specified.for.the.2.option.is.considered.a.security.risk.",
>                   "The %1$s algorithm specified for the %2$s option is
> considered a security risk."},
>          
> {"The.1.signing.key.has.a.keysize.of.2.which.is.considered.a.security.risk.",
> -                "The %s signing key has a keysize of %d which is
> considered a security risk."},
> +                "The %1$s signing key has a keysize of %2$d which is
> considered a security risk."},
>          
> {"This.jar.contains.entries.whose.certificate.chain.is.invalid.reason.1",
>                    "This jar contains entries whose certificate chain is
> invalid. Reason: %s"},
>          
> {"This.jar.contains.entries.whose.tsa.certificate.chain.is.invalid.reason.1",
>
> After this there is neither /%.*%[^0-9]/ nor /%[^0-9].*%/ patterns.
> Thanks
> Max