-
-
Notifications
You must be signed in to change notification settings - Fork 33
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
Support http2 #36
base: master
Are you sure you want to change the base?
Support http2 #36
Changes from 7 commits
3a18e17
61e5115
e8e8cbc
c2806ac
e0f0cc6
70d00de
e6da059
765a024
09e012d
9bb739f
0108d17
395e271
9efda35
97c0d7a
e5b45d4
a18f5a6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -84,16 +84,36 @@ function typeis (value, types_) { | |
* or `content-length` headers set. | ||
* http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.3 | ||
* | ||
* A http/2 request with DataFrame can have no `content-length` header. | ||
* https://httpwg.org/specs/rfc7540.html | ||
* | ||
* A http/2 request without DataFrame send HeaderFrame with end-stream-flag. | ||
* If nodejs gets end-stream-flag, then nodejs ends readable stream. | ||
* https://github.com/nodejs/node/blob/master/lib/internal/http2/core.js#L301 | ||
* | ||
* @param {Object} request | ||
* @return {Boolean} | ||
* @public | ||
*/ | ||
|
||
function hasbody (req) { | ||
return req.headers['transfer-encoding'] !== undefined || | ||
return (ishttp2(req) && !req.stream._readableState.ended) || | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some testing seems like this may have an issue. If you add a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I checked and happens an issue you said. I address this issue and add a test. |
||
req.headers['transfer-encoding'] !== undefined || | ||
!isNaN(req.headers['content-length']) | ||
} | ||
|
||
/** | ||
* Check if a request is a http2 request. | ||
* | ||
* @param {Object} request | ||
* @return {Boolean} | ||
* @public | ||
*/ | ||
|
||
function ishttp2 (req) { | ||
return req.httpVersionMajor === 2 | ||
} | ||
|
||
/** | ||
* Check if the incoming request contains the "Content-Type" | ||
* header field, and it contains any of the give mime `type`s. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please only install deps from npm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, superagent with http2 have not released. I will wait for release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also what does the only flag do? According to https://docs.npmjs.com/cli/install it only does something if you do not provide a module name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is my mistake.
--only=dev
flag does nothing here. I should delete this flag.