Skip to content

Commit 078ee27

Browse files
jasnellMylesBorins
jasnell
authored and
MylesBorins
committed
http2: refactor method arguments to avoid bools
PR-URL: #15693 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>
1 parent 86ee05d commit 078ee27

File tree

4 files changed

+80
-71
lines changed

4 files changed

+80
-71
lines changed

‎lib/internal/http2/core.js

Lines changed: 37 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,10 @@ const {
126126
HTTP_STATUS_OK,
127127
HTTP_STATUS_NO_CONTENT,
128128
HTTP_STATUS_NOT_MODIFIED,
129-
HTTP_STATUS_SWITCHING_PROTOCOLS
129+
HTTP_STATUS_SWITCHING_PROTOCOLS,
130+
131+
STREAM_OPTION_EMPTY_PAYLOAD,
132+
STREAM_OPTION_GET_TRAILERS
130133
} = constants;
131134

132135
// Top level to avoid creating a closure
@@ -411,20 +414,22 @@ function requestOnConnect(headers, options) {
411414
return;
412415
}
413416

414-
let getTrailers = false;
417+
let streamOptions = 0;
418+
if (options.endStream)
419+
streamOptions |= STREAM_OPTION_EMPTY_PAYLOAD;
420+
415421
if (typeof options.getTrailers === 'function') {
416-
getTrailers = true;
422+
streamOptions |= STREAM_OPTION_GET_TRAILERS;
417423
this[kState].getTrailers = options.getTrailers;
418424
}
419425

420426
// ret will be either the reserved stream ID (if positive)
421427
// or an error code (if negative)
422428
const ret = handle.submitRequest(headersList,
423-
!!options.endStream,
429+
streamOptions,
424430
options.parent | 0,
425431
options.weight | 0,
426-
!!options.exclusive,
427-
getTrailers);
432+
!!options.exclusive);
428433

429434
// In an error condition, one of three possible response codes will be
430435
// possible:
@@ -1567,7 +1572,7 @@ function processHeaders(headers) {
15671572
}
15681573

