Allow to drop custom sensitive headers when following redirects#2014
Allow to drop custom sensitive headers when following redirects#2014vansergen wants to merge 3 commits intowebsockets:masterfrom vansergen:redirects
Conversation
|
I prefer to not do this. The user should enable the I also don't like the current behavior but it is similar to what many popular HTTP clients do and should be enough to prevent "security researchers" from reporting a security vulnerability for this. |
Agree. But one of the use cases is the redirect between two trusted servers with different subdomains (
Indeed, I chose the easiest way to achieve that |
It does not matter as long as We can eventually add the |
There is 👍 The |
|
There are a few things to keep in mind:
I was thinking about emitting the event right after creating the request, before ending it and giving the user the whole diff --git a/lib/websocket.js b/lib/websocket.js
index 6fff935..81fe59c 100644
--- a/lib/websocket.js
+++ b/lib/websocket.js
@@ -647,7 +647,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 +701,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 +766,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 +785,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 +808,16 @@ 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.listenerCount('redirect') !== 0) {
+ websocket.emit('redirect', websocket.url, req);
+ if (websocket.readyState !== WebSocket.CONNECTING) return;
+ }
+ } else {
+ req = websocket._req = request(opts);
+ }
if (opts.timeout) {
req.on('timeout', () => {
@@ -947,6 +959,8 @@ function initAsClient(websocket, address, protocols, options) {
skipUTF8Validation: opts.skipUTF8Validation
});
});
+
+ req.end();
}
/**This would give the user the ability to inspect, add, and remove headers using the Node.js API ( Anyway I'm a bit short on time, and I'm not sure if it is worth the effort. Feel free to take a stab at it. |
Add the ability to remove confidential headers on a per-redirect basis. Closes #2014
Add the ability to remove confidential headers on a per-redirect basis. Closes #2014
Add the ability to remove confidential headers on a per-redirect basis. Closes #2014
@lpinca What do you think about using the existing option
followRedirectsto achieve that?