Skip to content

Commit

Permalink
Update redirection strategies to drop confidential headers
Browse files Browse the repository at this point in the history
  • Loading branch information
Avaq committed Mar 1, 2022
1 parent a17d0bd commit 125e447
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 11 deletions.
41 changes: 33 additions & 8 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -534,13 +534,20 @@ export const matchStatus = f => fs => res => {
return (hasProp (statusCode) (fs) ? fs[statusCode] : f) (res);
};

// mergeUrls :: Url -> Any -> String
const mergeUrls = base => input => (
// mergeUrls :: (Url, Any) -> String
const mergeUrls = (base, input) => (
typeof input === 'string' ?
new URL (input, base).href :
base
);

// sameHost :: (Url, Url) -> Boolean
const sameHost = (parent, child) => {
const p = new URL (parent);
const c = new URL (child);
return p.host === c.host || c.host.endsWith ('.' + p.host);
};

// overHeaders :: (Request, Array2 String String -> Array2 String String)
// -> Request
const overHeaders = (request, f) => {
Expand All @@ -551,21 +558,34 @@ const overHeaders = (request, f) => {
(Request.body (request));
};

// confidentialHeaders :: Array String
const confidentialHeaders = [
'authorization',
'cookie',
];

//# redirectAnyRequest :: Response -> Request
//.
//. A redirection strategy that simply reissues the original Request to the
//. Location specified in the given Response.
//.
//. If the new location is on an external host, then any confidential headers
//. (such as the cookie header) will be dropped from the new request.
//.
//. Used in the [`defaultRedirectionPolicy`](#defaultRedirectionPolicy) and
//. the [`aggressiveRedirectionPolicy`](#aggressiveRedirectionPolicy).
export const redirectAnyRequest = response => {
const {headers: {location}} = Response.message (response);
const original = Response.request (response);
const oldUrl = Request.url (original);
const newUrl = mergeUrls (oldUrl) (location);
return (Request (Request.options (original))
(newUrl)
(Request.body (original)));
const newUrl = mergeUrls (oldUrl, location);
const request = Request (Request.options (original))
(newUrl)
(Request.body (original));

return sameHost (oldUrl, newUrl) ? request : overHeaders (request, xs => (
xs.filter (([name]) => !confidentialHeaders.includes (name.toLowerCase ()))
));
};

//# redirectIfGetMethod :: Response -> Request
Expand All @@ -574,6 +594,9 @@ export const redirectAnyRequest = response => {
//. Location specified in the given Response, but only if the original request
//. was using the GET method.
//.
//. If the new location is on an external host, then any confidential headers
//. (such as the cookie header) will be dropped from the new request.
//.
//. Used in [`followRedirectsStrict`](#followRedirectsStrict).
export const redirectIfGetMethod = response => {
const {method} = cleanRequestOptions (Response.request (response));
Expand All @@ -590,8 +613,10 @@ export const redirectIfGetMethod = response => {
//. request to the Location specified in the given Response. If the response
//. does not contain a valid location, the request is not redirected.
//.
//. The original request method and body are discarded, but all the options
//. are preserved.
//. The original request method and body are discarded, but other options
//. are preserved. If the new location is on an external host, then any
//. confidential headers (such as the cookie header) will be dropped from the
//. new request.
//.
//. Used in the [`defaultRedirectionPolicy`](#defaultRedirectionPolicy) and
//. the [`aggressiveRedirectionPolicy`](#aggressiveRedirectionPolicy).
Expand Down
26 changes: 23 additions & 3 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -259,18 +259,38 @@ test ('redirectAnyRequest', () => Promise.all ([
assertResolves (fl.map (fn.redirectAnyRequest) (mockResponse ({})))
(getRequest),
assertResolves (fl.map (fn.redirectAnyRequest) (getResponse (301) ('ftp://xxx')))
(fn.Request ({}) ('ftp://xxx/') (fn.emptyStream)),
(fn.Request ({headers: {}}) ('ftp://xxx/') (fn.emptyStream)),
assertResolves (fl.map (fn.redirectAnyRequest) (getResponse (301) ('/echo')))
(fn.Request ({}) ('https://example.com/echo') (fn.emptyStream)),
assertResolves (fl.map (fn.redirectAnyRequest) (postResponse (301) ('/echo')))
(fn.Request ({method: 'POST'}) ('https://example.com/echo') (fn.streamOf (Buffer.from ('test')))),
assertResolves (fl.map (fn.redirectAnyRequest)
(mockResponse ({code: 301,
headers: {location: 'https://example.com/path'},
request: fn.Request ({headers: {cookie: 'yum'}}) ('https://example.com') (fn.emptyStream)})))
(fn.Request ({headers: {cookie: 'yum'}}) ('https://example.com/path') (fn.emptyStream)),
assertResolves (fl.map (fn.redirectAnyRequest)
(mockResponse ({code: 301,
headers: {location: 'https://sub.example.com/'},
request: fn.Request ({headers: {cookie: 'yum'}}) ('https://example.com') (fn.emptyStream)})))
(fn.Request ({headers: {cookie: 'yum'}}) ('https://sub.example.com/') (fn.emptyStream)),
assertResolves (fl.map (fn.redirectAnyRequest)
(mockResponse ({code: 301,
headers: {location: 'https://bigsub.example.com/'},
request: fn.Request ({headers: {cookie: 'yum'}}) ('https://example.com') (fn.emptyStream)})))
(fn.Request ({headers: {cookie: 'yum'}}) ('https://bigsub.example.com/') (fn.emptyStream)),
assertResolves (fl.map (fn.redirectAnyRequest)
(mockResponse ({code: 301,
headers: {location: 'https://elsewhere.com/'},
request: fn.Request ({headers: {cookie: 'yum'}}) ('https://example.com') (fn.emptyStream)})))
(fn.Request ({headers: {}}) ('https://elsewhere.com/') (fn.emptyStream)),
]));

test ('redirectIfGetMethod', () => Promise.all ([
assertResolves (fl.map (fn.redirectIfGetMethod) (mockResponse ({})))
(getRequest),
assertResolves (fl.map (fn.redirectIfGetMethod) (getResponse (301) ('ftp://xxx')))
(fn.Request ({}) ('ftp://xxx/') (fn.emptyStream)),
(fn.Request ({headers: {}}) ('ftp://xxx/') (fn.emptyStream)),
assertResolves (fl.map (fn.redirectIfGetMethod) (getResponse (301) ('/echo')))
(fn.Request ({}) ('https://example.com/echo') (fn.emptyStream)),
assertResolves (fl.map (fn.redirectIfGetMethod) (postResponse (301) ('/echo')))
Expand All @@ -281,7 +301,7 @@ test ('redirectUsingGetMethod', () => Promise.all ([
assertResolves (fl.map (fn.redirectUsingGetMethod) (mockResponse ({})))
(fn.Request ({method: 'GET'}) ('https://example.com') (fn.emptyStream)),
assertResolves (fl.map (fn.redirectUsingGetMethod) (getResponse (301) ('ftp://xxx')))
(fn.Request ({method: 'GET'}) ('ftp://xxx/') (fn.emptyStream)),
(fn.Request ({method: 'GET', headers: {}}) ('ftp://xxx/') (fn.emptyStream)),
assertResolves (fl.map (fn.redirectUsingGetMethod) (getResponse (200) ('/echo')))
(fn.Request ({method: 'GET'}) ('https://example.com/echo') (fn.emptyStream)),
assertResolves (fl.map (fn.redirectUsingGetMethod) (postResponse (200) ('/echo')))
Expand Down

0 comments on commit 125e447

Please sign in to comment.