From e049e945bd6148207578eb00eabf60426e8c1101 Mon Sep 17 00:00:00 2001 From: Josua Hunziker Date: Sun, 18 Sep 2022 20:03:11 +0200 Subject: [PATCH] Improve docs and disable method rewriting on 307 and 308 (#2145) --- documentation/2-options.md | 9 ++++++--- source/core/index.ts | 9 +++++---- source/core/options.ts | 9 ++++++--- test/redirects.ts | 26 +++++++++++++++++++++++++- 4 files changed, 42 insertions(+), 11 deletions(-) diff --git a/documentation/2-options.md b/documentation/2-options.md index bc39348ba..088d3c052 100644 --- a/documentation/2-options.md +++ b/documentation/2-options.md @@ -692,7 +692,7 @@ Defines if redirect responses should be followed automatically. #### **Note:** > - If a `303` is sent by the server in response to any request type (POST, DELETE, etc.), Got will request the resource pointed to in the location header via GET.\ -> This is in accordance with the [specification](https://tools.ietf.org/html/rfc7231#section-6.4.4). +> This is in accordance with the [specification](https://tools.ietf.org/html/rfc7231#section-6.4.4). You can optionally turn on this behavior also for other redirect codes - see [`methodRewriting`](#methodrewriting). ```js import got from 'got'; @@ -936,9 +936,12 @@ Optionally overrides the value of [`--max-http-header-size`](https://nodejs.org/ **Type: `boolean`**\ **Default: `false`** -By default, requests will not use [method rewriting](https://datatracker.ietf.org/doc/html/rfc7231#section-6.4). +Specifies if the HTTP request method should be [rewritten as `GET`](https://tools.ietf.org/html/rfc7231#section-6.4) on redirects. -For example, when sending a `POST` request and receiving a `302`, it will resend the body to the new location using the same HTTP method (`POST` in this case). To rewrite the request as `GET`, set this option to `true`. +As the [specification](https://tools.ietf.org/html/rfc7231#section-6.4) prefers to rewrite the HTTP method only on `303` responses, this is Got's default behavior. Setting `methodRewriting` to `true` will also rewrite `301` and `302` responses, as allowed by the spec. This is the behavior followed by `curl` and browsers. + +**Note:** +> - Got never performs method rewriting on `307` and `308` responses, as this is [explicitly prohibited by the specification](https://www.rfc-editor.org/rfc/rfc7231#section-6.4.7). ### `enableUnixSockets` diff --git a/source/core/index.ts b/source/core/index.ts index 5d0e2f6b4..9fa49df49 100644 --- a/source/core/index.ts +++ b/source/core/index.ts @@ -731,10 +731,11 @@ export default class Request extends Duplex implements RequestEvents { const updatedOptions = new Options(undefined, undefined, this.options); - const shouldBeGet = statusCode === 303 && updatedOptions.method !== 'GET' && updatedOptions.method !== 'HEAD'; - if (shouldBeGet || updatedOptions.methodRewriting) { - // 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. + const serverRequestedGet = statusCode === 303 && updatedOptions.method !== 'GET' && updatedOptions.method !== 'HEAD'; + const canRewrite = statusCode !== 307 && statusCode !== 308; + const userRequestedGet = updatedOptions.methodRewriting && canRewrite; + + if (serverRequestedGet || userRequestedGet) { updatedOptions.method = 'GET'; updatedOptions.body = undefined; diff --git a/source/core/options.ts b/source/core/options.ts index cb5f7bbfa..cc722bff7 100644 --- a/source/core/options.ts +++ b/source/core/options.ts @@ -1764,7 +1764,7 @@ export default class Options { 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 is in accordance with [the spec](https://tools.ietf.org/html/rfc7231#section-6.4.4). You can optionally turn on this behavior also for other redirect codes - see `methodRewriting`. @default true */ @@ -1955,9 +1955,12 @@ export default class Options { } /** - Specifies if the redirects should be [rewritten as `GET`](https://tools.ietf.org/html/rfc7231#section-6.4). + Specifies if the HTTP request method should be [rewritten as `GET`](https://tools.ietf.org/html/rfc7231#section-6.4) on redirects. - If `false`, when sending a POST request and receiving a `302`, it will resend the body to the new location using the same HTTP method (`POST` in this case). + As the [specification](https://tools.ietf.org/html/rfc7231#section-6.4) prefers to rewrite the HTTP method only on `303` responses, this is Got's default behavior. + Setting `methodRewriting` to `true` will also rewrite `301` and `302` responses, as allowed by the spec. This is the behavior followed by `curl` and browsers. + + __Note__: Got never performs method rewriting on `307` and `308` responses, as this is [explicitly prohibited by the specification](https://www.rfc-editor.org/rfc/rfc7231#section-6.4.7). @default false */ diff --git a/test/redirects.ts b/test/redirects.ts index 982803a80..44104423d 100644 --- a/test/redirects.ts +++ b/test/redirects.ts @@ -459,11 +459,20 @@ test('method rewriting', withServer, async (t, server, got) => { }); response.end(); }); - server.get('/', (_request, response) => { response.end(); }); + server.post('/temporaryRedirect', (_request, response) => { + response.writeHead(307, { + location: '/', + }); + response.end(); + }); + server.post('/', (request, response) => { + request.pipe(response); + }); + const {body} = await got.post('redirect', { body: 'foobar', methodRewriting: true, @@ -477,6 +486,21 @@ test('method rewriting', withServer, async (t, server, got) => { }); t.is(body, ''); + + // Do not rewrite method on 307 or 308 + const {body: temporaryRedirectBody} = await got.post('temporaryRedirect', { + body: 'foobar', + methodRewriting: true, + hooks: { + beforeRedirect: [ + options => { + t.is(options.body, 'foobar'); + }, + ], + }, + }); + + t.is(temporaryRedirectBody, 'foobar'); }); test('clears username and password when redirecting to a different hostname', withServer, async (t, server, got) => {