fix(http): don't expose body on GET/HEAD requests #12260
Conversation
GET/HEAD requests can't have bodies according to `fetch` spec. This commit changes the HTTP server to hide request bodies for requests with GET or HEAD methods.
| @@ -95,7 +95,12 @@ | |||
| let body = null; | |||
| if (typeof requestRid === "number") { | |||
| SetPrototypeAdd(this.managedResources, requestRid); | |||
| body = createRequestBodyStream(this, requestRid); | |||
| // There might be a body, but we don't expose it for GET/HEAD requests. | |||
There was a problem hiding this comment.
Is it possible to refer to the relevant portion(s) of the spec here?
There was a problem hiding this comment.
This is the Request creation that is internal to our HTTP server, so there is no spec. The new Request constructor that is exposed to users enforces this in step 34 of the constructor steps: https://fetch.spec.whatwg.org/#dom-request
There was a problem hiding this comment.
This is the
Requestcreation that is internal to our HTTP server, so there is no spec.
Then why do we have to hide bodies on incoming requests?
And if we do, what about other methods that can't have bodies - OPTIONS and TRACE (DELETE?).
There was a problem hiding this comment.
Because users can not do new Request(incomingReq) (this throws) for an unmodified incoming GET request from the HTTP server. See #11927. We are creating an invalid Request instance because we are creating it internally, bypassing the usual request constructor checks. There is no possibility on the web platform to create a Request object where method === "GET" && body !== null. Because of this we should not have one either.
OPTIONS, TRACE, and DELETE requests can have bodies in fetch on the web platform. GET and HEAD can not.
GET/HEAD requests can't have bodies according to
fetchspec. Thiscommit changes the HTTP server to hide request bodies for requests with
GET or HEAD methods.
Fixes #11927