Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: h2 without body #2258

Merged
merged 2 commits into from
Sep 10, 2023
Merged

fix: h2 without body #2258

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f0755a8
fix: h2 without body
metcoder95 Sep 8, 2023
dfa6fcf
refactor: close writable on non-expected body requests
metcoder95 Sep 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions lib/client.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1687,6 +1687,7 @@ function writeH2 (client, session, request) {
// we are already connected, streams are pending, first request
// will create a new stream. We trigger a request to create the stream and wait until
// `ready` event is triggered
// We disabled endStream to allow the user to write to the stream
stream = session.request(headers, { endStream: false, signal })

if (stream.id && !stream.pending) {
Expand Down Expand Up @@ -1761,17 +1762,21 @@ function writeH2 (client, session, request) {

session.ref()

const shouldEndStream = method === 'GET' || method === 'HEAD'
if (expectContinue) {
headers[HTTP2_HEADER_EXPECT] = '100-continue'
/**
* @type {import('node:http2').ClientHttp2Stream}
*/
stream = session.request(headers, { endStream: false, signal })
stream = session.request(headers, { endStream: shouldEndStream, signal })

stream.once('continue', writeBodyH2)
} else {
/** @type {import('node:http2').ClientHttp2Stream} */
stream = session.request(headers, { endStream: false, signal })
stream = session.request(headers, {
endStream: shouldEndStream,
signal
})
writeBodyH2()
}

Expand Down
4 changes: 3 additions & 1 deletion lib/core/request.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,11 +320,13 @@ class Request {

static [kHTTP2CopyHeaders] (raw) {
const rawHeaders = raw.split('\r\n')

const headers = {}

for (const header of rawHeaders) {
const [key, value] = header.split(': ')

if (value == null || value.length === 0) continue
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this needed?

Copy link
Member Author

@metcoder95 metcoder95 Sep 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a small fix, basically if the content of the headers was an empty string, the object was initialized with { '': undefined }.

Has nothing to do with the h2 issue though 😅

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why the header would ever be an empty string


if (headers[key]) headers[key] += `,${value}`
else headers[key] = value
}
Expand Down
Loading