Bug 1491159 - More complete handling of TZ environment variable when initializing ICU timezone. r=anba
authorL. David Baron <dbaron@dbaron.org>
Tue, 18 Sep 2018 20:51:30 +0000
changeset 437018 75664900fcd9403abc477828c9ae93193bf4ee8f
parent 437017 fb07be80c0b7dc5a74a90764489759b6851f1d5a
child 437019 dc263497b3391bc08472307a8f3f5d5340c582f9
push id34667
push useraiakab@mozilla.com
push dateWed, 19 Sep 2018 02:13:23 +0000
treeherdermozilla-central@3857cbe7b717 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersanba
bugs1491159, 1484829
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1491159 - More complete handling of TZ environment variable when initializing ICU timezone. r=anba This improves on the changes in bug 1484829 by handling the absolute filename form of TZ environment variables both with and without an initial ':' character, and by improving comments about relative and absolute paths. (Note that relative paths without the ':' are just Olson time zone names.) Note that the only test that failed without the patch was: setTimeZone("/usr/share/zoneinfo/America/Chicago"); assertEq(timeZoneName(), "Central Daylight Time"); and it reported Error: Assertion failed: got "GMT-06:00", expected "Central Daylight Time" which seems to be a sign that ICU gets some information out of that time zone identifier (i.e., it reported the correct standard-time GMT offset), but not everything. Differential Revision: https://phabricator.services.mozilla.com/D5827
js/src/tests/non262/Date/time-zone-path.js
js/src/vm/DateTime.cpp
copy from js/src/tests/non262/Date/time-zone-etc_localetime.js
copy to js/src/tests/non262/Date/time-zone-path.js
--- a/js/src/tests/non262/Date/time-zone-etc_localetime.js
+++ b/js/src/tests/non262/Date/time-zone-path.js
@@ -1,29 +1,30 @@
 // |reftest| skip-if(xulRuntime.OS=="WINNT"||!xulRuntime.shell)
 
 assertEq(/^(PST|PDT)$/.test(getTimeZone()), true,
          "The default time zone is set to PST8PDT for all jstests (when run in the shell)");
 
 function timeZoneName() {
     var dtf = new Intl.DateTimeFormat("en-US", {timeZoneName: "long"});
-    return dtf.formatToParts().filter(x => x.type === "timeZoneName")[0].value;
+    return dtf.formatToParts(Date.UTC(2017, 2, 31, 12, 0, 0)).filter(x => x.type === "timeZoneName")[0].value;
 }
 
-// Calling setTimeZone() with an undefined argument clears the TZ environment
-// variable and by that reveal the actual system time zone.
-setTimeZone(undefined);
-var systemTimeZone = getTimeZone();
-var systemTimeZoneName = timeZoneName();
+setTimeZone("Europe/Paris");
+assertEq(timeZoneName(), "Central European Summer Time");
+
+setTimeZone(":Europe/Helsinki");
+assertEq(timeZoneName(), "Eastern European Summer Time");
+
+setTimeZone("::Europe/London"); // two colons, invalid
+assertEq(timeZoneName(), "Coordinated Universal Time");
 
-// Set to an unlikely system time zone, so that the next call to setTimeZone()
-// will lead to a time zone change.
-setTimeZone("Antarctica/Troll");
+setTimeZone("/this-part-is-ignored/zoneinfo/America/Chicago");
+assertEq(timeZoneName(), "Central Daylight Time");
 
-// Now call with the file path ":/etc/localtime" which is special-cased in
-// DateTimeInfo to read the system time zone.
-setTimeZone(":/etc/localtime");
+setTimeZone(":/this-part-is-ignored/zoneinfo/America/Phoenix");
+assertEq(timeZoneName(), "Mountain Standard Time");
 
-assertEq(getTimeZone(), systemTimeZone);
-assertEq(timeZoneName(), systemTimeZoneName);
+setTimeZone("::/this-part-is-ignored/zoneinfo/America/Los_Angeles"); // two colons, invalid
+assertEq(timeZoneName(), "Coordinated Universal Time");
 
 if (typeof reportCompare === "function")
     reportCompare(true, true);
--- a/js/src/vm/DateTime.cpp
+++ b/js/src/vm/DateTime.cpp
@@ -621,23 +621,27 @@ IsOlsonCompatibleWindowsTimeZoneId(const
         if (std::strcmp(allowedId, tz) == 0) {
             return true;
         }
     }
     return false;
 }
 #elif ENABLE_INTL_API && defined(ICU_TZ_HAS_RECREATE_DEFAULT)
 static inline const char*
-TZContainsPath(const char* tzVar)
+TZContainsAbsolutePath(const char* tzVar)
 {
-    // A TZ beginning with a colon is expected to be followed by a path --
-    // typically an absolute path (often /etc/localtime), but alternatively a
-    // path relative to /usr/share/zoneinfo/.
-    // NB: We currently only support absolute paths.
-    return tzVar[0] == ':' && tzVar[1] == '/' ? tzVar + 1 : nullptr;
+    // A TZ environment variable may be an absolute path. The path
+    // format of TZ may begin with a colon. (ICU handles relative paths.)
+    if (tzVar[0] == ':' && tzVar[1] == '/') {
+        return tzVar + 1;
+    }
+    if (tzVar[0] == '/') {
+        return tzVar;
+    }
+    return nullptr;
 }
 
 /**
  * Given a presumptive path |tz| to a zoneinfo time zone file
  * (e.g. /etc/localtime), attempt to compute the time zone encoded by that
  * path by repeatedly resolving symlinks until a path containing "/zoneinfo/"
  * followed by time zone looking components is found. If a symlink is broken,
  * symlink-following recurs too deeply, non time zone looking components are
@@ -772,20 +776,23 @@ js::ResyncICUDefaultTimeZone()
             if (IsOlsonCompatibleWindowsTimeZoneId(tz)) {
                 tzid.setTo(icu::UnicodeString(tz, -1, US_INV));
             } else {
                 // If |tz| isn't a supported time zone identifier, use the
                 // default Windows time zone for ICU.
                 // TODO: Handle invalid time zone identifiers (bug 342068).
             }
 #else
-            // Handle links (starting with ':') manually because ICU currently
-            // doesn't support the TZ filespec format.
+            // The TZ environment variable allows both absolute and
+            // relative paths, optionally beginning with a colon (':').
+            // (Relative paths, without the colon, are just Olson time
+            // zone names.)  We need to handle absolute paths ourselves,
+            // including handling that they might be symlinks.
             // <https://unicode-org.atlassian.net/browse/ICU-13694>
-            if (const char* tzlink = TZContainsPath(tz))
+            if (const char* tzlink = TZContainsAbsolutePath(tz))
                 tzid.setTo(ReadTimeZoneLink(tzlink));
 #endif /* defined(XP_WIN) */
 
             if (!tzid.isEmpty()) {
                 mozilla::UniquePtr<icu::TimeZone> newTimeZone(icu::TimeZone::createTimeZone(tzid));
                 MOZ_ASSERT(newTimeZone);
                 if (*newTimeZone != icu::TimeZone::getUnknown()) {
                     // adoptDefault() takes ownership of the time zone.