Skip to content

Commit 8f4db31

Browse files
jasnelljasnell
committed
http2: refactor method arguments to avoid bools
PR-URL: #15693 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
1 parent 8fa0247 commit 8f4db31

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
@@ -127,7 +127,10 @@ const {
127127
HTTP_STATUS_OK,
128128
HTTP_STATUS_NO_CONTENT,
129129
HTTP_STATUS_NOT_MODIFIED,
130-
HTTP_STATUS_SWITCHING_PROTOCOLS
130+
HTTP_STATUS_SWITCHING_PROTOCOLS,
131+
132+
STREAM_OPTION_EMPTY_PAYLOAD,
133+
STREAM_OPTION_GET_TRAILERS
131134
} = constants;
132135

133136
// Top level to avoid creating a closure
@@ -412,20 +415,22 @@ function requestOnConnect(headers, options) {
412415
return;
413416
}
414417

415-
let getTrailers = false;
418+
let streamOptions = 0;
419+
if (options.endStream)
420+
streamOptions |= STREAM_OPTION_EMPTY_PAYLOAD;
421+
416422
if (typeof options.getTrailers === 'function') {
417-
getTrailers = true;
423+
streamOptions |= STREAM_OPTION_GET_TRAILERS;
418424
this[kState].getTrailers = options.getTrailers;
419425
}
420426

421427
// ret will be either the reserved stream ID (if positive)
422428
// or an error code (if negative)
423429
const ret = handle.submitRequest(headersList,
424-
!!options.endStream,
430+
streamOptions,
425431
options.parent | 0,
426432
options.weight | 0,
427-
!!options.exclusive,
428-
getTrailers);
433+
!!options.exclusive);
429434

430435
// In an error condition, one of three possible response codes will be
431436
// possible:
@@ -1568,7 +1573,7 @@ function processHeaders(headers) {
15681573
}
15691574

