FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

inspector: Do cleanups before notifying callback · nodejs/node@c4aaf47 · GitHub

/ node Public

Commit c4aaf47

Browse files
Eugene Ostroukhov
authored and
Fishrock123
committed
inspector: Do cleanups before notifying callback
Inspector socket implementation was notifying handshake callback before performing the cleanups, which meant that callback could not reclaim resources allocated by the client. New implementation will free all resource not allocated by the client before calling the callback, allowing the client to complete the cleanup. Fixes: #7418 PR-URL: #7450 Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
1 parent d857e9a commit c4aaf47

2 files changed

Lines changed: 39 additions & 23 deletions

File tree

‎src/inspector_socket.cc‎

Lines changed: 35 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ struct http_parsing_state_s {
2222
http_parser parser;
2323
http_parser_settings parser_settings;
2424
handshake_cb callback;
25+
bool done;
2526
bool parsing_value;
2627
char* ws_key;
2728
char* path;
@@ -482,24 +483,42 @@ static void handshake_complete(inspector_socket_t* inspector) {
482483
inspector->http_parsing_state->path);
483484
}
484485

485-
static void cleanup_http_parsing_state(struct http_parsing_state_s* state) {
486+
static void cleanup_http_parsing_state(inspector_socket_t* inspector) {
487+
struct http_parsing_state_s* state = inspector->http_parsing_state;
486488
free(state->current_header);
487489
free(state->path);
488490
free(state->ws_key);
489491
free(state);
492+
inspector->http_parsing_state = nullptr;
493+
}
494+
495+
static void report_handshake_failure_cb(uv_handle_t* handle) {
496+
dispose_inspector(handle);
497+
inspector_socket_t* inspector =
498+
static_cast<inspector_socket_t*>(handle->data);
499+
handshake_cb cb = inspector->http_parsing_state->callback;
500+
cleanup_http_parsing_state(inspector);
501+
cb(inspector, kInspectorHandshakeFailed, nullptr);
502+
}
503+
504+
static void close_and_report_handshake_failure(inspector_socket_t* inspector) {
505+
uv_handle_t* socket = reinterpret_cast<uv_handle_t*>(&inspector->client);
506+
if (uv_is_closing(socket)) {
507+
report_handshake_failure_cb(socket);
508+
} else {
509+
uv_read_stop(reinterpret_cast<uv_stream_t*>(socket));
510+
uv_close(socket, report_handshake_failure_cb);
511+
}
490512
}
491513

492514
static void handshake_failed(inspector_socket_t* inspector) {
493-
http_parsing_state_s* state = inspector->http_parsing_state;
494515
const char HANDSHAKE_FAILED_RESPONSE[] =
495516
"HTTP/1.0 400 Bad Request\r\n"
496517
"Content-Type: text/html; charset=UTF-8\r\n\r\n"
497518
"WebSockets request was expected\r\n";
498519
write_to_client(inspector, HANDSHAKE_FAILED_RESPONSE,
499520
sizeof(HANDSHAKE_FAILED_RESPONSE) - 1);
500-
close_connection(inspector);
501-
inspector->http_parsing_state = nullptr;
502-
state->callback(inspector, kInspectorHandshakeFailed, state->path);
521+
close_and_report_handshake_failure(inspector);
503522
}
504523

505524
// init_handshake references message_complete_cb
@@ -542,11 +561,10 @@ static int message_complete_cb(http_parser* parser) {
542561
int len = sizeof(accept_response);
543562
if (write_to_client(inspector, accept_response, len) >= 0) {
544563
handshake_complete(inspector);
564+
inspector->http_parsing_state->done = true;
545565
} else {
546-
state->callback(inspector, kInspectorHandshakeFailed, nullptr);
547-
close_connection(inspector);
566+
close_and_report_handshake_failure(inspector);
548567
}
549-
inspector->http_parsing_state = nullptr;
550568
} else {
551569
handshake_failed(inspector);
552570
}
@@ -565,27 +583,21 @@ static void data_received_cb(uv_stream_s* client, ssize_t nread,
565583
#endif
566584
inspector_socket_t* inspector =
567585
reinterpret_cast<inspector_socket_t*>((client->data));
568-
http_parsing_state_s* state = inspector->http_parsing_state;
569586
if (nread < 0 || nread == UV_EOF) {
570-
inspector->http_parsing_state->callback(inspector,
571-
kInspectorHandshakeFailed,
572-
nullptr);
573-
close_connection(inspector);
574-
inspector->http_parsing_state = nullptr;
587+
close_and_report_handshake_failure(inspector);
575588
} else {
589+
http_parsing_state_s* state = inspector->http_parsing_state;
576590
http_parser* parser = &state->parser;
577-
ssize_t parsed = http_parser_execute(parser, &state->parser_settings,
578-
inspector->buffer,
579-
nread);
580-
if (parsed < nread) {
591+
http_parser_execute(parser, &state->parser_settings, inspector->buffer,
592+
nread);
593+
if (parser->http_errno != HPE_OK) {
581594
handshake_failed(inspector);
582595
}
596+
if (inspector->http_parsing_state->done) {
597+
cleanup_http_parsing_state(inspector);
598+
}
583599
inspector->data_len = 0;
584600
}
585-
586-
if (inspector->http_parsing_state == nullptr) {
587-
cleanup_http_parsing_state(state);
588-
}
589601
}
590602

591603
static void init_handshake(inspector_socket_t* inspector) {
@@ -600,6 +612,7 @@ static void init_handshake(inspector_socket_t* inspector) {
600612
if (state->path) {
601613
state->path[0] = '\0';
602614
}
615+
state->done = false;
603616
http_parser_init(&state->parser, HTTP_REQUEST);
604617
state->parser.data = inspector;
605618
http_parser_settings* settings = &state->parser_settings;

‎test/cctest/test_inspector_socket.cc‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -628,6 +628,8 @@ static void ReportsHttpGet_handshake(enum inspector_handshake_event state,
628628
break;
629629
case 5:
630630
expected_state = kInspectorHandshakeFailed;
631+
expected_path = nullptr;
632+
break;
631633
case 4:
632634
expected_path = "/close";
633635
*cont = false;
@@ -677,15 +679,16 @@ HandshakeCanBeCanceled_handshake(enum inspector_handshake_event state,
677679
switch (handshake_events - 1) {
678680
case 0:
679681
EXPECT_EQ(kInspectorHandshakeUpgrading, state);
682+
EXPECT_STREQ("/ws/path", path);
680683
break;
681684
case 1:
682685
EXPECT_EQ(kInspectorHandshakeFailed, state);
686+
EXPECT_STREQ(nullptr, path);
683687
break;
684688
default:
685689
EXPECT_TRUE(false);
686690
break;
687691
}
688-
EXPECT_STREQ("/ws/path", path);
689692
*cont = false;
690693
}
691694

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL