Skip to content

Commit 9229f1f

Browse files
lib: respect stream.isTTY=false in styleText even when FORCE_COLOR is set
When FORCE_COLOR is set (e.g. by ode --test, which injects it into child-process environments to colorize test output), the shouldColorize() helper in lib/internal/util/colors.js was ignoring the isTTY property of the stream argument entirely. This caused util.styleText() to apply ANSI codes to streams that were explicitly marked as non-TTY by the caller. Fix: if a stream is supplied and its isTTY property is explicitly false, return false even when FORCE_COLOR is set. FORCE_COLOR is intended to force color support in terminals that lie about their capabilities; it should not override an explicit isTTY=false on a caller-supplied stream. Fixes: #57921 Signed-off-by: piyushrajyadav <piyushyadavrajyadav@gmail.com>
1 parent 7b6b21a commit 9229f1f

2 files changed

Lines changed: 43 additions & 1 deletion

File tree

‎lib/internal/util/colors.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,12 @@ module.exports = {
1717
hasColors: false,
1818
shouldColorize(stream) {
1919
if (process.env.FORCE_COLOR !== undefined) {
20+
// If a stream is explicitly provided and it is not a TTY, respect that
21+
// even when FORCE_COLOR is set. FORCE_COLOR should not override an
22+
// explicit isTTY=false on the stream passed by the caller.
23+
if (stream?.isTTY === false) {
24+
return false;
25+
}
2026
return lazyInternalTTY().getColorDepth() > 2;
2127
}
2228
return stream?.isTTY && (

‎test/parallel/test-util-styletext.js‎

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,10 @@ if (fd !== -1) {
199199
{ isTTY: true, env: { NO_COLOR: '1' }, expected: noChange },
200200
{ isTTY: true, env: { FORCE_COLOR: '1' }, expected: styled },
201201
{ isTTY: true, env: { FORCE_COLOR: '1', NODE_DISABLE_COLORS: '1' }, expected: styled },
202-
{ isTTY: false, env: { FORCE_COLOR: '1', NO_COLOR: '1', NODE_DISABLE_COLORS: '1' }, expected: styled },
202+
// When the stream is explicitly non-TTY, FORCE_COLOR must not override it.
203+
// Regression test for https://github.com/nodejs/node/issues/57921
204+
{ isTTY: false, env: { FORCE_COLOR: '1' }, expected: noChange },
205+
{ isTTY: false, env: { FORCE_COLOR: '1', NO_COLOR: '1', NODE_DISABLE_COLORS: '1' }, expected: noChange },
203206
{ isTTY: true, env: { FORCE_COLOR: '1', NO_COLOR: '1', NODE_DISABLE_COLORS: '1' }, expected: styled },
204207
].forEach((testCase) => {
205208
writeStream.isTTY = testCase.isTTY;
@@ -221,3 +224,36 @@ if (fd !== -1) {
221224
} else {
222225
common.skip('Could not create TTY fd');
223226
}
227+
228+
// Regression test for https://github.com/nodejs/node/issues/57921:
229+
// styleText() must honour stream.isTTY === false even when FORCE_COLOR is set
230+
// (e.g. as injected by `node --test` into child-process environments).
231+
{
232+
const originalEnv = process.env;
233+
process.env = { ...process.env, FORCE_COLOR: '1' };
234+
235+
// A plain object with isTTY=false — not a real WriteStream, but validateStream
236+
// is false so the stream-type check is skipped; only isTTY is consulted.
237+
const nonTTYStream = { isTTY: false };
238+
assert.strictEqual(
239+
util.styleText('red', 'test', { stream: nonTTYStream, validateStream: false }),
240+
noChange,
241+
'FORCE_COLOR should not override isTTY=false when a stream is supplied',
242+
);
243+
244+
const ttyStream = { isTTY: true };
245+
assert.strictEqual(
246+
util.styleText('red', 'test', { stream: ttyStream, validateStream: false }),
247+
styled,
248+
'FORCE_COLOR should still enable color on isTTY=true streams',
249+
);
250+
251+
// No stream provided — FORCE_COLOR should still work as before
252+
assert.strictEqual(
253+
util.styleText('red', 'test', { validateStream: false }),
254+
styled,
255+
'FORCE_COLOR should enable color when no stream constraint is set',
256+
);
257+
258+
process.env = originalEnv;
259+
}

0 commit comments

Comments
 (0)