15701575
function processRespondWithFD(fd, headers, offset = 0, length = -1,
1571-
getTrailers = false) {
1576+
streamOptions = 0) {
15721577
const session = this[kSession];
15731578
const state = this[kState];
15741579
state.headersSent = true;
@@ -1578,7 +1583,7 @@ function processRespondWithFD(fd, headers, offset = 0, length = -1,
15781583

15791584
const handle = session[kHandle];
15801585
const ret =
1581-
handle.submitFile(this[kID], fd, headers, offset, length, getTrailers);
1586+
handle.submitFile(this[kID], fd, headers, offset, length, streamOptions);
15821587
let err;
15831588
switch (ret) {
15841589
case NGHTTP2_ERR_NOMEM:
@@ -1593,7 +1598,7 @@ function processRespondWithFD(fd, headers, offset = 0, length = -1,
15931598
}
15941599
}
15951600

1596-
function doSendFD(session, options, fd, headers, getTrailers, err, stat) {
1601+
function doSendFD(session, options, fd, headers, streamOptions, err, stat) {
15971602
if (this.destroyed || session.destroyed) {
15981603
abort(this);
15991604
return;
@@ -1623,10 +1628,10 @@ function doSendFD(session, options, fd, headers, getTrailers, err, stat) {
16231628
processRespondWithFD.call(this, fd, headersList,
16241629
statOptions.offset,
16251630
statOptions.length,
1626-
getTrailers);
1631+
streamOptions);
16271632
}
16281633

1629-
function doSendFileFD(session, options, fd, headers, getTrailers, err, stat) {
1634+
function doSendFileFD(session, options, fd, headers, streamOptions, err, stat) {
16301635
if (this.destroyed || session.destroyed) {
16311636
abort(this);
16321637
return;
@@ -1681,10 +1686,10 @@ function doSendFileFD(session, options, fd, headers, getTrailers, err, stat) {
16811686
processRespondWithFD.call(this, fd, headersList,
16821687
options.offset,
16831688
options.length,
1684-
getTrailers);
1689+
streamOptions);
16851690
}
16861691

1687-
function afterOpen(session, options, headers, getTrailers, err, fd) {
1692+
function afterOpen(session, options, headers, streamOptions, err, fd) {
16881693
const state = this[kState];
16891694
const onError = options.onError;
16901695
if (this.destroyed || session.destroyed) {
@@ -1702,7 +1707,8 @@ function afterOpen(session, options, headers, getTrailers, err, fd) {
17021707
state.fd = fd;
17031708

17041709
fs.fstat(fd,
1705-
doSendFileFD.bind(this, session, options, fd, headers, getTrailers));
1710+
doSendFileFD.bind(this, session, options, fd,
1711+
headers, streamOptions));
17061712
}
17071713

17081714
function streamOnError(err) {
@@ -1786,9 +1792,9 @@ class ServerHttp2Stream extends Http2Stream {
17861792
throw headersList;
17871793
}
17881794

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

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

@@ -1881,10 +1890,7 @@ class ServerHttp2Stream extends Http2Stream {
18811890

18821891
const handle = session[kHandle];
18831892
const ret =
1884-
handle.submitResponse(this[kID],
1885-
headersList,
1886-
options.endStream,
1887-
getTrailers);
1893+
handle.submitResponse(this[kID], headersList, streamOptions);
18881894
let err;
18891895
switch (ret) {
18901896
case NGHTTP2_ERR_NOMEM:
@@ -1937,14 +1943,14 @@ class ServerHttp2Stream extends Http2Stream {
19371943
options.statCheck);
19381944
}
19391945

1940-
let getTrailers = false;
1946+
let streamOptions = 0;
19411947
if (options.getTrailers !== undefined) {
19421948
if (typeof options.getTrailers !== 'function') {
19431949
throw new errors.TypeError('ERR_INVALID_OPT_VALUE',
19441950
'getTrailers',
19451951
options.getTrailers);
19461952
}
1947-
getTrailers = true;
1953+
streamOptions |= STREAM_OPTION_GET_TRAILERS;
19481954
state.getTrailers = options.getTrailers;
19491955
}
19501956

@@ -1963,7 +1969,8 @@ class ServerHttp2Stream extends Http2Stream {
19631969

19641970
if (options.statCheck !== undefined) {
19651971
fs.fstat(fd,
1966-
doSendFD.bind(this, session, options, fd, headers, getTrailers));
1972+
doSendFD.bind(this, session, options, fd,
1973+
headers, streamOptions));
19671974
return;
19681975
}
19691976

@@ -1977,7 +1984,7 @@ class ServerHttp2Stream extends Http2Stream {
19771984
processRespondWithFD.call(this, fd, headersList,
19781985
options.offset,
19791986
options.length,
1980-
getTrailers);
1987+
streamOptions);
19811988
}
19821989

19831990
// Initiate a file response on this Http2Stream. The path is passed to
@@ -2019,14 +2026,14 @@ class ServerHttp2Stream extends Http2Stream {
20192026
options.statCheck);
20202027
}
20212028

2022-
let getTrailers = false;
2029+
let streamOptions = 0;
20232030
if (options.getTrailers !== undefined) {
20242031
if (typeof options.getTrailers !== 'function') {
20252032
throw new errors.TypeError('ERR_INVALID_OPT_VALUE',
20262033
'getTrailers',
20272034
options.getTrailers);
20282035
}
2029-
getTrailers = true;
2036+
streamOptions |= STREAM_OPTION_GET_TRAILERS;
20302037
state.getTrailers = options.getTrailers;
20312038
}
20322039

@@ -2040,7 +2047,7 @@ class ServerHttp2Stream extends Http2Stream {
20402047
}
20412048

20422049
fs.open(path, 'r',
2043-
afterOpen.bind(this, session, options, headers, getTrailers));
2050+
afterOpen.bind(this, session, options, headers, streamOptions));
20442051
}
20452052

20462053
// 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)