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

[Sean Mullan]

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

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  wrote:
> 
> On Tue, 2017-11-14 at 18:47 +0800, Wang Weijun wrote:
>>> 在 2017年11月14日，18:02，Severin Gehwolf  写道：
>>> 
>>> 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.

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

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  写道：
> > 
> > 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

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

[Wang Weijun]

> 在 2017年11月14日，18:02，Severin Gehwolf  写道：
> 
> 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

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

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