Skip to content

Commit

Permalink
Allow method rewriting on redirects (#913)
Browse files Browse the repository at this point in the history
  • Loading branch information
@szmarczak
szmarczak committed Nov 5, 2019
1 parent e09dfcd commit b7ead5f
Show file tree
Hide file tree
Showing 4 changed files with 174 additions and 47 deletions.
2 changes: 2 additions & 0 deletions readme.md
View file
Expand Up @@ -368,6 +368,8 @@ Defines if redirect responses should be followed automatically.

Note that if a `303` is sent by the server in response to any request type (`POST`, `DELETE`, etc.), Got will automatically request the resource pointed to in the location header via `GET`. This is in accordance with [the spec](https://tools.ietf.org/html/rfc7231#section-6.4.4).

This supports [method rewriting](https://tools.ietf.org/html/rfc7231#section-6.4). For example, when sending a POST request and receiving a `302`, it will resend that request to the new location.

###### maxRedirects

Type: `number`<br>
Expand Down
75 changes: 37 additions & 38 deletions source/request-as-event-emitter.ts
View file
Expand Up @@ -20,13 +20,8 @@ import urlToOptions from './utils/url-to-options';
import {RequestFunction, NormalizedOptions, Response, ResponseObject, AgentByProtocol} from './utils/types';
import dynamicRequire from './utils/dynamic-require';

export type GetMethodRedirectCodes = 300 | 301 | 302 | 303 | 304 | 305 | 307 | 308;
export type AllMethodRedirectCodes = 300 | 303 | 307 | 308;
export type WithoutBody = 'GET' | 'HEAD';

const getMethodRedirectCodes: ReadonlySet<GetMethodRedirectCodes> = new Set([300, 301, 302, 303, 304, 305, 307, 308]);
const allMethodRedirectCodes: ReadonlySet<AllMethodRedirectCodes> = new Set([300, 303, 307, 308]);
const withoutBody: ReadonlySet<WithoutBody> = new Set(['GET', 'HEAD']);
const redirectCodes: ReadonlySet<number> = new Set([300, 301, 302, 303, 304, 307, 308]);
const withoutBody: ReadonlySet<string> = new Set(['GET', 'HEAD']);

export interface RequestAsEventEmitter extends EventEmitter {
retry: <T extends Error>(error: T) => boolean;
Expand Down Expand Up @@ -143,46 +138,50 @@ export default (options: NormalizedOptions, input?: TransformStream) => {
await Promise.all(promises);
}

if (options.followRedirect && 'location' in typedResponse.headers) {
if (allMethodRedirectCodes.has(statusCode as AllMethodRedirectCodes) || (getMethodRedirectCodes.has(statusCode as GetMethodRedirectCodes) && (options.method === 'GET' || options.method === 'HEAD'))) {
typedResponse.resume(); // We're being redirected, we don't care about the response.

if (statusCode === 303) {
// Server responded with "see other", indicating that the resource exists at another location,
// and the client should request it from that location via GET or HEAD.
options.method = 'GET';
}
if (options.followRedirect && Reflect.has(typedResponse.headers, 'location') && redirectCodes.has(statusCode)) {
typedResponse.resume(); // We're being redirected, we don't care about the response.

if (redirects.length >= options.maxRedirects) {
throw new MaxRedirectsError(typedResponse, options.maxRedirects, options);
}
if (statusCode === 303 && options.method !== 'GET' && options.method !== 'HEAD') {
// Server responded with "see other", indicating that the resource exists at another location,
// and the client should request it from that location via GET or HEAD.
options.method = 'GET';

// Handles invalid URLs. See https://github.com/sindresorhus/got/issues/604
const redirectBuffer = Buffer.from(typedResponse.headers.location, 'binary').toString();
const redirectURL = new URL(redirectBuffer, currentUrl);
redirectString = redirectURL.toString();
delete options.json;
delete options.form;
delete options.body;
}

redirects.push(redirectString);
if (redirects.length >= options.maxRedirects) {
throw new MaxRedirectsError(typedResponse, options.maxRedirects, options);
}

const redirectOptions = {
...options,
port: undefined,
auth: undefined,
...urlToOptions(redirectURL)
};
// Handles invalid URLs. See https://github.com/sindresorhus/got/issues/604
const redirectBuffer = Buffer.from(typedResponse.headers.location, 'binary').toString();
const redirectURL = new URL(redirectBuffer, currentUrl);
redirectString = redirectURL.toString();

for (const hook of options.hooks.beforeRedirect) {
// eslint-disable-next-line no-await-in-loop
await hook(redirectOptions, typedResponse);
}
redirects.push(redirectString);

emitter.emit('redirect', response, redirectOptions);
const redirectOptions = {
...options,
port: undefined,
auth: undefined,
...urlToOptions(redirectURL)
};

await get(redirectOptions);
return;
for (const hook of options.hooks.beforeRedirect) {
// eslint-disable-next-line no-await-in-loop
await hook(redirectOptions, typedResponse);
}

emitter.emit('redirect', response, redirectOptions);

await get(redirectOptions);
return;
}

delete options.body;

getResponse(typedResponse, options, emitter);
} catch (error) {
emitError(error);
Expand Down Expand Up @@ -362,7 +361,7 @@ export default (options: NormalizedOptions, input?: TransformStream) => {
const isForm = !is.nullOrUndefined(options.form);
const isJSON = !is.nullOrUndefined(options.json);
const isBody = !is.nullOrUndefined(body);
if ((isBody || isForm || isJSON) && withoutBody.has(options.method as WithoutBody)) {
if ((isBody || isForm || isJSON) && withoutBody.has(options.method)) {
throw new TypeError(`The \`${options.method}\` method cannot be used with a body`);
}

Expand Down
64 changes: 64 additions & 0 deletions test/arguments.ts
View file
Expand Up @@ -313,3 +313,67 @@ test('`context` option is accessible when extending instances', t => {
t.is(instance.defaults.options.context, context);
t.false({}.propertyIsEnumerable.call(instance.defaults.options, 'context'));
});

test('`options.body` is cleaned up when retrying - `options.json`', withServer, async (t, server, got) => {
let first = true;
server.post('/', (_request, response) => {
if (first) {
first = false;

response.statusCode = 401;
}

response.end();
});

await t.notThrowsAsync(got.post('', {
hooks: {
afterResponse: [
async (response, retryWithMergedOptions) => {
if (response.statusCode === 401) {
return retryWithMergedOptions();
}

t.is(response.request.options.body, undefined);

return response;
}
]
},
json: {
some: 'data'
}
}));
});

test('`options.body` is cleaned up when retrying - `options.form`', withServer, async (t, server, got) => {
let first = true;
server.post('/', (_request, response) => {
if (first) {
first = false;

response.statusCode = 401;
}

response.end();
});

await t.notThrowsAsync(got.post('', {
hooks: {
afterResponse: [
async (response, retryWithMergedOptions) => {
if (response.statusCode === 401) {
return retryWithMergedOptions();
}

t.is(response.request.options.body, undefined);

return response;
}
]
},
form: {
some: 'data'
}
}));
});
80 changes: 71 additions & 9 deletions test/redirects.ts
View file
Expand Up @@ -120,18 +120,30 @@ test('hostname + path are not breaking redirects', withServer, async (t, server,
})).body, 'reached');
});

test('redirects only GET and HEAD requests', withServer, async (t, server, got) => {
server.post('/', relativeHandler);
test('redirects GET and HEAD requests', withServer, async (t, server, got) => {
server.get('/', (_request, response) => {
response.writeHead(308, {
location: '/'
});
response.end();
});

const error = await t.throwsAsync(got.post({body: 'wow'}), {
instanceOf: got.HTTPError,
message: 'Response code 302 (Found)'
await t.throwsAsync(got.get(''), {
instanceOf: got.MaxRedirectsError
});
});

// @ts-ignore
t.is(error.options.path, '/');
// @ts-ignore
t.is(error.response.statusCode, 302);
test('redirects POST requests', withServer, async (t, server, got) => {
server.post('/', (_request, response) => {
response.writeHead(308, {
location: '/'
});
response.end();
});

await t.throwsAsync(got.post({body: 'wow'}), {
instanceOf: got.MaxRedirectsError
});
});

test('redirects on 303 response even on post, put, delete', withServer, async (t, server, got) => {
Expand Down Expand Up @@ -279,3 +291,53 @@ test('port is reset on redirect', withServer, async (t, server, got) => {
const {body} = await got('');
t.is(body, 'ok');
});

test('body is reset on GET redirect', withServer, async (t, server, got) => {
server.post('/', (_request, response) => {
response.writeHead(303, {
location: '/'
});
response.end();
});

server.get('/', (_request, response) => {
response.end();
});

await got.post('', {
body: 'foobar',
hooks: {
beforeRedirect: [
options => {
t.is(options.body, undefined);
}
]
}
});
});

test('body is passed on POST redirect', withServer, async (t, server, got) => {
server.post('/redirect', (_request, response) => {
response.writeHead(302, {
location: '/'
});
response.end();
});

server.post('/', (request, response) => {
request.pipe(response);
});

const {body} = await got.post('redirect', {
body: 'foobar',
hooks: {
beforeRedirect: [
options => {
t.is(options.body, 'foobar');
}
]
}
});

t.is(body, 'foobar');
});

0 comments on commit b7ead5f

Please sign in to comment.