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

Improve docs and disable method rewriting on 307 and 308 #2145

Merged
merged 6 commits into from
Sep 18, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions documentation/2-options.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
stjosh marked this conversation as resolved.
Show resolved Hide resolved

```js
import got from 'got';
Expand Down Expand Up @@ -936,9 +936,14 @@ 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.

However, since the spec also allows to rewrite to `GET` for `301` and `302` responses and many user agents - including, e.g., [`curl`](https://everything.curl.dev/http/redirects#get-or-post) and all major browsers - do so by default, this option may be used to mimic the same behavior and reach compatibility with servers expecting it.
stjosh marked this conversation as resolved.
Show resolved Hide resolved

**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`

Expand Down
9 changes: 5 additions & 4 deletions source/core/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -731,10 +731,11 @@ export default class Request extends Duplex implements RequestEvents<Request> {

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.
if ((updatedOptions.methodRewriting && ![307, 308].includes(statusCode))
|| (statusCode === 303 && !['GET', 'HEAD'].includes(updatedOptions.method))) {
stjosh marked this conversation as resolved.
Show resolved Hide resolved
// Either the server responded with 303 or
// the methodRewriting option explicitly requests
// a rewrite to GET on non-307/308 responses
stjosh marked this conversation as resolved.
Show resolved Hide resolved
updatedOptions.method = 'GET';

updatedOptions.body = undefined;
Expand Down
9 changes: 6 additions & 3 deletions source/core/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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.
However, since the spec also allows to rewrite to `GET` for `301` and `302` responses and many user agents - including, e.g., [`curl`](https://everything.curl.dev/http/redirects#get-or-post) and all major browsers - do so by default, this option may be used to mimic the same behavior and reach compatibility with servers expecting it.
stjosh marked this conversation as resolved.
Show resolved Hide resolved

__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
*/
Expand Down
26 changes: 25 additions & 1 deletion test/redirects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -459,10 +459,19 @@ test('method rewriting', withServer, async (t, server, got) => {
});
response.end();
});
server.get('/', (request, response) => {
request.pipe(response);
});
stjosh marked this conversation as resolved.
Show resolved Hide resolved

server.get('/', (_request, response) => {
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',
Expand All @@ -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) => {
Expand Down