type-is
type-is copied to clipboard
jshttp
Support http2
Support http2 to enable express tests with http2. expressjs/express#3390.
body-parser cannot parse http2 body stream, because hasBody with http2ServerRequest always return false. A http2ServerRequest does not have any information about body had. I think hasBody with http2ServerRequest should return always true.
@dougwilson Thank you for feedback. you are right. I will change hasBody to return false if request is OPTIONS, TRACE, CONNECT, HEADor GET method.
In HTTP1 a GET request may or may not have a body. I assume this is still the same in http/2 as I have used a body in a GET request to a http/2 server. Also that logic doesn't seem robust for custom methods or new future methods. There should be somewhere in the http/2 rfc that states exactly how to know if a request includes a body or not. That should be the implemented logic here.
Skipping around through the rfc you linked,, from what I can gather is that a body = data frames from the client on a stream. So to know if there is a body in http/2 you would say "will this steam send data frames?" and it seems that answer would be yes for the following condition: you get a header or continuation frame with end headers flag but no end stream flag set.
So to know if there is a body in http/2 you would say "will this steam send data frames?" and it seems that answer would be yes for the following condition: you get a header or continuation frame with end headers flag but no end stream flag set.
Definitely yes. And I had checked some nodejs code how get end stream flag. But I cannot found... At least, stream still not ended when express get http2ServerRequest even if no body. If you know that, it is great!
It's possible that new API needs to be added to Node.js to get that information. User land code should be able to get at all the data at the http/2 level from my understanding. If it's purposely hidden though the API then at the very least have an API to know if the http/2 request actually has a body or not.
I dig in nodejs code and found parameter that indicate already get end stream flag.
When Nodejs get end stream flag with HeaderFrame, Nodejs push null in order to end a readableStream and set stream._readableStream.ended to true.
https://github.com/nodejs/node/blob/master/lib/internal/http2/core.js#L301
I fixed this PR.
OK, I will file a issue in nodejs repo and pending this PR.
Did you ever make this issue / can you provide a link I can look at?
Yes, I already make the issue. See also https://github.com/nodejs/node/issues/22497
Thanks! I left a comment there. Let's give 30 days from now on that issue if it's not resolved and then we'll go with the reaching into internals solution of this PR :+1: