[8] RFR of 8066985: Java Webstart downloading packed files can result in Timezone set to UTC

[Xueming Shen]

On 03/19/2015 09:29 AM, Peter Levart wrote:
> On 03/19/2015 04:46 PM, Peter Levart wrote:
>> On 03/19/2015 02:53 PM, Kumar Srinivasan wrote:
>>> Mikhail,
>>>
>>> You can move the common utilitieschangeDefaultTimeZoneToUtc and
>>> restoreDefaultTimeZoneto Utils.java.
>>>
>>> Also I am not sure how effective the test is ?  does it catch the issue ?
>>> if you don't have the fix in PackerImpl and UnpackerImpl ?
>>>
>>> Otherwise it looks good, and I can sponsor this patch for you.
>>>
>>> Kumar
>>
>> Hi Mikhail,
>>
>> Is this code part of some utility so that it is executed exclusively in it's own JVM, or can it also be executed by some public API or internal JVM thread? It appears to be the later (used by java.util.jar.Pack200 public API), but haven't checked.
>>
>> In case of the later, I think it is very strange that the code changes global JVM timezone, albeit just temporarily. This could affect other code that needs default TZ and executes concurrently.
>>
>> Even in case of the former, if the PackerImpl.pack can be executed by multiple concurrent threads and if UnpackerImpl.unpack can be executed by multiple concurrent threads, what guarantees that some PackerImpl.pack thread is not executed concurrently with some UnpackerImpl.unpack thread? You only track each method separately.
>>
>>
>> Regards, Peter
>
> I guess this default TZ manipulation is needed because JarOutputStream/JarInputStream don't provide constructors that would take a TZ object, but always use default TZ, right?
>
> The cleanest way would be to add such constructors, but if this is too much for 8u, then perhaps some internal ThreadLocal<TZ> could be exposed to both PackerImpl/UnpackerImpl and Jar[Zip]Input[Output] stream internals. It think the use of default TZ in Jar/Zip/Streams is very localized. It is needed to convert local time (specified as DOS time) to and from epoch-based time exposed in ZipEntry. The code is located in package-private class java.util.zip.ZipUtils in methods dosToJavaTime /javaToDosTime (or equivalent extendedDosToJavaTime / javaToExtendedDosTime in JDK9).
>

Default TZ manipulation is really not the preferred/good approach.

Given the timestamp in a zip entry is really a dos/local date-time, a "clean"
solution is to add a pair of ZipEntry.set/getTimeLocal() methods to free
ZipEntry from handing local date-time <-> utc millis conversion with the
default timezone.

     public void setTimeLocal(LocalDateTime ldt) {
         this.xdostime = ((ldt.getYear() - 1980) << 25 |
             ldt.getMonthValue() << 21 |
             ldt.getDayOfMonth() << 16 |
             ldt.getHour() << 11 |
             ldt.getMinute() << 5 |
             ldt.getSecond() >> 1)
             + ((long)(((ldt.getSecond() & 0x1) * 1000) +
                       ldt.getNano() / 1000_000) << 32);
         this.mtime = null;
     }


     public LocalDateTime getTimeLocal() {
         int ms = (int)(xdostime >> 32);
         return LocalDateTime.of((int)(((xdostime >> 25) & 0x7f) + 1980),
                              (int)((xdostime >> 21) & 0x0f),
                              (int)((xdostime >> 16) & 0x1f),
                              (int)((xdostime >> 11) & 0x1f),
                              (int)((xdostime >> 5) & 0x3f),
                              (int)((xdostime << 1) & 0x3e) + ms / 1000,
                              (ms % 1000) * 1000_000);
     }


For pack/unpack, as they always assume UTC, it should convert the utc
millis to/from LocalDateTime as

ZipEntry.setTimeLocale(LocalDateTime.ofEpochSecond(time / 1000, (int)(time % 1000) * 1000_000, ZoneOffset.UTC));

and

Math.multiplyExact(ZipEntry.getTimeLocal().atOffset(ZoneOffset.UTC).toEpochSecond(), 1000);


-Sherman