15691574
function processRespondWithFD(fd, headers, offset = 0, length = -1,
1570-
getTrailers = false) {
1575+
streamOptions = 0) {
15711576
const session = this[kSession];
15721577
const state = this[kState];
15731578
state.headersSent = true;
@@ -1577,7 +1582,7 @@ function processRespondWithFD(fd, headers, offset = 0, length = -1,
15771582

15781583
const handle = session[kHandle];
15791584
const ret =
1580-
handle.submitFile(this[kID], fd, headers, offset, length, getTrailers);
1585+
handle.submitFile(this[kID], fd, headers, offset, length, streamOptions);
15811586
let err;
15821587
switch (ret) {
15831588
case NGHTTP2_ERR_NOMEM:
@@ -1592,7 +1597,7 @@ function processRespondWithFD(fd, headers, offset = 0, length = -1,
15921597
}
15931598
}
15941599

1595-
function doSendFD(session, options, fd, headers, getTrailers, err, stat) {
1600+
function doSendFD(session, options, fd, headers, streamOptions, err, stat) {
15961601
if (this.destroyed || session.destroyed) {
15971602
abort(this);
15981603
return;
@@ -1622,10 +1627,10 @@ function doSendFD(session, options, fd, headers, getTrailers, err, stat) {
16221627
processRespondWithFD.call(this, fd, headersList,
16231628
statOptions.offset,
16241629
statOptions.length,
1625-
getTrailers);
1630+
streamOptions);
16261631
}
16271632

1628-
function doSendFileFD(session, options, fd, headers, getTrailers, err, stat) {
1633+
function doSendFileFD(session, options, fd, headers, streamOptions, err, stat) {
16291634
if (this.destroyed || session.destroyed) {
16301635
abort(this);
16311636
return;
@@ -1680,10 +1685,10 @@ function doSendFileFD(session, options, fd, headers, getTrailers, err, stat) {
16801685
processRespondWithFD.call(this, fd, headersList,
16811686
options.offset,
16821687
options.length,
1683-
getTrailers);
1688+
streamOptions);
16841689
}
16851690

1686-
function afterOpen(session, options, headers, getTrailers, err, fd) {
1691+
function afterOpen(session, options, headers, streamOptions, err, fd) {
16871692
const state = this[kState];
16881693
const onError = options.onError;
16891694
if (this.destroyed || session.destroyed) {
@@ -1701,7 +1706,8 @@ function afterOpen(session, options, headers, getTrailers, err, fd) {
17011706
state.fd = fd;
17021707

17031708
fs.fstat(fd,
1704-
doSendFileFD.bind(this, session, options, fd, headers, getTrailers));
1709+
doSendFileFD.bind(this, session, options, fd,
1710+
headers, streamOptions));
17051711
}
17061712

17071713
function streamOnError(err) {
@@ -1785,9 +1791,9 @@ class ServerHttp2Stream extends Http2Stream {
17851791
throw headersList;
17861792
}
17871793

1788-
const ret = handle.submitPushPromise(this[kID],
1789-
headersList,
1790-
options.endStream);
1794+
const streamOptions = options.endStream ? STREAM_OPTION_EMPTY_PAYLOAD : 0;
1795+
1796+
const ret = handle.submitPushPromise(this[kID], headersList, streamOptions);
17911797
let err;
17921798
switch (ret) {
17931799
case NGHTTP2_ERR_NOMEM:
@@ -1843,14 +1849,17 @@ class ServerHttp2Stream extends Http2Stream {
18431849
options = Object.assign(Object.create(null), options);
18441850
options.endStream = !!options.endStream;
18451851

1846-
let getTrailers = false;
1852+
let streamOptions = 0;
1853+
if (options.endStream)
1854+
streamOptions |= STREAM_OPTION_EMPTY_PAYLOAD;
1855+
18471856
if (options.getTrailers !== undefined) {
18481857
if (typeof options.getTrailers !== 'function') {
18491858
throw new errors.TypeError('ERR_INVALID_OPT_VALUE',
18501859
'getTrailers',
18511860
options.getTrailers);
18521861
}
1853-
getTrailers = true;
1862+
streamOptions |= STREAM_OPTION_GET_TRAILERS;
18541863
state.getTrailers = options.getTrailers;
18551864
}
18561865

@@ -1880,10 +1889,7 @@ class ServerHttp2Stream extends Http2Stream {
18801889

18811890
const handle = session[kHandle];
18821891
const ret =
1883-
handle.submitResponse(this[kID],
1884-
headersList,
1885-
options.endStream,
1886-
getTrailers);
1892+
handle.submitResponse(this[kID], headersList, streamOptions);
18871893
let err;
18881894
switch (ret) {
18891895
case NGHTTP2_ERR_NOMEM:
@@ -1936,14 +1942,14 @@ class ServerHttp2Stream extends Http2Stream {
19361942
options.statCheck);
19371943
}
19381944

1939-
let getTrailers = false;
1945+
let streamOptions = 0;
19401946
if (options.getTrailers !== undefined) {
19411947
if (typeof options.getTrailers !== 'function') {
19421948
throw new errors.TypeError('ERR_INVALID_OPT_VALUE',
19431949
'getTrailers',
19441950
options.getTrailers);
19451951
}
1946-
getTrailers = true;
1952+
streamOptions |= STREAM_OPTION_GET_TRAILERS;
19471953
state.getTrailers = options.getTrailers;
19481954
}
19491955

@@ -1962,7 +1968,8 @@ class ServerHttp2Stream extends Http2Stream {
19621968

19631969
if (options.statCheck !== undefined) {
19641970
fs.fstat(fd,
1965-
doSendFD.bind(this, session, options, fd, headers, getTrailers));
1971+
doSendFD.bind(this, session, options, fd,
1972+
headers, streamOptions));
19661973
return;
19671974
}
19681975

@@ -1976,7 +1983,7 @@ class ServerHttp2Stream extends Http2Stream {
19761983
processRespondWithFD.call(this, fd, headersList,
19771984
options.offset,
19781985
options.length,
1979-
getTrailers);
1986+
streamOptions);
19801987
}
19811988

19821989
// Initiate a file response on this Http2Stream. The path is passed to
@@ -2018,14 +2025,14 @@ class ServerHttp2Stream extends Http2Stream {
20182025
options.statCheck);
20192026
}
20202027

2021-
let getTrailers = false;
2028+
let streamOptions = 0;
20222029
if (options.getTrailers !== undefined) {
20232030
if (typeof options.getTrailers !== 'function') {
20242031
throw new errors.TypeError('ERR_INVALID_OPT_VALUE',
20252032
'getTrailers',
20262033
options.getTrailers);
20272034
}
2028-
getTrailers = true;
2035+
streamOptions |= STREAM_OPTION_GET_TRAILERS;
20292036
state.getTrailers = options.getTrailers;
20302037
}
20312038

@@ -2039,7 +2046,7 @@ class ServerHttp2Stream extends Http2Stream {
20392046
}
20402047

20412048
fs.open(path, 'r',
2042-
afterOpen.bind(this, session, options, headers, getTrailers));
2049+
afterOpen.bind(this, session, options, headers, streamOptions));
20432050
}
20442051

20452052
// Sends a block of informational headers. In theory, the HTTP/2 spec

‎src/node_http2.cc

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -479,7 +479,7 @@ void Http2Session::SubmitRstStream(const FunctionCallbackInfo<Value>& args) {
479479

480480
void Http2Session::SubmitRequest(const FunctionCallbackInfo<Value>& args) {
481481
// args[0] Array of headers
482-
// args[1] endStream boolean
482+
// args[1] options int
483483
// args[2] parentStream ID (for priority spec)
484484
// args[3] weight (for priority spec)
485485
// args[4] exclusive boolean (for priority spec)
@@ -492,15 +492,14 @@ void Http2Session::SubmitRequest(const FunctionCallbackInfo<Value>& args) {
492492
Isolate* isolate = env->isolate();
493493

494494
Local<Array> headers = args[0].As<Array>();
495-
bool endStream = args[1]->BooleanValue(context).ToChecked();
495+
int options = args[1]->IntegerValue(context).ToChecked();
496496
int32_t parent = args[2]->Int32Value(context).ToChecked();
497497
int32_t weight = args[3]->Int32Value(context).ToChecked();
498498
bool exclusive = args[4]->BooleanValue(context).ToChecked();
499-
bool getTrailers = args[5]->BooleanValue(context).ToChecked();
500499

501-
DEBUG_HTTP2("Http2Session: submitting request: headers: %d, end-stream: %d, "
500+
DEBUG_HTTP2("Http2Session: submitting request: headers: %d, options: %d, "
502501
"parent: %d, weight: %d, exclusive: %d\n", headers->Length(),
503-
endStream, parent, weight, exclusive);
502+
options, parent, weight, exclusive);
504503

505504
nghttp2_priority_spec prispec;
506505
nghttp2_priority_spec_init(&prispec, parent, weight, exclusive ? 1 : 0);
@@ -509,8 +508,7 @@ void Http2Session::SubmitRequest(const FunctionCallbackInfo<Value>& args) {
509508

510509
int32_t ret = session->Nghttp2Session::SubmitRequest(&prispec,
511510
*list, list.length(),
512-
nullptr, endStream,
513-
getTrailers);
511+
nullptr, options);
514512
DEBUG_HTTP2("Http2Session: request submitted, response: %d\n", ret);
515513
args.GetReturnValue().Set(ret);
516514
}
@@ -529,11 +527,10 @@ void Http2Session::SubmitResponse(const FunctionCallbackInfo<Value>& args) {
529527

530528
int32_t id = args[0]->Int32Value(context).ToChecked();
531529
Local<Array> headers = args[1].As<Array>();
532-
bool endStream = args[2]->BooleanValue(context).ToChecked();
533-
bool getTrailers = args[3]->BooleanValue(context).ToChecked();
530+
int options = args[2]->IntegerValue(context).ToChecked();
534531

535532
DEBUG_HTTP2("Http2Session: submitting response for stream %d: headers: %d, "
536-
"end-stream: %d\n", id, headers->Length(), endStream);
533+
"options: %d\n", id, headers->Length(), options);
537534

538535
if (!(stream = session->FindStream(id))) {
539536
return args.GetReturnValue().Set(NGHTTP2_ERR_INVALID_STREAM_ID);
@@ -542,7 +539,7 @@ void Http2Session::SubmitResponse(const FunctionCallbackInfo<Value>& args) {
542539
Headers list(isolate, context, headers);
543540

544541
args.GetReturnValue().Set(
545-
stream->SubmitResponse(*list, list.length(), endStream, getTrailers));
542+
stream->SubmitResponse(*list, list.length(), options));
546543
}
547544

548545
void Http2Session::SubmitFile(const FunctionCallbackInfo<Value>& args) {
@@ -566,7 +563,7 @@ void Http2Session::SubmitFile(const FunctionCallbackInfo<Value>& args) {
566563

567564
int64_t offset = args[3]->IntegerValue(context).ToChecked();
568565
int64_t length = args[4]->IntegerValue(context).ToChecked();
569-
bool getTrailers = args[5]->BooleanValue(context).ToChecked();
566+
int options = args[5]->IntegerValue(context).ToChecked();
570567

571568
CHECK_GE(offset, 0);
572569

@@ -580,7 +577,7 @@ void Http2Session::SubmitFile(const FunctionCallbackInfo<Value>& args) {
580577
Headers list(isolate, context, headers);
581578

582579
args.GetReturnValue().Set(stream->SubmitFile(fd, *list, list.length(),
583-
offset, length, getTrailers));
580+
offset, length, options));
584581
}
585582

586583
void Http2Session::SendHeaders(const FunctionCallbackInfo<Value>& args) {
@@ -719,10 +716,10 @@ void Http2Session::SubmitPushPromise(const FunctionCallbackInfo<Value>& args) {
719716
Nghttp2Stream* parent;
720717
int32_t id = args[0]->Int32Value(context).ToChecked();
721718
Local<Array> headers = args[1].As<Array>();
722-
bool endStream = args[2]->BooleanValue(context).ToChecked();
719+
int options = args[2]->IntegerValue(context).ToChecked();
723720

724721
DEBUG_HTTP2("Http2Session: submitting push promise for stream %d: "
725-
"end-stream: %d, headers: %d\n", id, endStream,
722+
"options: %d, headers: %d\n", id, options,
726723
headers->Length());
727724

728725
if (!(parent = session->FindStream(id))) {
@@ -732,7 +729,7 @@ void Http2Session::SubmitPushPromise(const FunctionCallbackInfo<Value>& args) {
732729
Headers list(isolate, context, headers);
733730

734731
int32_t ret = parent->SubmitPushPromise(*list, list.length(),
735-
nullptr, endStream);
732+
nullptr, options);
736733
DEBUG_HTTP2("Http2Session: push promise submitted, ret: %d\n", ret);
737734
args.GetReturnValue().Set(ret);
738735
}
@@ -1250,6 +1247,9 @@ void Initialize(Local<Object> target,
12501247
NODE_DEFINE_HIDDEN_CONSTANT(constants, NGHTTP2_ERR_STREAM_CLOSED);
12511248
NODE_DEFINE_CONSTANT(constants, NGHTTP2_ERR_FRAME_SIZE_ERROR);
12521249

1250+
NODE_DEFINE_HIDDEN_CONSTANT(constants, STREAM_OPTION_EMPTY_PAYLOAD);
1251+
NODE_DEFINE_HIDDEN_CONSTANT(constants, STREAM_OPTION_GET_TRAILERS);
1252+
12531253
NODE_DEFINE_CONSTANT(constants, NGHTTP2_FLAG_NONE);
12541254
NODE_DEFINE_CONSTANT(constants, NGHTTP2_FLAG_END_STREAM);
12551255
NODE_DEFINE_CONSTANT(constants, NGHTTP2_FLAG_END_HEADERS);

0 commit comments

Comments
 (0)