Time format in jar and jarsigner output (was Re: Code review request: 7163483 JarSigner -verify -verbose does not format date string according to locale)

Wed Jul 11 07:56:09 UTC 2012

Hello Sherman,

Thanks a lot for review!

I've made another patch to no include the changes for jar tool,
http://cr.openjdk.java.net/~luchsh/7163483_6/

And to Max,

Does it make sense for you to change jarSigner only?

best regards!

Jonathan

On 07/11/2012 03:13 PM, Xueming Shen wrote:
>
> As Max point out, you definitely need to avoid creating SDF everytime 
> you have to
> print out one entry. But I'm more worried about the fact you are 
> changing the out
> put of jar t. Jar is a widely used tool, It might be a surprise to 
> someone who has code
> depend on the current output format. Personally I would suggest not 
> make this change
> in jar tool.
>
> -Sherman
>
> On 7/10/2012 11:21 PM, Weijun Wang wrote:
>> Mostly fine.
>>
>> 1. "import java.text.FieldPosition;" is not needed anymore.
>>
>> 2. In jar/Main, you can also consider creating a separate 
>> SimpleDateFormat object (maybe private final static?). It seems the 
>> initialization is quite heavy.
>>
>> You recently became a jdk8 committer, right? That means you can do 
>> your own push but if you want to make any changes to the bug 
>> database, just tell me.
>>
>> Thanks
>> Max
>>
>> On 07/11/2012 02:07 PM, Jonathan Lu wrote:
>>> Hello Max,
>>>
>>> Thanks a lot for review, here's the updated patch,
>>>
>>> http://cr.openjdk.java.net/~luchsh/7163483_5/
>>>
>>> On 07/10/2012 08:54 AM, Weijun Wang wrote:
>>>> Hi Jonathon
>>>>
>>>> This is better. Two minor comments, you can decide what to do or we
>>>> can see what core-lib-devs think:
>>>
>>> I was proposing my solution for this issue and I'd like to hear more
>>> from core-lib-devs list. :)
>>>
>>>>
>>>> 1. Current output uses "zzz" for timezones, you're using "ZZZ". It
>>>> might be more of an ISO flavor.
>>>
>>> Changed to "zzz" format which seems to be more readable for me.
>>>
>>>>
>>>> 2. The DateFormat.format(Date,StringBuffer,FilePosition) is quite
>>>> advanced. If it's me, I'll use the simpler format(Data) method.
>>>
>>> Updated in the new patch.
>>>
>>>>
>>>> Since the jar command also uses the same output format, can you also
>>>> make the same change there (that's sun.tools.jar.Main) and ask
>>>> core-libs-dev at openjdk.java.net for a review? Sherman
>>>> (xueming.shen at oracle.com) owns the jar tool.
>>> I've included sun.tools.jar.Main in the latest patch and CCed Sherman.
>>>
>>> Could you please help to take another look?
>>>
>>> Many thanks
>>>
>>> Jonathan
>>>
>>>>
>>>> Thanks
>>>> Max
>>>>
>>>>
>>>> On 07/09/2012 03:24 PM, Jonathan Lu wrote:
>>>>> Hi Max,
>>>>>
>>>>> Thanks for reviewing.
>>>>>
>>>>> On 07/06/2012 06:27 PM, Weijun Wang wrote:
>>>>>> Hi Jonathan
>>>>>>
>>>>>> I have these questions:
>>>>>>
>>>>>> 1. Why always CAPITAL letters for month and weekday?
>>>>>
>>>>> This is a fault of the patch, which can be easily fixed by 
>>>>> updating the
>>>>> format string.
>>>>>
>>>>>>
>>>>>> 2. Why always "dd HH:mm:ss zzz yyyy" for the rest? Some locales uses
>>>>>> "." instead of ":" as times delimiters.
>>>>>> 3. Why always dd after MMM? Some locales prefer dd before MMM.
>>>>>
>>>>> For question #2 and #3, I was just trying to follow the original 
>>>>> format
>>>>> of Date.toString().
>>>>>
>>>>>>
>>>>>> Well, if you really think the current "Fri Jul" output is too 
>>>>>> English,
>>>>>> instead of localizing the string, how about we de-localize it 
>>>>>> totally
>>>>>> and choose a neutral format?
>>>>>>
>>>>>> There are several flavors of ISO date/time format at
>>>>>>
>>>>>>   http://www.w3.org/TR/NOTE-datetime
>>>>>>
>>>>>> or we can just choose
>>>>>>
>>>>>>   YYYY-MM-dd HH:mm:ss zzz
>>>>>
>>>>> Good idea, how about a patch in this way?
>>>>> http://cr.openjdk.java.net/~luchsh/7163483_4/
>>>>>
>>>>> And I prefer to your format since it looks more readable to me.
>>>>>
>>>>> Thanks and best regards!
>>>>> Jonathan
>>>>>
>>>>>>
>>>>>> BTW, the jar command is using the same format, therefore I'm adding
>>>>>> core-libs-dev.
>>>>>>
>>>>>> Thanks
>>>>>> Max
>>>>>>
>>>>>>
>>>>>> On 07/06/2012 05:16 PM, Jonathan Lu wrote:
>>>>>>> Hello Max,
>>>>>>>
>>>>>>> I's been a long time since my last mail, I did some 
>>>>>>> investigation and
>>>>>>> had some discussion with i18n developers,  but still did not see a
>>>>>>> nice
>>>>>>> solution for the alignment problem. There does not seem be an 
>>>>>>> existing
>>>>>>> API to do this job in JDK scope. So I implemented a simple format
>>>>>>> function, and use it to format under different locales.
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~luchsh/7163483_3/
>>>>>>>
>>>>>>> The patch is trying to format the code in the same way as
>>>>>>> java.util.Date.toString() in the format of "EEE MMM dd HH:mm:ss zzz
>>>>>>> yyyy", except for using names of month and DOW in localized 
>>>>>>> format. So
>>>>>>> far, it works good for me under all supported locales.
>>>>>>>
>>>>>>> Here's a test case to verify the vertical alignment, which I has 
>>>>>>> been
>>>>>>> posted to i18n mailing list before,
>>>>>>> http://cr.openjdk.java.net/~luchsh/VerticalAlignmentTest.java
>>>>>>>
>>>>>>> It may still fail under "vi_VN" locale with this solution due to 
>>>>>>> test
>>>>>>> case limit, but I do not think it is a real failure since the 
>>>>>>> result
>>>>>>> fields still get aligned except for multiple words in one field.
>>>>>>>
>>>>>>> Could you please take a look at the patch?
>>>>>>>
>>>>>>> Many thanks & best regards
>>>>>>> Jonathan
>>>>>>>
>>>>>>> On 04/25/2012 07:48 PM, Weijun Wang wrote:
>>>>>>>> Hi Jonathan
>>>>>>>>
>>>>>>>> I'm using English.
>>>>>>>>
>>>>>>>> In your test all the files have a similar modified time so you 
>>>>>>>> cannot
>>>>>>>> see the difference. However, in my example, you can see that the
>>>>>>>> widths for date and hour are not zero-padded so the width can be
>>>>>>>> either 1 or 2.
>>>>>>>>
>>>>>>>> French is even worse
>>>>>>>>
>>>>>>>> smk       76 10 nov. 2009 08:57:54 bin/vbin/go
>>>>>>>> smk     1149 8 avr. 2012 16:03:20 bin/vbin/netbeans
>>>>>>>> smk      170 20 nov. 2009 16:47:42 bin/vbin/syncdown
>>>>>>>> smk      671 8 févr. 2012 20:11:22 bin/vbin/ssh.desktop
>>>>>>>> smk      187 20 nov. 2009 16:47:34 bin/vbin/syncsf
>>>>>>>>
>>>>>>>> So here even the width of month abbr can be different.
>>>>>>>>
>>>>>>>> Thanks
>>>>>>>> Max
>>>>>>>>
>>>>>>>>
>>>>>>>> On 04/25/2012 07:09 PM, Jonathan Lu wrote:
>>>>>>>>> Hello Max,
>>>>>>>>>
>>>>>>>>> Terribly sorry for my misunderstanding!
>>>>>>>>>
>>>>>>>>> On 04/25/2012 05:39 PM, Weijun Wang wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 04/25/2012 05:23 PM, Jonathan Lu wrote:
>>>>>>>>>>> Hi Max,
>>>>>>>>>>>
>>>>>>>>>>> On 04/25/2012 05:12 PM, Weijun Wang wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On 04/25/2012 03:28 PM, Jonathan Lu wrote:
>>>>>>>>>>>>> Hi Weijun,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks for your time, I've updated the webrev, could you 
>>>>>>>>>>>>> please
>>>>>>>>>>>>> take a
>>>>>>>>>>>>> look?
>>>>>>>>>>>>> http://cr.openjdk.java.net/~luchsh/7163483_2/
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 04/24/2012 03:06 PM, Weijun Wang wrote:
>>>>>>>>>>>>>> Hi Jonathan
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Some comments:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 1. Can you be sure that the new format always has the same
>>>>>>>>>>>>>> length?
>>>>>>>>>>>>>> jarsigner tries to output in a tabular style and each column
>>>>>>>>>>>>>> should be
>>>>>>>>>>>>>> aligned.
>>>>>>>>>>>>>
>>>>>>>>>>>>> I'm not sure of that, so the test case was updated to compare
>>>>>>>>>>>>> the
>>>>>>>>>>>>> first
>>>>>>>>>>>>> several tokens to determine whether there's any differences
>>>>>>>>>>>>> in the
>>>>>>>>>>>>> expression of date time.
>>>>>>>>>>>>
>>>>>>>>>>>> Sorry, I didn't make myself clear last time, I was mainly
>>>>>>>>>>>> afraid of
>>>>>>>>>>>> unaligned lines that make the output ugly.
>>>>>>>>>>>>
>>>>>>>>>>>> For example:
>>>>>>>>>>>>
>>>>>>>>>>>> smk 76 Nov 10, 2009 8:57:54 AM bin/vbin/go
>>>>>>>>>>>> smk 1149 Apr 8, 2012 4:03:20 PM bin/vbin/netbeans
>>>>>>>>>>>> smk 170 Nov 20, 2009 4:47:42 PM bin/vbin/syncdown
>>>>>>>>>>>> smk 671 Feb 8, 2012 8:11:22 PM bin/vbin/ssh.desktop
>>>>>>>>>>>> smk 187 Nov 20, 2009 4:47:34 PM bin/vbin/syncsf
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> I think that would not be a problem in the new test case which
>>>>>>>>>>> compares
>>>>>>>>>>> tokenized strings splited by blank spaces instead of
>>>>>>>>>>> String#equals.
>>>>>>>>>>> Does
>>>>>>>>>>> that make sense?
>>>>>>>>>>
>>>>>>>>>> I'm not talking about the test. It's the output of jarsigner
>>>>>>>>>> looking
>>>>>>>>>> ugly.
>>>>>>>>>>
>>>>>>>>>> smk 76 Nov 10, 2009 8:57:54 AM bin/vbin/go
>>>>>>>>>> smk 1149 Apr 8, 2012 4:03:20 PM bin/vbin/netbeans
>>>>>>>>>> smk 170 Nov 20, 2009 4:47:42 PM bin/vbin/syncdown
>>>>>>>>>> smk 671 Feb 8, 2012 8:11:22 PM bin/vbin/ssh.desktop
>>>>>>>>>> smk 187 Nov 20, 2009 4:47:34 PM bin/vbin/syncsf
>>>>>>>>>>
>>>>>>>>>> Compare with the current output:
>>>>>>>>>>
>>>>>>>>>> smk 76 Tue Nov 10 08:57:54 CST 2009 bin/vbin/go
>>>>>>>>>> smk 1149 Sun Apr 08 16:03:20 CST 2012 bin/vbin/netbeans
>>>>>>>>>> smk 170 Fri Nov 20 16:47:42 CST 2009 bin/vbin/syncdown
>>>>>>>>>> smk 671 Wed Feb 08 20:11:22 CST 2012 bin/vbin/ssh.desktop
>>>>>>>>>> smk 187 Fri Nov 20 16:47:34 CST 2009 bin/vbin/syncsf
>>>>>>>>>
>>>>>>>>> I did not see unaligned format in my testing, did you get these
>>>>>>>>> unaligned output after applying the patch? From above lines, I
>>>>>>>>> see the
>>>>>>>>> starting indices of date string in each line are always the same,
>>>>>>>>> which
>>>>>>>>> is achieved by jarsigner, but the length of the date strings are
>>>>>>>>> not the
>>>>>>>>> same, which locale were you testing on?
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thanks
>>>>>>>>>> Max
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>> Thanks
>>>>>>>>>>>> Max
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 2. You might need to reformat the modified line to make 
>>>>>>>>>>>>>> it fit
>>>>>>>>>>>>>> into 80
>>>>>>>>>>>>>> characters width.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 3. Why not include the test inside the changeset?
>>>>>>>>>>>>> 2, 3 were done in the new patch
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks
>>>>>>>>>>>>>> Max
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 04/23/2012 05:46 PM, Jonathan Lu wrote:
>>>>>>>>>>>>>>> Hello security-dev,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Here's a patch for bug 7163483, could anybody please 
>>>>>>>>>>>>>>> help to
>>>>>>>>>>>>>>> take a
>>>>>>>>>>>>>>> look?
>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~luchsh/7163483/
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> The problem is that command "jarsigner -verify -verbose
>>>>>>>>>>>>>>> my.jar"
>>>>>>>>>>>>>>> does not
>>>>>>>>>>>>>>> format date string according to current locale. following
>>>>>>>>>>>>>>> simple
>>>>>>>>>>>>>>> test
>>>>>>>>>>>>>>> case can be used to disclose this problem.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> /*
>>>>>>>>>>>>>>> * Copyright (c) 2012 Oracle and/or its affiliates. All 
>>>>>>>>>>>>>>> rights
>>>>>>>>>>>>>>> reserved.
>>>>>>>>>>>>>>> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE
>>>>>>>>>>>>>>> HEADER.
>>>>>>>>>>>>>>> *
>>>>>>>>>>>>>>> * This code is free software; you can redistribute it 
>>>>>>>>>>>>>>> and/or
>>>>>>>>>>>>>>> modify it
>>>>>>>>>>>>>>> * under the terms of the GNU General Public License 
>>>>>>>>>>>>>>> version 2
>>>>>>>>>>>>>>> only, as
>>>>>>>>>>>>>>> * published by the Free Software Foundation.
>>>>>>>>>>>>>>> *
>>>>>>>>>>>>>>> * This code is distributed in the hope that it will be
>>>>>>>>>>>>>>> useful, but
>>>>>>>>>>>>>>> WITHOUT
>>>>>>>>>>>>>>> * ANY WARRANTY; without even the implied warranty of
>>>>>>>>>>>>>>> MERCHANTABILITY or
>>>>>>>>>>>>>>> * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General 
>>>>>>>>>>>>>>> Public
>>>>>>>>>>>>>>> License
>>>>>>>>>>>>>>> * version 2 for more details (a copy is included in the
>>>>>>>>>>>>>>> LICENSE
>>>>>>>>>>>>>>> file
>>>>>>>>>>>>>>> that
>>>>>>>>>>>>>>> * accompanied this code).
>>>>>>>>>>>>>>> *
>>>>>>>>>>>>>>> * You should have received a copy of the GNU General Public
>>>>>>>>>>>>>>> License
>>>>>>>>>>>>>>> version
>>>>>>>>>>>>>>> * 2 along with this work; if not, write to the Free 
>>>>>>>>>>>>>>> Software
>>>>>>>>>>>>>>> Foundation,
>>>>>>>>>>>>>>> * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301
>>>>>>>>>>>>>>> USA.
>>>>>>>>>>>>>>> *
>>>>>>>>>>>>>>> * Please contact Oracle, 500 Oracle Parkway, Redwood
>>>>>>>>>>>>>>> Shores, CA
>>>>>>>>>>>>>>> 94065
>>>>>>>>>>>>>>> USA
>>>>>>>>>>>>>>> * or visit www.oracle.com if you need additional
>>>>>>>>>>>>>>> information or
>>>>>>>>>>>>>>> have any
>>>>>>>>>>>>>>> * questions.
>>>>>>>>>>>>>>> */
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> /*
>>>>>>>>>>>>>>> * Portions Copyright (c) 2012 IBM Corporation
>>>>>>>>>>>>>>> */
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> import java.io.ByteArrayOutputStream;
>>>>>>>>>>>>>>> import java.io.PrintStream;
>>>>>>>>>>>>>>> import java.util.Locale;
>>>>>>>>>>>>>>> import sun.security.tools.JarSigner;
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> public class bug7163483 {
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> public static void main(String[] args) throws Exception {
>>>>>>>>>>>>>>> final String[] arg = { "-verify", "-verbose",
>>>>>>>>>>>>>>> System.getProperty("java.home")+"/lib/jce.jar"};
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> ByteArrayOutputStream stream = new
>>>>>>>>>>>>>>> ByteArrayOutputStream(1024*64);
>>>>>>>>>>>>>>> PrintStream out = new PrintStream(stream);
>>>>>>>>>>>>>>> System.setOut(out);
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Locale.setDefault(Locale.GERMAN);
>>>>>>>>>>>>>>> JarSigner js = new JarSigner();
>>>>>>>>>>>>>>> js.run(arg);
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> out.flush();
>>>>>>>>>>>>>>> String s1 = stream.toString();
>>>>>>>>>>>>>>> s1 = s1.substring(0, s1.length()/2);
>>>>>>>>>>>>>>> stream.reset();
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Locale.setDefault(Locale.FRANCE);
>>>>>>>>>>>>>>> js = new JarSigner();
>>>>>>>>>>>>>>> js.run(arg);
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> out.flush();
>>>>>>>>>>>>>>> String s2 = stream.toString();
>>>>>>>>>>>>>>> s2 = s2.substring(0, s2.length()/2);
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> if (s1.equals(s2)) {
>>>>>>>>>>>>>>> System.err.println("Header output for GERMAN locale 
>>>>>>>>>>>>>>> is:"+s1);
>>>>>>>>>>>>>>> System.err.println("Header output for FRANCE locale 
>>>>>>>>>>>>>>> is:"+s2);
>>>>>>>>>>>>>>> throw new RuntimeException(
>>>>>>>>>>>>>>> "JarSigner verbose outputs are the same after setting
>>>>>>>>>>>>>>> locale!!");
>>>>>>>>>>>>>>> } else {
>>>>>>>>>>>>>>> System.err.println("Header output for GERMAN locale 
>>>>>>>>>>>>>>> is:"+s1);
>>>>>>>>>>>>>>> System.err.println("Header output for FRANCE locale 
>>>>>>>>>>>>>>> is:"+s2);
>>>>>>>>>>>>>>> System.err.println("Test passed!");
>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Thanks and best regards!
>>>>>>>>>>>>>>> - Jonathan Lu
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Best regards!
>>>>>>>>>>>>> - Jonathan
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>> Thanks & regards!
>>>>>>>>>>> - Jonathan
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks
>>>>>>>>> - Jonathan
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>