Skip to content

Commit

Permalink
fix(ext/node): clear tz cache when setting process.env.TZ (#27826)
Browse files Browse the repository at this point in the history
  • Loading branch information
littledivy authored Jan 28, 2025
1 parent ce31688 commit 5c64146
Show file tree
Hide file tree
Showing 6 changed files with 106 additions and 2 deletions.
28 changes: 28 additions & 0 deletions ext/os/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,35 @@ fn op_exec_path(state: &mut OpState) -> Result<String, OsError> {
.map_err(OsError::InvalidUtf8)
}

fn dt_change_notif(isolate: &mut v8::Isolate, key: &str) {
extern "C" {
#[cfg(unix)]
fn tzset();

#[cfg(windows)]
fn _tzset();
}

if key == "TZ" {
// SAFETY: tzset/_tzset (libc) is called to update the timezone information
unsafe {
#[cfg(unix)]
tzset();

#[cfg(windows)]
_tzset();
}

isolate.date_time_configuration_change_notification(
v8::TimeZoneDetection::Redetect,
);
}
}

#[op2(fast, stack_trace)]
fn op_set_env(
state: &mut OpState,
scope: &mut v8::HandleScope,
#[string] key: &str,
#[string] value: &str,
) -> Result<(), OsError> {
Expand All @@ -182,7 +208,9 @@ fn op_set_env(
if value.contains('\0') {
return Err(OsError::EnvInvalidValue(value.to_string()));
}

env::set_var(key, value);
dt_change_notif(scope, key);
Ok(())
}

Expand Down
1 change: 1 addition & 0 deletions tests/node_compat/config.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -854,6 +854,7 @@
"test-process-emitwarning.js",
"test-process-env-allowed-flags.js",
"test-process-env-delete.js",
"test-process-env-tz.js",
"test-process-env-windows-error-reset.js",
"test-process-exit-from-before-exit.js",
"test-process-exit-handler.js",
Expand Down
3 changes: 1 addition & 2 deletions tests/node_compat/runner/TODO.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<!-- deno-fmt-ignore-file -->
# Remaining Node Tests

1154 tests out of 3681 have been ported from Node 20.11.1 (31.35% ported, 69.19% remaining).
1155 tests out of 3681 have been ported from Node 20.11.1 (31.38% ported, 69.17% remaining).

NOTE: This file should not be manually edited. Please edit `tests/node_compat/config.json` and run `deno task setup` in `tests/node_compat/runner` dir instead.

Expand Down Expand Up @@ -1578,7 +1578,6 @@ NOTE: This file should not be manually edited. Please edit `tests/node_compat/co
- [parallel/test-process-env-ignore-getter-setter.js](https://github.com/nodejs/node/tree/v20.11.1/test/parallel/test-process-env-ignore-getter-setter.js)
- [parallel/test-process-env-sideeffects.js](https://github.com/nodejs/node/tree/v20.11.1/test/parallel/test-process-env-sideeffects.js)
- [parallel/test-process-env-symbols.js](https://github.com/nodejs/node/tree/v20.11.1/test/parallel/test-process-env-symbols.js)
- [parallel/test-process-env-tz.js](https://github.com/nodejs/node/tree/v20.11.1/test/parallel/test-process-env-tz.js)
- [parallel/test-process-env.js](https://github.com/nodejs/node/tree/v20.11.1/test/parallel/test-process-env.js)
- [parallel/test-process-euid-egid.js](https://github.com/nodejs/node/tree/v20.11.1/test/parallel/test-process-euid-egid.js)
- [parallel/test-process-exception-capture-errors.js](https://github.com/nodejs/node/tree/v20.11.1/test/parallel/test-process-exception-capture-errors.js)
Expand Down
54 changes: 54 additions & 0 deletions tests/node_compat/test/parallel/test-process-env-tz.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// deno-fmt-ignore-file
// deno-lint-ignore-file

// Copyright Joyent and Node contributors. All rights reserved. MIT license.
// Taken from Node 20.11.1
// This file is automatically generated by `tests/node_compat/runner/setup.ts`. Do not modify this file manually.

'use strict';
const common = require('../common');
const assert = require('assert');

if (!common.isMainThread)
common.skip('process.env.TZ is not intercepted in Workers');

if (common.isWindows) // Using a different TZ format.
common.skip('todo: test on Windows');

const date = new Date('2018-04-14T12:34:56.789Z');

process.env.TZ = 'Europe/Amsterdam';

if (date.toString().includes('(Europe)'))
common.skip('not using bundled ICU'); // Shared library or --with-intl=none.

if ('Sat Apr 14 2018 12:34:56 GMT+0000 (GMT)' === date.toString())
common.skip('missing tzdata'); // Alpine buildbots lack Europe/Amsterdam.

if (date.toString().includes('(Central European Time)') ||
date.toString().includes('(CET)')) {
// The AIX and SmartOS buildbots report 2018 CEST as CET
// because apparently for them that's still the deep future.
common.skip('tzdata too old');
}

// Text representation of timezone depends on locale in environment
assert.match(
date.toString(),
/^Sat Apr 14 2018 14:34:56 GMT\+0200 \(.+\)$/);

process.env.TZ = 'Europe/London';
assert.match(
date.toString(),
/^Sat Apr 14 2018 13:34:56 GMT\+0100 \(.+\)$/);

process.env.TZ = 'Etc/UTC';
assert.match(
date.toString(),
/^Sat Apr 14 2018 12:34:56 GMT\+0000 \(.+\)$/);

// Just check that deleting the environment variable doesn't crash the process.
// We can't really check the result of date.toString() because we don't know
// the default time zone.
delete process.env.TZ;
date.toString();
9 changes: 9 additions & 0 deletions tests/specs/run/tz_env/__test__.jsonc
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"steps": [
{
"if": "unix",
"args": "run -A main.ts",
"output": "ok\n"
}
]
}
13 changes: 13 additions & 0 deletions tests/specs/run/tz_env/main.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
const date = new Date("2018-04-14T12:34:56.789Z");

Deno.env.set("TZ", "Europe/Amsterdam");
if (!date.toString().match(/^Sat Apr 14 2018 14:34:56 GMT\+0200 \(.+\)$/)) {
throw new Error(`date.toString() did not match the expected pattern`);
}

Deno.env.set("TZ", "Europe/London");
if (!date.toString().match(/^Sat Apr 14 2018 13:34:56 GMT\+0100 \(.+\)$/)) {
throw new Error(`date.toString() did not match the expected pattern`);
}

console.log("ok");

0 comments on commit 5c64146

Please sign in to comment.