Skip to content

Commit a0922b0

Browse files
authored
fix: handle frozen globalThis in setGlobalDispatcher (#5574)
* fix: handle frozen globalThis in setGlobalDispatcher When Object.freeze(globalThis) is called before undici globals are accessed, setGlobalDispatcher would throw TypeError because it cannot extend globalThis. This fix wraps the Object.defineProperty calls in try/catch. When globalThis is not extensible (frozen), the dispatcher is stored in a module-level fallback variable instead. getGlobalDispatcher is updated to return the fallback dispatcher when the globalThis property is not available. This allows undici to work correctly even when globalThis has been frozen, which is recommended by Node.js security best practices (CWE-349). Fixes issue where Object.freeze(globalThis) breaks undici access. * test: add unit test for frozen globalThis in setGlobalDispatcher Add comprehensive test coverage for the frozen globalThis fix. Tests verify: 1. setGlobalDispatcher does not throw when globalThis is frozen 2. getGlobalDispatcher continues to return a valid dispatcher 3. The fallback mechanism works correctly when globalThis is not extensible This addresses the review feedback from mcollina requesting tests. * test: improve frozen globalThis test coverage Add comprehensive test cases to ensure all code paths in the frozen globalThis fix are exercised. Tests verify: 1. setGlobalDispatcher does not throw when globalThis is frozen 2. getGlobalDispatcher returns a valid dispatcher 3. Fallback dispatcher persists across multiple calls This addresses review feedback requesting tests.
1 parent 354a151 commit a0922b0

2 files changed

Lines changed: 98 additions & 16 deletions

File tree

lib/global.js

Lines changed: 36 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ const { InvalidArgumentError } = require('./core/errors')
88
const Agent = require('./dispatcher/agent')
99
const Dispatcher1Wrapper = require('./dispatcher/dispatcher1-wrapper')
1010

11+
// Fallback storage for when globalThis is not extensible (e.g. frozen)
12+
let fallbackDispatcher
13+
1114
if (getGlobalDispatcher() === undefined) {
1215
setGlobalDispatcher(new Agent())
1316
}
@@ -17,25 +20,42 @@ function setGlobalDispatcher (agent) {
1720
throw new InvalidArgumentError('Argument agent must implement Agent')
1821
}
1922

20-
Object.defineProperty(globalThis, globalDispatcher, {
21-
value: agent,
22-
writable: true,
23-
enumerable: false,
24-
configurable: false
25-
})
26-
27-
const legacyAgent = agent instanceof Dispatcher1Wrapper ? agent : new Dispatcher1Wrapper(agent)
28-
29-
Object.defineProperty(globalThis, legacyGlobalDispatcher, {
30-
value: legacyAgent,
31-
writable: true,
32-
enumerable: false,
33-
configurable: false
34-
})
23+
try {
24+
Object.defineProperty(globalThis, globalDispatcher, {
25+
value: agent,
26+
writable: true,
27+
enumerable: false,
28+
configurable: false
29+
})
30+
} catch (err) {
31+
// globalThis is not extensible (e.g. Object.freeze(globalThis))
32+
// Use fallback storage instead
33+
if (err instanceof TypeError) {
34+
fallbackDispatcher = agent
35+
return
36+
}
37+
throw err
38+
}
39+
40+
try {
41+
const legacyAgent = agent instanceof Dispatcher1Wrapper ? agent : new Dispatcher1Wrapper(agent)
42+
43+
Object.defineProperty(globalThis, legacyGlobalDispatcher, {
44+
value: legacyAgent,
45+
writable: true,
46+
enumerable: false,
47+
configurable: false
48+
})
49+
} catch (err) {
50+
// globalThis is not extensible; fallback storage is already set
51+
if (!(err instanceof TypeError)) {
52+
throw err
53+
}
54+
}
3555
}
3656

3757
function getGlobalDispatcher () {
38-
return globalThis[globalDispatcher]
58+
return globalThis[globalDispatcher] ?? fallbackDispatcher
3959
}
4060

4161
// These are the globals that can be installed by undici.install().

test/global-frozen.js

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
'use strict'
2+
3+
const { tspl } = require('@matteo.collina/tspl')
4+
const { test } = require('node:test')
5+
6+
test('frozen globalThis - setGlobalDispatcher succeeds', (t) => {
7+
t = tspl(t, { plan: 2 })
8+
9+
// Freeze globalThis
10+
Object.freeze(globalThis)
11+
12+
// Dynamically require inside test to get fresh module state
13+
// in a frozen globalThis context
14+
const { setGlobalDispatcher, getGlobalDispatcher } = require('../lib/global')
15+
const Agent = require('../lib/dispatcher/agent')
16+
17+
// Create a new dispatcher and set it - should not throw
18+
const newAgent = new Agent()
19+
let setError = null
20+
try {
21+
setGlobalDispatcher(newAgent)
22+
} catch (err) {
23+
setError = err
24+
}
25+
26+
t.ifError(setError, 'setGlobalDispatcher should not throw with frozen globalThis')
27+
28+
// Verify we can retrieve a dispatcher
29+
const retrieved = getGlobalDispatcher()
30+
t.ok(retrieved, 'getGlobalDispatcher should return a dispatcher')
31+
})
32+
33+
test('frozen globalThis - graceful degradation', (t) => {
34+
t = tspl(t, { plan: 1 })
35+
36+
// globalThis is already frozen from previous test
37+
const { getGlobalDispatcher } = require('../lib/global')
38+
39+
// Should still be able to get a dispatcher without errors
40+
const dispatcher = getGlobalDispatcher()
41+
t.ok(dispatcher !== null && dispatcher !== undefined, 'getGlobalDispatcher should return a valid dispatcher even with frozen globalThis')
42+
})
43+
44+
test('frozen globalThis - fallback dispatcher persists', (t) => {
45+
t = tspl(t, { plan: 2 })
46+
47+
// globalThis is already frozen from previous tests
48+
const { getGlobalDispatcher, setGlobalDispatcher } = require('../lib/global')
49+
const Agent = require('../lib/dispatcher/agent')
50+
51+
// Get current dispatcher
52+
const dispatcher1 = getGlobalDispatcher()
53+
t.ok(dispatcher1, 'First call to getGlobalDispatcher returns dispatcher')
54+
55+
// Set a new one
56+
const newAgent = new Agent()
57+
setGlobalDispatcher(newAgent)
58+
59+
// Get again - should return the one we just set (from fallback)
60+
const dispatcher2 = getGlobalDispatcher()
61+
t.equal(dispatcher2, newAgent, 'getGlobalDispatcher returns dispatcher set in frozen globalThis')
62+
})

0 commit comments

Comments
 (0)