prism icon indicating copy to clipboard operation
prism copied to clipboard

Prism 4.10 Returning "Internal Server Error" when Content-Type header is set but body is empty/null

Open acolombier opened this issue 3 years ago • 2 comments
trafficstars

This issue seems related to #1939

Context

I'm using Prism as a validator proxy under AppSync HTTP VTL (AWS services), which must pass a Content-Type (and a Content-Lenght computed from the body length) even when using GET.

Current Behavior

For the following request:

curl --request GET \
  --url http://localhost:4010/my/endpoint \
  --header 'Content-Length: 0' \
  --header 'Content-Type: text/plain'

You would get this error:

< HTTP/1.1 500 Internal Server Error
...
{
  "type": "TypeError",
  "title": "Request with GET/HEAD method cannot have body",
  "status": 500,
  "detail": ""
}

(note that there is similar result when using application/json)

This impacts both the last image (stoplight/prism:4 and also master)

Expected Behavior

The request should be forward as-is , without any validation error. Potentially, an sl-violations should flag unexpected headers if the GET endpoint spec doesn't specify acceptance for this `Content-Type (this is arguable since the content is explicitly said to be null there)

Possible Workaround/Solution

(The issue seems to come from fetch, which denies bodies to be passed for GET request and seem to suffer from a ~drama~disagreement from contributor to remove the limitation)

I was able to fix the problem by applyinmg the following patch

diff --git a/packages/http-server/src/server.ts b/packages/http-server/src/server.ts
index 5339e428..e0f4ba87 100644
--- a/packages/http-server/src/server.ts
+++ b/packages/http-server/src/server.ts
@@ -40,9 +40,11 @@ function addressInfoToString(addressInfo: AddressInfo | string | null) {
 function parseRequestBody(request: IncomingMessage) {
   // if no body provided then return null instead of empty string
   if (
-    request.headers['content-type'] === undefined &&
-    request.headers['transfer-encoding'] === undefined &&
-    (request.headers['content-length'] === '0' || request.headers['content-length'] === undefined)
+    (request.headers['content-type'] === undefined &&
+      request.headers['transfer-encoding'] === undefined &&
+      request.headers['content-length'] === undefined) ||
+    // 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'
   ) {
     return Promise.resolve(null);
   }

It is however not containing the sl-violations feature.

Let me know if that sounds like an acceptable fix, I can issue a PR + update the unit test for it!

Steps to Reproduce

  1. Run the latest version of Prism (e.g: docker run -it -p 4010:4010 -v "$PWD:/app" stoplight/prism:4 proxy examples/petstore.oas2.yaml https://petstore.swagger.io/ -h 0.0.0.0)
  2. Query a GET endpoint with an empty body (e.g: curl http://localhost:4010/pets --header 'Content-Length: 0' --header 'Content-Type: text/plain')

Environment

  • Version used: 4.10.1 and master (commit a9b9c383d20a021fae123600710f4948a5d47073)
  • Environment name and version (e.g. Chrome 39, node.js 5.4): node v16.16.0
  • Operating System and version (desktop or mobile): Linux
  • Link to your environment/workspace/project: N/A

acolombier avatar Aug 04 '22 17:08 acolombier

@acolombier thank you for submitting this! That change seems perfectly reasonable, so feel free to put up a PR for it.

chohmann avatar Aug 05 '22 16:08 chohmann

Hi @chohmann, thanks for coming back to me so quickly. I have issued the PR #2103.

acolombier avatar Aug 06 '22 15:08 acolombier