-
-
Notifications
You must be signed in to change notification settings - Fork 729
Description
Bug Description
The includesCredentials() function in lib/web/fetch/util.js uses the && (AND) operator instead of || (OR) when checking if a URL includes credentials. This contradicts the WHATWG URL Standard §4.4:
A URL includes credentials if its username is not the empty string or its password is not the empty string.
Affected Code
lib/web/fetch/util.js, lines 1437-1440:
function includesCredentials (url) {
// A URL includes credentials if its username or password is not the empty string.
return !!(url.username && url.password)
}Note the comment correctly states "or" while the code implements "and."
Impact
This causes the 401 authentication retry path in httpNetworkOrCacheFetch to skip constructing the Authorization header when a URL contains only a username or only a password:
} else if (includesCredentials(requestCurrentURL(httpRequest)) && isAuthenticationFetch) {
const { username, password } = requestCurrentURL(httpRequest);
authorizationValue = `Basic ${Buffer.from(`${username}:${password}`).toString("base64")}`;
}When includesCredentials() incorrectly returns false, authorizationValue remains null and no Authorization header is sent on the retry. This primarily affects WebSocket connections (which set useURLCredentials: true), causing silent authentication failure for token-as-username patterns like ws://apitoken@host/path.
Reproduction
Tested on Node.js v25.6.0 (bundled undici):
const net = require('net');
function runTest(port, url, label) {
return new Promise((resolve) => {
let connections = [];
const server = net.createServer((socket) => {
let data = '';
socket.on('data', (chunk) => {
data += chunk.toString();
const authMatch = data.match(/Authorization: (.+)\r\n/);
connections.push({
hasAuth: !!authMatch,
authValue: authMatch ? authMatch[1] : 'NONE',
});
socket.write(
'HTTP/1.1 401 Unauthorized\r\nWWW-Authenticate: Basic realm="test"\r\n' +
'Content-Length: 0\r\nConnection: close\r\n\r\n'
);
socket.end();
});
});
server.listen(port, () => {
const ws = new WebSocket(url);
ws.addEventListener('error', () => {});
ws.addEventListener('close', () => {
setTimeout(() => { server.close(); resolve({ label, connections }); }, 500);
});
setTimeout(() => { server.close(); resolve({ label, connections }); }, 3000);
});
});
}
(async () => {
const r1 = await runTest(5070, 'ws://admin:secret@localhost:5070/test', 'user:pass');
const r2 = await runTest(5071, 'ws://admin@localhost:5071/test', 'user only');
const r3 = await runTest(5072, 'ws://:secret@localhost:5072/test', 'pass only');
console.log('user:pass -> Auth on retry:', r1.connections[1]?.authValue);
console.log('user only -> Auth on retry:', r2.connections[1]?.authValue);
console.log('pass only -> Auth on retry:', r3.connections[1]?.authValue);
process.exit(0);
})();Results:
| URL pattern | Auth on 401 retry | Expected |
|---|---|---|
ws://user:pass@host |
Basic dXNlcjpwYXNz |
Sent |
ws://user@host |
NONE |
Should be sent |
ws://:pass@host |
NONE |
Should be sent |
Expected Fix
function includesCredentials (url) {
// A URL includes credentials if its username or password is not the empty string.
- return !!(url.username && url.password)
+ return !!(url.username || url.password)
}