dxbjavid commented on code in PR #406:
URL: https://github.com/apache/commons-net/pull/406#discussion_r3671704863
##########
src/main/java/org/apache/commons/net/ftp/parser/FTPTimestampParserImpl.java:
##########
@@ -294,6 +295,7 @@ public Calendar parseTimestamp(final String timestampStr,
final Calendar serverT
final String year = Integer.toString(now.get(Calendar.YEAR));
final String timeStampStrPlusYear = timestampStr + " " + year;
final SimpleDateFormat hackFormatter = new
SimpleDateFormat(recentDateFormat.toPattern() + " yyyy",
recentDateFormat.getDateFormatSymbols());
+ hackFormatter.setCalendar(new GregorianCalendar());
Review Comment:
good point, the formatter being Gregorian wasn't enough on its own. the
appended year came from the caller's serverTime, which under a Thai default
locale is itself a Buddhist calendar, so it was still 543 out. now reading the
year through a Gregorian calendar at the same instant before building the
string. added a recent-date test
(testParseRecentTimestampWithNonGregorianDefaultLocale) that passes a Buddhist
serverTime and fails without this change.
##########
src/test/java/org/apache/commons/net/ftp/parser/MLSxEntryParserTest.java:
##########
@@ -68,6 +75,25 @@ protected FTPFile nullFileOrNullDate(final FTPFile f) {
return f;
}
+ /**
+ * The RFC 3659 time stamp is a numeric Gregorian date. Parsing it must
not depend on the JVM default locale's calendar, which for e.g. a Thai locale
is a
+ * Buddhist calendar that would read the year 543 years out.
+ */
+ @Test
+ void testParseGMTdateTimeWithNonGregorianDefaultLocale() {
+ final Locale defaultLocale = Locale.getDefault();
+ try {
+ Locale.setDefault(new Locale("th", "TH"));
+ final Calendar parsed =
MLSxEntryParser.parseGMTdateTime("20100313224553");
+ final GregorianCalendar expected = new
GregorianCalendar(TimeZone.getTimeZone("GMT"), Locale.ROOT);
+ expected.clear();
+ expected.set(2010, Calendar.MARCH, 13, 22, 45, 53);
+ assertEquals(expected.getTimeInMillis(), parsed.getTimeInMillis());
+ } finally {
+ Locale.setDefault(defaultLocale);
+ }
+ }
+
@Override
@Test
void testDefaultPrecision() {
Review Comment:
switched all three locale tests over to junit pioneer's @DefaultLocale.
added the test dependency in the pom (version comes from commons-parent, 1.9.1
on Java 8). this also drops the manual setDefault/try-finally, which Copilot
flagged as parallel-unsafe.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
