RFR: 8255078: sun/net/ftp/imp/FtpClient$MLSxParser uses wrong datetime format [v3]

[Marcono1234]

On Fri, 23 Oct 2020 17:50:51 GMT, Igor Ignatyev <iignatyev at openjdk.org> wrote:

>> Hi all,
>> 
>> could you please review this small patch?
>> 
>> according to [RFC3659](https://tools.ietf.org/html/rfc3659), time values in MLSx response have the following format: 
>>>    time-val = 14DIGIT [ "." 1*DIGIT ] 
>>> 
>>>    The leading, mandatory, fourteen digits are to be interpreted as, in 
>>>    order from the leftmost, four digits giving the year, with a range of 
>>>    1000--9999, two digits giving the month of the year, with a range of 
>>>    01--12, two digits giving the day of the month, with a range of 
>>>    01--31, two digits giving the hour of the day, with a range of 
>>>    00--23, two digits giving minutes past the hour, with a range of 
>>>    00--59, and finally, two digits giving seconds past the minute, with 
>>>    a range of 00--60 (with 60 being used only at a leap second). Years 
>>>    in the tenth century, and earlier, cannot be expressed. This is not 
>>>    considered a serious defect of the protocol. 
>>> 
>>>    The optional digits, which are preceded by a period, give decimal 
>>>    fractions of a second. These may be given to whatever precision is 
>>>    appropriate to the circumstance, however implementations MUST NOT add 
>>>    precision to time-vals where that precision does not exist in the 
>>>    underlying value being transmitted. 
>>> 
>>>    Symbolically, a time-val may be viewed as 
>>> 
>>>   YYYYMMDDHHMMSS.sss 
>>> 
>>>    The "." and subsequent digits ("sss") are optional. However the "." 
>>>    MUST NOT appear unless at least one following digit also appears. 
>>> 
>> 
>> `MLSxParser`, however, uses `SimpleDateFormat("yyyyMMddhhmmss")` (which is wrong b/c it uses `hh` instead of `HH` and doesn't account for optional `.sss`) to parse modify/create facts and ignore any parse exceptions.
>> 
>> `FtpClient` actually already has and uses `SimpleDateFormat` w/ right formats in `getLastModified` where it parses MDTM response, the patch refactors the code to reuse the same `SimpleDateFormat` in `MLSxParser`.
>> 
>> testing:
>> * [x] tier1
>> * [x] `test/jdk/sun/net` on `{linux,windows,macosx}-x64`
>
> Igor Ignatyev has updated the pull request incrementally with one additional commit since the last revision:
> 
>   use only filename in assert message

src/java.base/share/classes/sun/net/ftp/impl/FtpClient.java line 1777:

> 1775:     }
> 1776: 
> 1777:     private static Date parseRfc3659TimeValue(String s) {

Shouldn't this method be synchronized because `SimpleDateFormat` is not thread-safe, but instances are stored in the static field `dateFormats`?
(Note though that this issue existed before as well)

-------------

PR: https://git.openjdk.java.net/jdk/pull/776