You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "dxbjavid (via GitHub)" <gi...@apache.org> on 2026/07/28 06:05:46 UTC
[PR] force gregorian calendar in ftp listing timestamp parsers [commons-net]
dxbjavid opened a new pull request, #406:
URL: https://github.com/apache/commons-net/pull/406
the FTP listing parsers build their SimpleDateFormat and Calendar objects without an explicit locale, so on a JVM whose default locale carries a non-Gregorian calendar (a Thai Buddhist calendar, or the ja_JP_JP imperial calendar) a server-supplied numeric year is read in the wrong era and the parsed instant ends up centuries out, 543 years under th_TH. this affects MLSxEntryParser.parseGMTdateTime (the RFC 3659 MLSD/MLST time stamp), the shared FTPTimestampParserImpl used by the unix/nt/vms/os400/etc parsers, and EnterpriseUnixFTPEntryParser, which sets the absolute year from a listing into a default-locale Calendar. forcing a Gregorian calendar at each construction site keeps parsing independent of the ambient locale; the added tests parse a listing under a Thai default locale and check the resulting instant, and fail on the current code.
- [x] Read the [contribution guidelines](CONTRIBUTING.md) for this project.
- [ ] Read the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) if you use Artificial Intelligence (AI).
- [ ] I used AI to create any part of, or all of, this pull request. Which AI tool was used to create this pull request, and to what extent did it contribute?
- [x] Run a successful build using the default [Maven](https://maven.apache.org/) goal with `mvn`; that's `mvn` on the command line by itself.
- [x] Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied. This may not always be possible, but it is a best practice.
- [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [x] Each commit in the pull request should have a meaningful subject line and body. Note that a maintainer may squash commits during the merge process.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Force Gregorian calendar in FTP listing timestamp parsers [commons-net]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #406:
URL: https://github.com/apache/commons-net/pull/406#discussion_r3665197398
##########
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");
Review Comment:
These tests mutate the JVM-wide default locale, which can make the test suite flaky if JUnit is configured to run tests in parallel (or if other tests rely on the default locale concurrently). Consider guarding the mutation with JUnit 5 parallel-execution controls (e.g., a resource lock for system-wide settings / same-thread execution) and/or limiting the blast radius by using `Locale.setDefault(Locale.Category.FORMAT, ...)` with matching save/restore via `Locale.getDefault(Locale.Category.FORMAT)`.
##########
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:
`year` is derived from `now.get(Calendar.YEAR)`, where `now` is (based on typical usage) ultimately sourced from the `serverTime` calendar passed into `parseTimestamp`. If the caller constructed `serverTime` via `Calendar.getInstance(...)` under a non-Gregorian default locale, `now` may be a `BuddhistCalendar`, so `YEAR` will be the Buddhist year and will still shift parsing by +543 even though the formatter calendar is Gregorian. To fully decouple parsing from the ambient locale, compute the appended year using a `GregorianCalendar` (with the same instant/timezone as `serverTime`) before building `timeStampStrPlusYear`.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Force Gregorian calendar in FTP listing timestamp parsers [commons-net]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #406:
URL: https://github.com/apache/commons-net/pull/406#issuecomment-5119934981
YW!
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Force Gregorian calendar in FTP listing timestamp parsers [commons-net]
Posted by "dxbjavid (via GitHub)" <gi...@apache.org>.
dxbjavid commented on PR #406:
URL: https://github.com/apache/commons-net/pull/406#issuecomment-5119886945
@garydgregory Thank you for your time.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Force Gregorian calendar in FTP listing timestamp parsers [commons-net]
Posted by "dxbjavid (via GitHub)" <gi...@apache.org>.
dxbjavid commented on PR #406:
URL: https://github.com/apache/commons-net/pull/406#issuecomment-5114197328
@garydgregory pushed an update addressing both your note and Copilot's.
the three locale tests now use junit pioneer's @DefaultLocale instead of setting the default locale by hand; i added junit-pioneer as a test dependency and the version is inherited from commons-parent (1.9.1, which is the Java 8 line).
on Copilot's substantive point: forcing the formatter to a Gregorian calendar wasn't quite enough for the recent-date path, because the year appended there is read from the caller's serverTime, and under a Thai default locale that calendar is itself Buddhist, so the parsed instant was still 543 years out. it now reads that year through a Gregorian calendar at the same instant. i also added a recent-date test that passes a Buddhist serverTime and fails without the change.
full build is green, checkstyle clean.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Force Gregorian calendar in FTP listing timestamp parsers [commons-net]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #406:
URL: https://github.com/apache/commons-net/pull/406#discussion_r3665288653
##########
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");
Review Comment:
Use JUnit Pioneer to set the default Locale: https://junit-pioneer.org/docs/default-locale-timezone/
##########
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"));
Review Comment:
Use JUnit Pioneer to set the default Locale: https://junit-pioneer.org/docs/default-locale-timezone/
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Force gregorian calendar in ftp listing timestamp parsers [commons-net]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #406:
URL: https://github.com/apache/commons-net/pull/406#discussion_r3665106773
##########
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:
Use JUnit Pioneer to set the default Locale: https://junit-pioneer.org/docs/default-locale-timezone/
##########
src/test/java/org/apache/commons/net/ftp/parser/FTPTimestampParserImplTest.java:
##########
@@ -271,6 +271,32 @@ void testParser() throws ParseException {
}
}
+ /**
+ * A numeric year from a server listing must be read as a Gregorian year regardless of the JVM default locale. Under a Thai locale the default
+ * SimpleDateFormat calendar is a Buddhist calendar, which would otherwise shift the parsed instant by 543 years.
+ */
+ @Test
+ void testParseTimestampWithNonGregorianDefaultLocale() throws ParseException {
+ final Locale defaultLocale = Locale.getDefault();
+ try {
+ Locale.setDefault(new Locale("th", "TH"));
Review Comment:
Use JUnit Pioneer to set the default Locale: https://junit-pioneer.org/docs/default-locale-timezone/
##########
src/test/java/org/apache/commons/net/ftp/parser/EnterpriseUnixFTPEntryParserTest.java:
##########
@@ -156,6 +157,26 @@ void testParseFieldsOnFile() throws Exception {
assertEquals(0, zDateTime.getSecond());
}
+ /**
+ * A numeric year in a listing must be read as a Gregorian year regardless of the JVM default locale. Under a Thai locale the base Calendar is a Buddhist
+ * calendar, which would otherwise store the timestamp 543 years out.
+ */
+ @Test
+ void testAbsoluteYearWithNonGregorianDefaultLocale() {
+ final Locale defaultLocale = Locale.getDefault();
+ try {
+ Locale.setDefault(new Locale("th", "TH"));
Review Comment:
Use JUnit Pioneer to set the default Locale: https://junit-pioneer.org/docs/default-locale-timezone/
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Force Gregorian calendar in FTP listing timestamp parsers [commons-net]
Posted by "dxbjavid (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Force Gregorian calendar in FTP listing timestamp parsers [commons-net]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory merged PR #406:
URL: https://github.com/apache/commons-net/pull/406
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org