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

Add option allowGetBody to allow GET requests with payload #1081

Merged
merged 13 commits into from
Feb 20, 2020
11 changes: 10 additions & 1 deletion readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ Type: `string | Buffer | stream.Readable` or [`form-data` instance](https://gith

**Note #2:** If you provide this option, `got.stream()` will be read-only.

**Note #3:** If you provide a payload with the `GET` or `HEAD` method, it will throw a `TypeError`.
**Note #3:** If you provide a payload with the `GET` or `HEAD` method, it will throw a `TypeError` unless the method is `GET` and the `allowGetBody` option is set to `true`.

The `content-length` header will be automatically set if `body` is a `string` / `Buffer` / `fs.createReadStream` instance / [`form-data` instance](https://github.com/form-data/form-data), and `content-length` and `transfer-encoding` are not manually set in `options.headers`.

Expand Down Expand Up @@ -410,6 +410,15 @@ Default: `true`

By default, redirects will use [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 the body to the new location using the same HTTP method (`POST` in this case).

###### allowGetBody

Type: `boolean`\
Default: `false`

**Note:** The [RFC 7321](https://tools.ietf.org/html/rfc7231#section-4.3.1) doesn't specify any particular behavior for the GET method having a payload, therefore **it's considered an [anti-pattern](https://en.wikipedia.org/wiki/Anti-pattern)**.

Set this to `true` to allow sending body for the `GET` method. However, the [HTTP/2 specification](https://tools.ietf.org/html/rfc7540#section-8.1.3) says that `An HTTP GET request includes request header fields and no payload body`, therefore when using the HTTP/2 protocol this option will have no effect. This option is only meant to interact with non-compliant servers when you have no other choice.

###### maxRedirects

Type: `number`\
Expand Down
2 changes: 1 addition & 1 deletion source/as-stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export default function asStream<T>(options: NormalizedOptions): ProxyStream<T>
proxy.destroy();
throw new Error('Got\'s stream is not writable when the `body`, `json` or `form` option is used');
};
} else if (options.method === 'POST' || options.method === 'PUT' || options.method === 'PATCH') {
} else if (options.method === 'POST' || options.method === 'PUT' || options.method === 'PATCH' || (options.allowGetBody && options.method === 'GET')) {
options.body = input;
} else {
proxy.write = () => {
Expand Down
1 change: 1 addition & 0 deletions source/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ const defaults: Defaults = {
maxRedirects: 10,
prefixUrl: '',
methodRewriting: true,
allowGetBody: false,
ignoreInvalidCookies: false,
context: {},
_pagination: {
Expand Down
10 changes: 8 additions & 2 deletions source/normalize-arguments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ export const preNormalizeArguments = (options: Options, defaults?: NormalizedOpt
options.dnsCache = options.dnsCache ?? false;
options.useElectronNet = Boolean(options.useElectronNet);
options.methodRewriting = Boolean(options.methodRewriting);
options.allowGetBody = Boolean(options.allowGetBody);
options.context = options.context ?? {};

return options as NormalizedOptions;
Expand Down Expand Up @@ -362,7 +363,8 @@ export const normalizeArguments = (url: URLOrOptions, options?: Options, default
return normalizedOptions;
};

const withoutBody: ReadonlySet<string> = new Set(['GET', 'HEAD']);
const withoutBody: ReadonlySet<string> = new Set(['HEAD']);
const withoutBodyUnlessSpecified = 'GET';

export type NormalizedRequestArguments = Merge<https.RequestOptions, {
body?: stream.Readable;
Expand All @@ -386,6 +388,10 @@ export const normalizeRequestArguments = async (options: NormalizedOptions): Pro
throw new TypeError(`The \`${options.method}\` method cannot be used with a body`);
}

if (!options.allowGetBody && (isBody || isForm || isJson) && withoutBodyUnlessSpecified === options.method) {
throw new TypeError(`The \`${options.method}\` method cannot be used with a body`);
}

if ([isBody, isForm, isJson].filter(isTrue => isTrue).length > 1) {
throw new TypeError('The `body`, `json` and `form` options are mutually exclusive');
}
Expand Down Expand Up @@ -441,7 +447,7 @@ export const normalizeRequestArguments = async (options: NormalizedOptions): Pro
// body.
if (is.undefined(headers['content-length']) && is.undefined(headers['transfer-encoding'])) {
if (
(options.method === 'POST' || options.method === 'PUT' || options.method === 'PATCH' || options.method === 'DELETE') &&
(options.method === 'POST' || options.method === 'PUT' || options.method === 'PATCH' || options.method === 'DELETE' || (options.allowGetBody && options.method === 'GET')) &&
!is.undefined(uploadBodySize)
) {
// @ts-ignore We assign if it is undefined, so this IS correct
Expand Down
2 changes: 2 additions & 0 deletions source/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ export interface GotOptions extends PaginationOptions<unknown> {
context?: {[key: string]: any};
maxRedirects?: number;
lookup?: CacheableLookup['lookup'];
allowGetBody?: boolean;
methodRewriting?: boolean;
}

Expand Down Expand Up @@ -232,6 +233,7 @@ export interface NormalizedOptions extends Options {
followRedirect: boolean;
useElectronNet: boolean;
methodRewriting: boolean;
allowGetBody: boolean;
context: {[key: string]: any};

// UNIX socket support
Expand Down
7 changes: 7 additions & 0 deletions test/arguments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,13 @@ test('throws when passing body with a non payload method', async t => {
});
});

test('`allowGetBody` option', withServer, async (t, server, got) => {
server.get('/test', echoUrl);

const url = new URL(`${server.url}/test`);
await t.notThrowsAsync(got(url, {body: 'asdf', allowGetBody: true}));
});

test('WHATWG URL support', withServer, async (t, server, got) => {
server.get('/test', echoUrl);

Expand Down
8 changes: 7 additions & 1 deletion test/post.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,18 @@ const echoHeaders: Handler = (request, response) => {
response.end(JSON.stringify(request.headers));
};

test('GET cannot have body', withServer, async (t, server, got) => {
test('GET cannot have body without the `allowGetBody` option', withServer, async (t, server, got) => {
server.post('/', defaultEndpoint);

await t.throwsAsync(got.get({body: 'hi'}), {message: 'The `GET` method cannot be used with a body'});
});

test('GET can have body with option allowGetBody', withServer, async (t, server, got) => {
server.get('/', defaultEndpoint);

await t.notThrowsAsync(got.get({body: 'hi', allowGetBody: true}));
});

test('invalid body', async t => {
await t.throwsAsync(
// @ts-ignore Error tests
Expand Down