Commit eba348b
committed
src: make process.env.TZ setter clear tz cache
Since the presence of the libc and V8 timezone caches seem to be
a perennial source of confusion to users ("why doesn't it work?!"),
let's try to support that pattern by intercepting assignments to
the TZ environment variable and reset the caches as a side effect.
Fixes: #19974
PR-URL: #20026
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Steven R Loomis <srloomis@us.ibm.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>1 parent 58fe440 commit eba348b
2 files changed
Lines changed: 64 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -2,6 +2,8 @@ | |||
| 2 | 2 | #include "node_errors.h" | |
| 3 | 3 | #include "node_process.h" | |
| 4 | 4 | ||
| 5 | + #include <time.h> // tzset(), _tzset() | ||
| 6 | + | ||
| 5 | 7 | #ifdef __APPLE__ | |
| 6 | 8 | #include <crt_externs.h> | |
| 7 | 9 | #define environ (*_NSGetEnviron()) | |
@@ -64,6 +66,19 @@ Mutex env_var_mutex; | |||
| 64 | 66 | std::shared_ptr<KVStore> system_environment = std::make_shared<RealEnvStore>(); | |
| 65 | 67 | } // namespace per_process | |
| 66 | 68 | ||
| 69 | + template <typename T> | ||
| 70 | + void DateTimeConfigurationChangeNotification(Isolate* isolate, const T& key) { | ||
| 71 | + if (key.length() == 2 && key[0] == 'T' && key[1] == 'Z') { | ||
| 72 | + #ifdef __POSIX__ | ||
| 73 | + tzset(); | ||
| 74 | + #else | ||
| 75 | + _tzset(); | ||
| 76 | + #endif | ||
| 77 | + auto constexpr time_zone_detection = Isolate::TimeZoneDetection::kRedetect; | ||
| 78 | + isolate->DateTimeConfigurationChangeNotification(time_zone_detection); | ||
| 79 | + } | ||
| 80 | + } | ||
| 81 | + | ||
| 67 | 82 | Local<String> RealEnvStore::Get(Isolate* isolate, | |
| 68 | 83 | Local<String> property) const { | |
| 69 | 84 | Mutex::ScopedLock lock(per_process::env_var_mutex); | |
@@ -115,6 +130,7 @@ void RealEnvStore::Set(Isolate* isolate, | |||
| 115 | 130 | SetEnvironmentVariableW(key_ptr, reinterpret_cast<WCHAR*>(*val)); | |
| 116 | 131 | } | |
| 117 | 132 | #endif | |
| 133 | + DateTimeConfigurationChangeNotification(isolate, key); | ||
| 118 | 134 | } | |
| 119 | 135 | ||
| 120 | 136 | int32_t RealEnvStore::Query(Isolate* isolate, Local<String> property) const { | |
@@ -150,6 +166,7 @@ void RealEnvStore::Delete(Isolate* isolate, Local<String> property) { | |||
| 150 | 166 | WCHAR* key_ptr = reinterpret_cast<WCHAR*>(*key); | |
| 151 | 167 | SetEnvironmentVariableW(key_ptr, nullptr); | |
| 152 | 168 | #endif | |
| 169 | + DateTimeConfigurationChangeNotification(isolate, key); | ||
| 153 | 170 | } | |
| 154 | 171 | ||
| 155 | 172 | Local<Array> RealEnvStore::Enumerate(Isolate* isolate) const { | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,47 @@ | |||
| 1 | + 'use strict'; | ||
| 2 | + | ||
| 3 | + const common = require('../common'); | ||
| 4 | + const assert = require('assert'); | ||
| 5 | + | ||
| 6 | + if (!common.isMainThread) | ||
| 7 | + common.skip('process.env.TZ is not intercepted in Workers'); | ||
| 8 | + | ||
| 9 | + if (common.isWindows) // Using a different TZ format. | ||
| 10 | + common.skip('todo: test on Windows'); | ||
| 11 | + | ||
| 12 | + const date = new Date('2018-04-14T12:34:56.789Z'); | ||
| 13 | + | ||
| 14 | + process.env.TZ = 'Europe/Amsterdam'; | ||
| 15 | + | ||
| 16 | + if (date.toString().includes('(Europe)')) | ||
| 17 | + common.skip('not using bundled ICU'); // Shared library or --with-intl=none. | ||
| 18 | + | ||
| 19 | + if ('Sat Apr 14 2018 12:34:56 GMT+0000 (GMT)' === date.toString()) | ||
| 20 | + common.skip('missing tzdata'); // Alpine buildbots lack Europe/Amsterdam. | ||
| 21 | + | ||
| 22 | + if (date.toString().includes('(Central European Time)') || | ||
| 23 | + date.toString().includes('(CET)')) { | ||
| 24 | + // The AIX and SmartOS buildbots report 2018 CEST as CET | ||
| 25 | + // because apparently for them that's still the deep future. | ||
| 26 | + common.skip('tzdata too old'); | ||
| 27 | + } | ||
| 28 | + | ||
| 29 | + assert.strictEqual( | ||
| 30 | + date.toString().replace('Central European Summer Time', 'CEST'), | ||
| 31 | + 'Sat Apr 14 2018 14:34:56 GMT+0200 (CEST)'); | ||
| 32 | + | ||
| 33 | + process.env.TZ = 'Europe/London'; | ||
| 34 | + assert.strictEqual( | ||
| 35 | + date.toString().replace('British Summer Time', 'BST'), | ||
| 36 | + 'Sat Apr 14 2018 13:34:56 GMT+0100 (BST)'); | ||
| 37 | + | ||
| 38 | + process.env.TZ = 'Etc/UTC'; | ||
| 39 | + assert.strictEqual( | ||
| 40 | + date.toString().replace('Coordinated Universal Time', 'UTC'), | ||
| 41 | + 'Sat Apr 14 2018 12:34:56 GMT+0000 (UTC)'); | ||
| 42 | + | ||
| 43 | + // Just check that deleting the environment variable doesn't crash the process. | ||
| 44 | + // We can't really check the result of date.toString() because we don't know | ||
| 45 | + // the default time zone. | ||
| 46 | + delete process.env.TZ; | ||
| 47 | + date.toString(); | ||
0 commit comments