Skip to content

Commit 4f4bf4f

Browse files
authored
fix: avoid 401 failures for stream-backed request bodies (#4941)
* fix: avoid 401 failures for stream-backed request bodies - Properly implement isTraversableNavigable() to return false in Node.js - Return 401 response directly for stream-backed bodies instead of throwing network error - Aligns with Fetch spec discussion in whatwg/fetch#1132 - Add regression tests for PUT with ReadableStream and POST with JSON body * fix: preserve 401 auth retries for URL credentials
1 parent ce45abf commit 4f4bf4f

3 files changed

Lines changed: 63 additions & 4 deletions

File tree

lib/web/fetch/index.js

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1639,12 +1639,25 @@ async function httpNetworkOrCacheFetch (
16391639
// 14. If response’s status is 401, httpRequest’s response tainting is not "cors",
16401640
// includeCredentials is true, and request’s traversable for user prompts is
16411641
// a traversable navigable:
1642-
if (response.status === 401 && httpRequest.responseTainting !== 'cors' && includeCredentials && isTraversableNavigable(request.traversableForUserPrompts)) {
1642+
//
1643+
// In Node.js there is no traversable navigable to prompt the user, but we
1644+
// still need to handle URL-embedded credentials so authentication retries
1645+
// for WebSocket handshakes continue to work.
1646+
if (response.status === 401 && httpRequest.responseTainting !== 'cors' && includeCredentials && (
1647+
request.useURLCredentials !== undefined ||
1648+
isTraversableNavigable(request.traversableForUserPrompts)
1649+
)) {
16431650
// 2. If request’s body is non-null, then:
16441651
if (request.body != null) {
16451652
// 1. If request’s body’s source is null, then return a network error.
16461653
if (request.body.source == null) {
1647-
return makeNetworkError('expected non-null body source')
1654+
// Note: In Node.js, this code path should not be reached because
1655+
// isTraversableNavigable() returns false for non-navigable contexts.
1656+
// However, we handle it gracefully by returning the response instead of
1657+
// a network error, as we won't actually retry the request.
1658+
// This aligns with the Fetch spec discussion in whatwg/fetch#1132,
1659+
// which allows implementations flexibility when credentials can't be obtained.
1660+
return response
16481661
}
16491662

16501663
// 2. Set request’s body to the body of the result of safely extracting

lib/web/fetch/util.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1447,8 +1447,10 @@ function includesCredentials (url) {
14471447
* @param {object|string} navigable
14481448
*/
14491449
function isTraversableNavigable (navigable) {
1450-
// TODO
1451-
return true
1450+
// Returns true only if we have an actual traversable navigable object
1451+
// that can prompt the user for credentials. In Node.js, this will always
1452+
// be false since there's no Window object or navigable.
1453+
return navigable != null && navigable !== 'client' && navigable !== 'no-traversable'
14521454
}
14531455

14541456
class EnvironmentSettingsObjectBase {

test/fetch/401-statuscode-no-infinite-loop.js

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,47 @@ test('Receiving a 401 status code should not cause infinite retry loop', async (
2020
const response = await fetch(`http://localhost:${server.address().port}`)
2121
assert.strictEqual(response.status, 401)
2222
})
23+
24+
test('Receiving a 401 status code should not fail for stream-backed request bodies', async (t) => {
25+
const server = createServer({ joinDuplicateHeaders: true }, (req, res) => {
26+
res.statusCode = 401
27+
res.end('Unauthorized')
28+
}).listen(0)
29+
30+
t.after(closeServerAsPromise(server))
31+
await once(server, 'listening')
32+
33+
const response = await fetch(`http://localhost:${server.address().port}`, {
34+
method: 'PUT',
35+
duplex: 'half',
36+
body: new ReadableStream({
37+
start (controller) {
38+
controller.enqueue(Buffer.from('hello world'))
39+
controller.close()
40+
}
41+
})
42+
})
43+
44+
assert.strictEqual(response.status, 401)
45+
})
46+
47+
test('Receiving a 401 status code should work for POST with JSON body', async (t) => {
48+
const server = createServer({ joinDuplicateHeaders: true }, (req, res) => {
49+
res.statusCode = 401
50+
res.setHeader('Content-Type', 'application/json')
51+
res.end(JSON.stringify({ error: 'unauthorized' }))
52+
}).listen(0)
53+
54+
t.after(closeServerAsPromise(server))
55+
await once(server, 'listening')
56+
57+
const response = await fetch(`http://localhost:${server.address().port}`, {
58+
method: 'POST',
59+
headers: { 'Content-Type': 'application/json' },
60+
body: JSON.stringify({ data: 'test' })
61+
})
62+
63+
assert.strictEqual(response.status, 401)
64+
const body = await response.json()
65+
assert.deepStrictEqual(body, { error: 'unauthorized' })
66+
})

0 commit comments

Comments
 (0)