From ed0c6e1ee9e0f1f72620aee7c3d3225c0d49e2fa Mon Sep 17 00:00:00 2001 From: Luigi Pinca Date: Thu, 7 Apr 2022 15:56:39 +0200 Subject: [PATCH] [feature] Introduce the `'redirect'` event Add the ability to remove confidential headers on a per-redirect basis. Closes #2014 --- doc/ws.md | 17 ++ lib/websocket.js | 40 ++++- test/websocket.test.js | 344 +++++++++++++++++++++++++++++++++++------ 3 files changed, 350 insertions(+), 51 deletions(-) diff --git a/doc/ws.md b/doc/ws.md index ed8f04f1c..744112018 100644 --- a/doc/ws.md +++ b/doc/ws.md @@ -24,6 +24,7 @@ - [Event: 'open'](#event-open) - [Event: 'ping'](#event-ping) - [Event: 'pong'](#event-pong) + - [Event: 'redirect'](#event-redirect) - [Event: 'unexpected-response'](#event-unexpected-response) - [Event: 'upgrade'](#event-upgrade) - [websocket.addEventListener(type, listener[, options])](#websocketaddeventlistenertype-listener-options) @@ -361,6 +362,19 @@ Emitted when a ping is received from the server. Emitted when a pong is received from the server. +### Event: 'redirect' + +- `url` {String} +- `request` {http.ClientRequest} + +Emitted before a redirect is followed. `url` is the redirect URL. `request` is +the HTTP GET request with the headers queued. This event gives the ability to +inspect confidential headers and remove them on a per-redirect basis using the +[`request.getHeader()`][] and [`request.removeHeader()`][] API. The `request` +object should be used only for this purpose. When there is at least one listener +for this event, no header is removed by default, even if the redirect is to a +different domain. + ### Event: 'unexpected-response' - `request` {http.ClientRequest} @@ -616,5 +630,8 @@ as configured by the `maxPayload` option. https://nodejs.org/api/https.html#https_https_request_options_callback [permessage-deflate]: https://tools.ietf.org/html/draft-ietf-hybi-permessage-compression-19 +[`request.getheader()`]: https://nodejs.org/api/http.html#requestgetheadername +[`request.removeheader()`]: + https://nodejs.org/api/http.html#requestremoveheadername [`socket.destroy()`]: https://nodejs.org/api/net.html#net_socket_destroy_error [zlib-options]: https://nodejs.org/api/zlib.html#zlib_class_options diff --git a/lib/websocket.js b/lib/websocket.js index 3a56ea069..ca44bc344 100644 --- a/lib/websocket.js +++ b/lib/websocket.js @@ -30,10 +30,11 @@ const { const { format, parse } = require('./extension'); const { toBuffer } = require('./buffer-util'); +const closeTimeout = 30 * 1000; +const kAborted = Symbol('kAborted'); +const protocolVersions = [8, 13]; const readyStates = ['CONNECTING', 'OPEN', 'CLOSING', 'CLOSED']; const subprotocolRegex = /^[!#$%&'*+\-.0-9A-Z^_`|a-z~]+$/; -const protocolVersions = [8, 13]; -const closeTimeout = 30 * 1000; /** * Class representing a WebSocket. @@ -647,7 +648,7 @@ function initAsClient(websocket, address, protocols, options) { hostname: undefined, protocol: undefined, timeout: undefined, - method: undefined, + method: 'GET', host: undefined, path: undefined, port: undefined @@ -701,7 +702,7 @@ function initAsClient(websocket, address, protocols, options) { const defaultPort = isSecure ? 443 : 80; const key = randomBytes(16).toString('base64'); - const get = isSecure ? https.get : http.get; + const request = isSecure ? https.request : http.request; const protocolSet = new Set(); let perMessageDeflate; @@ -766,6 +767,8 @@ function initAsClient(websocket, address, protocols, options) { opts.path = parts[1]; } + let req; + if (opts.followRedirects) { if (websocket._redirects === 0) { websocket._originalHost = parsedUrl.host; @@ -783,7 +786,10 @@ function initAsClient(websocket, address, protocols, options) { options.headers[key.toLowerCase()] = value; } } - } else if (parsedUrl.host !== websocket._originalHost) { + } else if ( + websocket.listenerCount('redirect') === 0 && + parsedUrl.host !== websocket._originalHost + ) { // // Match curl 7.77.0 behavior and drop the following headers. These // headers are also dropped when following a redirect to a subdomain. @@ -803,9 +809,24 @@ function initAsClient(websocket, address, protocols, options) { options.headers.authorization = 'Basic ' + Buffer.from(opts.auth).toString('base64'); } - } - let req = (websocket._req = get(opts)); + req = websocket._req = request(opts); + + if (websocket._redirects) { + // + // Unlike what is done for the `'upgrade'` event, no early exit is + // triggered here if the user calls `websocket.close()` or + // `websocket.terminate()` from a listener of the `'redirect'` event. This + // is because the user can also call `request.destroy()` with an error + // before calling `websocket.close()` or `websocket.terminate()` and this + // would result in an error being emitted on the `request` object with no + // `'error'` event listeners attached. + // + websocket.emit('redirect', websocket.url, req); + } + } else { + req = websocket._req = request(opts); + } if (opts.timeout) { req.on('timeout', () => { @@ -814,7 +835,7 @@ function initAsClient(websocket, address, protocols, options) { } req.on('error', (err) => { - if (req === null || req.aborted) return; + if (req === null || req[kAborted]) return; req = websocket._req = null; emitErrorAndClose(websocket, err); @@ -947,6 +968,8 @@ function initAsClient(websocket, address, protocols, options) { skipUTF8Validation: opts.skipUTF8Validation }); }); + + req.end(); } /** @@ -1007,6 +1030,7 @@ function abortHandshake(websocket, stream, message) { Error.captureStackTrace(err, abortHandshake); if (stream.setHeader) { + stream[kAborted] = true; stream.abort(); if (stream.socket && !stream.socket.destroyed) { diff --git a/test/websocket.test.js b/test/websocket.test.js index e3e11437b..a27b2c278 100644 --- a/test/websocket.test.js +++ b/test/websocket.test.js @@ -645,6 +645,38 @@ describe('WebSocket', () => { ws.close(); }); }); + + it("emits a 'redirect' event", (done) => { + const server = http.createServer(); + const wss = new WebSocket.Server({ noServer: true, path: '/foo' }); + + server.once('upgrade', (req, socket) => { + socket.end('HTTP/1.1 302 Found\r\nLocation: /foo\r\n\r\n'); + server.once('upgrade', (req, socket, head) => { + wss.handleUpgrade(req, socket, head, (ws) => { + ws.close(); + }); + }); + }); + + server.listen(() => { + const port = server.address().port; + const ws = new WebSocket(`ws://localhost:${port}`, { + followRedirects: true + }); + + ws.on('redirect', (url, req) => { + assert.strictEqual(ws._redirects, 1); + assert.strictEqual(url, `ws://localhost:${port}/foo`); + assert.ok(req instanceof http.ClientRequest); + + ws.on('close', (code) => { + assert.strictEqual(code, 1005); + server.close(done); + }); + }); + }); + }); }); describe('Connection establishing', () => { @@ -1175,83 +1207,237 @@ describe('WebSocket', () => { }); describe('When the redirect host is different', () => { - it('drops the `auth` option', (done) => { - const wss = new WebSocket.Server({ port: 0 }, () => { - const port = wss.address().port; + describe("If there is no 'redirect' event listener", () => { + it('drops the `auth` option', (done) => { + const wss = new WebSocket.Server({ port: 0 }, () => { + const port = wss.address().port; + + server.once('upgrade', (req, socket) => { + socket.end( + 'HTTP/1.1 302 Found\r\n' + + `Location: ws://localhost:${port}/\r\n\r\n` + ); + }); - server.once('upgrade', (req, socket) => { - socket.end( - `HTTP/1.1 302 Found\r\nLocation: ws://localhost:${port}/\r\n\r\n` + const ws = new WebSocket( + `ws://localhost:${server.address().port}`, + { + auth: 'foo:bar', + followRedirects: true + } ); + + assert.strictEqual( + ws._req.getHeader('Authorization'), + 'Basic Zm9vOmJhcg==' + ); + + ws.on('close', (code) => { + assert.strictEqual(code, 1005); + assert.strictEqual(ws.url, `ws://localhost:${port}/`); + assert.strictEqual(ws._redirects, 1); + + wss.close(done); + }); }); - const ws = new WebSocket(`ws://localhost:${server.address().port}`, { - auth: 'foo:bar', - followRedirects: true + wss.on('connection', (ws, req) => { + assert.strictEqual(req.headers.authorization, undefined); + ws.close(); }); + }); - assert.strictEqual( - ws._req.getHeader('Authorization'), - 'Basic Zm9vOmJhcg==' - ); + it('drops the Authorization, Cookie, and Host headers', (done) => { + const wss = new WebSocket.Server({ port: 0 }, () => { + const port = wss.address().port; - ws.on('close', (code) => { - assert.strictEqual(code, 1005); - assert.strictEqual(ws.url, `ws://localhost:${port}/`); - assert.strictEqual(ws._redirects, 1); + server.once('upgrade', (req, socket) => { + socket.end( + 'HTTP/1.1 302 Found\r\n' + + `Location: ws://localhost:${port}/\r\n\r\n` + ); + }); - wss.close(done); + const ws = new WebSocket( + `ws://localhost:${server.address().port}`, + { + headers: { + Authorization: 'Basic Zm9vOmJhcg==', + Cookie: 'foo=bar', + Host: 'foo' + }, + followRedirects: true + } + ); + + assert.strictEqual( + ws._req.getHeader('Authorization'), + 'Basic Zm9vOmJhcg==' + ); + assert.strictEqual(ws._req.getHeader('Cookie'), 'foo=bar'); + assert.strictEqual(ws._req.getHeader('Host'), 'foo'); + + ws.on('close', (code) => { + assert.strictEqual(code, 1005); + assert.strictEqual(ws.url, `ws://localhost:${port}/`); + assert.strictEqual(ws._redirects, 1); + + wss.close(done); + }); + }); + + wss.on('connection', (ws, req) => { + assert.strictEqual(req.headers.authorization, undefined); + assert.strictEqual(req.headers.cookie, undefined); + assert.strictEqual( + req.headers.host, + `localhost:${wss.address().port}` + ); + ws.close(); }); }); + }); - wss.on('connection', (ws, req) => { - assert.strictEqual(req.headers.authorization, undefined); - ws.close(); + describe("If there is at least one 'redirect' event listener", () => { + it('does not drop any headers by default', (done) => { + const headers = { + authorization: 'Basic Zm9vOmJhcg==', + cookie: 'foo=bar', + host: 'foo' + }; + + const wss = new WebSocket.Server({ port: 0 }, () => { + const port = wss.address().port; + + server.once('upgrade', (req, socket) => { + socket.end( + 'HTTP/1.1 302 Found\r\n' + + `Location: ws://localhost:${port}/\r\n\r\n` + ); + }); + + const ws = new WebSocket( + `ws://localhost:${server.address().port}`, + { headers, followRedirects: true } + ); + + const firstRequest = ws._req; + + assert.strictEqual( + firstRequest.getHeader('Authorization'), + headers.authorization + ); + assert.strictEqual( + firstRequest.getHeader('Cookie'), + headers.cookie + ); + assert.strictEqual(firstRequest.getHeader('Host'), headers.host); + + ws.on('redirect', (url, req) => { + assert.strictEqual(ws._redirects, 1); + assert.strictEqual(url, `ws://localhost:${port}/`); + assert.notStrictEqual(firstRequest, req); + assert.strictEqual( + req.getHeader('Authorization'), + headers.authorization + ); + assert.strictEqual(req.getHeader('Cookie'), headers.cookie); + assert.strictEqual(req.getHeader('Host'), headers.host); + + ws.on('close', (code) => { + assert.strictEqual(code, 1005); + wss.close(done); + }); + }); + }); + + wss.on('connection', (ws, req) => { + assert.strictEqual( + req.headers.authorization, + headers.authorization + ); + assert.strictEqual(req.headers.cookie, headers.cookie); + assert.strictEqual(req.headers.host, headers.host); + ws.close(); + }); }); }); + }); + + describe("In a listener of the 'redirect' event", () => { + it('allows to abort the request without swallowing errors', (done) => { + server.once('upgrade', (req, socket) => { + socket.end('HTTP/1.1 302 Found\r\nLocation: /foo\r\n\r\n'); + }); + + const port = server.address().port; + const ws = new WebSocket(`ws://localhost:${port}`, { + followRedirects: true + }); + + ws.on('redirect', (url, req) => { + assert.strictEqual(ws._redirects, 1); + assert.strictEqual(url, `ws://localhost:${port}/foo`); - it('drops the Authorization, Cookie, and Host headers', (done) => { + req.on('socket', () => { + req.abort(); + }); + + ws.on('error', (err) => { + assert.ok(err instanceof Error); + assert.strictEqual(err.message, 'socket hang up'); + + ws.on('close', (code) => { + assert.strictEqual(code, 1006); + done(); + }); + }); + }); + }); + + it('allows to remove headers', (done) => { const wss = new WebSocket.Server({ port: 0 }, () => { const port = wss.address().port; server.once('upgrade', (req, socket) => { socket.end( - `HTTP/1.1 302 Found\r\nLocation: ws://localhost:${port}/\r\n\r\n` + 'HTTP/1.1 302 Found\r\n' + + `Location: ws://localhost:${port}/\r\n\r\n` ); }); + const headers = { + authorization: 'Basic Zm9vOmJhcg==', + cookie: 'foo=bar' + }; + const ws = new WebSocket(`ws://localhost:${server.address().port}`, { - headers: { - Authorization: 'Basic Zm9vOmJhcg==', - Cookie: 'foo=bar', - Host: 'foo' - }, + headers, followRedirects: true }); - assert.strictEqual( - ws._req.getHeader('Authorization'), - 'Basic Zm9vOmJhcg==' - ); - assert.strictEqual(ws._req.getHeader('Cookie'), 'foo=bar'); - assert.strictEqual(ws._req.getHeader('Host'), 'foo'); - - ws.on('close', (code) => { - assert.strictEqual(code, 1005); - assert.strictEqual(ws.url, `ws://localhost:${port}/`); + ws.on('redirect', (url, req) => { assert.strictEqual(ws._redirects, 1); + assert.strictEqual(url, `ws://localhost:${port}/`); + assert.strictEqual( + req.getHeader('Authorization'), + headers.authorization + ); + assert.strictEqual(req.getHeader('Cookie'), headers.cookie); - wss.close(done); + req.removeHeader('authorization'); + req.removeHeader('cookie'); + + ws.on('close', (code) => { + assert.strictEqual(code, 1005); + wss.close(done); + }); }); }); wss.on('connection', (ws, req) => { assert.strictEqual(req.headers.authorization, undefined); assert.strictEqual(req.headers.cookie, undefined); - assert.strictEqual( - req.headers.host, - `localhost:${wss.address().port}` - ); ws.close(); }); }); @@ -2172,6 +2358,42 @@ describe('WebSocket', () => { }); }).timeout(4000); + it("can be called from a listener of the 'redirect' event", (done) => { + const server = http.createServer(); + + server.once('upgrade', (req, socket) => { + socket.end('HTTP/1.1 302 Found\r\nLocation: /foo\r\n\r\n'); + }); + + server.listen(() => { + const port = server.address().port; + const ws = new WebSocket(`ws://localhost:${port}`, { + followRedirects: true + }); + + ws.on('open', () => { + done(new Error("Unexpected 'open' event")); + }); + + ws.on('error', (err) => { + assert.ok(err instanceof Error); + assert.strictEqual( + err.message, + 'WebSocket was closed before the connection was established' + ); + + ws.on('close', (code) => { + assert.strictEqual(code, 1006); + server.close(done); + }); + }); + + ws.on('redirect', () => { + ws.close(); + }); + }); + }); + it("can be called from a listener of the 'upgrade' event", (done) => { const wss = new WebSocket.Server({ port: 0 }, () => { const ws = new WebSocket(`ws://localhost:${wss.address().port}`); @@ -2410,6 +2632,42 @@ describe('WebSocket', () => { }); }).timeout(4000); + it("can be called from a listener of the 'redirect' event", (done) => { + const server = http.createServer(); + + server.once('upgrade', (req, socket) => { + socket.end('HTTP/1.1 302 Found\r\nLocation: /foo\r\n\r\n'); + }); + + server.listen(() => { + const port = server.address().port; + const ws = new WebSocket(`ws://localhost:${port}`, { + followRedirects: true + }); + + ws.on('open', () => { + done(new Error("Unexpected 'open' event")); + }); + + ws.on('error', (err) => { + assert.ok(err instanceof Error); + assert.strictEqual( + err.message, + 'WebSocket was closed before the connection was established' + ); + + ws.on('close', (code) => { + assert.strictEqual(code, 1006); + server.close(done); + }); + }); + + ws.on('redirect', () => { + ws.terminate(); + }); + }); + }); + it("can be called from a listener of the 'upgrade' event", (done) => { const wss = new WebSocket.Server({ port: 0 }, () => { const ws = new WebSocket(`ws://localhost:${wss.address().port}`);