Skip to content

Commit

Permalink
feat: adds more standard compliant request body handling (#2260)
Browse files Browse the repository at this point in the history
  • Loading branch information
Daniel A. White authored Apr 12, 2023
1 parent 00233b7 commit 3b56cb7
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 11 deletions.
7 changes: 4 additions & 3 deletions packages/http-server/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,10 @@ function parseRequestBody(request: IncomingMessage) {
if (
// If the body size is null, it means the body itself is null so the promise can resolve with a null value
request.headers['content-length'] === '0' ||
(request.headers['content-type'] === undefined &&
request.headers['transfer-encoding'] === undefined &&
request.headers['content-length'] === undefined)
// Per HTTP 1.1 - these 2 headers are the valid way to indicate that a body exists:
// > The presence of a message body in a request is signaled by a Content-Length or Transfer-Encoding header field.
// https://httpwg.org/specs/rfc9112.html#message.body
(request.headers['transfer-encoding'] === undefined && request.headers['content-length'] === undefined)
) {
return Promise.resolve(null);
}
Expand Down
7 changes: 7 additions & 0 deletions packages/http/src/forwarder/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,10 @@ export const UPSTREAM_NOT_IMPLEMENTED: Omit<ProblemJson, 'detail'> = {
title: 'The server does not support the functionality required to fulfill the request',
status: 501,
};

export const PROXY_UNSUPPORTED_REQUEST_BODY: Omit<ProblemJson, 'detail'> = {
type: 'PROXY_UNSUPPORTED_REQUEST_BODY',
title:
'The Prism proxy does not support sending a GET/HEAD request with a message body to an upstream server. See: https://github.com/stoplightio/prism/issues/2259',
status: 501,
};
26 changes: 18 additions & 8 deletions packages/http/src/forwarder/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { parseResponse } from '../utils/parseResponse';
import { hopByHopHeaders } from './resources';
import { createUnauthorisedResponse, createUnprocessableEntityResponse } from '../mocker';
import { ProblemJsonError } from '../types';
import { UPSTREAM_NOT_IMPLEMENTED } from './errors';
import { PROXY_UNSUPPORTED_REQUEST_BODY, UPSTREAM_NOT_IMPLEMENTED } from './errors';
import * as createHttpProxyAgent from 'http-proxy-agent';
import * as createHttpsProxyAgent from 'https-proxy-agent';
import { toURLSearchParams } from '../utils/url';
Expand Down Expand Up @@ -44,7 +44,7 @@ const forward: IPrismComponents<IHttpOperation, IHttpRequest, IHttpResponse, IHt
: createUnprocessableEntityResponse(failedValidations);
}),
TE.swap,
TE.chainEitherK(() => serializeBody(input.body)),
TE.chainEitherK(() => serializeBodyForFetch(input, logger)),
TE.chain(body =>
TE.tryCatch(async () => {
const partialUrl = new URL(baseUrl);
Expand Down Expand Up @@ -98,13 +98,23 @@ const forward: IPrismComponents<IHttpOperation, IHttpRequest, IHttpResponse, IHt

export default forward;

function serializeBodyForFetch(input: IHttpRequest, logger: Logger): E.Either<Error, string | undefined> {
const upperMethod = input.method.toUpperCase();
if (['GET', 'HEAD'].includes(upperMethod) && ![null, undefined].includes(input.body as any)) {
logger.warn(`Upstream ${upperMethod} call to ${input.url.path} has request body`);
return E.left(ProblemJsonError.fromTemplate(PROXY_UNSUPPORTED_REQUEST_BODY));
}

return serializeBody(input.body);
}

export function serializeBody(body: unknown): E.Either<Error, string | undefined> {
if (typeof body === 'string') {
return E.right(body);
}

if (body) return pipe(J.stringify(body), E.mapLeft(E.toError));

return E.right(undefined);
}

Expand All @@ -113,21 +123,21 @@ function logForwardRequest({ logger, url, request }: { logger: Logger; url: stri
logger.info(`${prefix}Forwarding "${request.method}" request to ${url}...`);
logRequest({ logger, prefix, ...pick(request, 'body', 'headers') });
}

function forwardResponseLogger(logger: Logger) {
return (response: Response) => {
const prefix = chalk.grey('< ');

logger.info(`${prefix}Received forward response`);

const { status: statusCode } = response;

logResponse({
logger,
statusCode,
...pick(response, 'body', 'headers'),
prefix,
});
});

return response;
};
Expand Down
18 changes: 18 additions & 0 deletions test-harness/specs/proxy/proxy.get-null-body.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
====test====
Making a GET request with a Content-Type header through the proxy server ignores null body
====spec====
swagger: "2.0"
paths:
/status/204:
get:
produces:
- text/plain
responses:
204:
description: No Content
====server====
proxy -p 4010 ${document} http://httpbin.org
====command====
curl -i http://localhost:4010/status/204 -X GET --header 'Content-Type: application/json'
====expect====
HTTP/1.1 204 No Content
20 changes: 20 additions & 0 deletions test-harness/specs/proxy/proxy.get-with-body.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
====test====
Making a GET request with request body through the proxy server rejects with 501
====spec====
swagger: "2.0"
paths:
/status/204:
get:
produces:
- text/plain
responses:
204:
description: No Content
====server====
proxy -p 4010 ${document} http://httpbin.org
====command====
curl -i http://localhost:4010/status/204 -X GET --header 'Content-Type: application/json' --data '{}'
====expect====
HTTP/1.1 501 Not Implemented

{"type":"https://stoplight.io/prism/errors#PROXY_UNSUPPORTED_REQUEST_BODY","title":"The Prism proxy does not support sending a GET/HEAD request with a message body to an upstream server. See: https://github.com/stoplightio/prism/issues/2259","status":501,"detail":""}

0 comments on commit 3b56cb7

Please sign in to comment.