Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

backport of #1449 #1453

Merged
merged 2 commits into from Jan 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
@@ -1,6 +1,6 @@
{
"name": "node-fetch",
"version": "2.6.6",
"version": "2.6.7",
"description": "A light-weight module that brings window.fetch to node.js",
"main": "lib/index.js",
"browser": "./browser.js",
Expand Down
49 changes: 40 additions & 9 deletions src/index.js
Expand Up @@ -13,16 +13,29 @@ import https from 'https';
import zlib from 'zlib';
import Stream from 'stream';

import Body, { writeToStream, getTotalBytes } from './body';
import Response from './response';
import Headers, { createHeadersLenient } from './headers';
import Request, { getNodeRequestOptions } from './request';
import FetchError from './fetch-error';
import AbortError from './abort-error';
import Body, { writeToStream, getTotalBytes } from './body.js';
import Response from './response.js';
import Headers, { createHeadersLenient } from './headers.js';
import Request, { getNodeRequestOptions } from './request.js';
import FetchError from './fetch-error.js';
import AbortError from './abort-error.js';

import whatwgUrl from 'whatwg-url';

const URL = Url.URL || whatwgUrl.URL;

// fix an issue where "PassThrough", "resolve" aren't a named export for node <10
const PassThrough = Stream.PassThrough;
const resolve_url = Url.resolve;

const isDomainOrSubdomain = (destination, original) => {
const orig = new URL(original).hostname;
const dest = new URL(destination).hostname;

return orig === dest || (
orig[orig.length - dest.length - 1] === '.' && orig.endsWith(dest)
);
};


/**
* Fetch function
Expand Down Expand Up @@ -109,7 +122,19 @@ export default function fetch(url, opts) {
const location = headers.get('Location');

// HTTP fetch step 5.3
const locationURL = location === null ? null : resolve_url(request.url, location);
let locationURL = null;
try {
locationURL = location === null ? null : new URL(location, request.url).toString();
} catch (err) {
// error here can only be invalid URL in Location: header
// do not throw when options.redirect == manual
// let the user extract the errorneous redirect URL
if (request.redirect !== 'manual') {
reject(new FetchError(`uri requested responds with an invalid redirect URL: ${location}`, 'invalid-redirect'));
finalize();
return;
}
}

// HTTP fetch step 5.5
switch (request.redirect) {
Expand Down Expand Up @@ -154,9 +179,15 @@ export default function fetch(url, opts) {
body: request.body,
signal: request.signal,
timeout: request.timeout,
size: request.size
size: request.size
};

if (!isDomainOrSubdomain(request.url, locationURL)) {
for (const name of ['authorization', 'www-authenticate', 'cookie', 'cookie2']) {
requestOpts.headers.delete(name);
}
}

// HTTP-redirect fetch step 9
if (res.statusCode !== 303 && request.body && getTotalBytes(request) === null) {
reject(new FetchError('Cannot follow redirect with body being a readable stream', 'unsupported-redirect'));
Expand Down
7 changes: 6 additions & 1 deletion test/server.js
@@ -1,7 +1,6 @@
import * as http from 'http';
import { parse } from 'url';
import * as zlib from 'zlib';
import * as stream from 'stream';
import { multipart as Multipart } from 'parted';

let convert;
Expand Down Expand Up @@ -66,6 +65,12 @@ export default class TestServer {
}));
}

if (p.startsWith('/redirect-to/3')) {
res.statusCode = p.slice(13, 16);
res.setHeader('Location', p.slice(17));
res.end();
}

if (p === '/gzip') {
res.statusCode = 200;
res.setHeader('Content-Type', 'text/plain');
Expand Down
47 changes: 47 additions & 0 deletions test/test.js
Expand Up @@ -1569,6 +1569,53 @@ describe('node-fetch', () => {
});
});

it('should not forward secure headers to 3th party', () => {
return fetch(`${base}redirect-to/302/https://httpbin.org/get`, {
headers: new Headers({
cookie: 'gets=removed',
cookie2: 'gets=removed',
authorization: 'gets=removed',
'www-authenticate': 'gets=removed',
'other-safe-headers': 'stays',
'x-foo': 'bar'
})
}).then(res => res.json()).then(json => {
const headers = new Headers(json.headers);
// Safe headers are not removed
expect(headers.get('other-safe-headers')).to.equal('stays');
expect(headers.get('x-foo')).to.equal('bar');
// Unsafe headers should not have been sent to httpbin
expect(headers.get('cookie')).to.equal(null);
expect(headers.get('cookie2')).to.equal(null);
expect(headers.get('www-authenticate')).to.equal(null);
expect(headers.get('authorization')).to.equal(null);
});
});

it('should forward secure headers to same host', () => {
return fetch(`${base}redirect-to/302/${base}inspect`, {
headers: new Headers({
cookie: 'is=cookie',
cookie2: 'is=cookie2',
authorization: 'is=authorization',
'other-safe-headers': 'stays',
'www-authenticate': 'is=www-authenticate',
'x-foo': 'bar'
})
}).then(res => res.json().then(json => {
const headers = new Headers(json.headers);
// Safe headers are not removed
expect(res.url).to.equal(`${base}inspect`);
expect(headers.get('other-safe-headers')).to.equal('stays');
expect(headers.get('x-foo')).to.equal('bar');
// Unsafe headers should not have been sent to httpbin
expect(headers.get('cookie')).to.equal('is=cookie');
expect(headers.get('cookie2')).to.equal('is=cookie2');
expect(headers.get('www-authenticate')).to.equal('is=www-authenticate');
expect(headers.get('authorization')).to.equal('is=authorization');
}));
});

it('should allow PATCH request', function() {
const url = `${base}inspect`;
const opts = {
Expand Down