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.

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

v8_inspector: any netty-based WS client cannot connect to inspector because Sec-WebSocket-Key check is not case-insensitive #7247

Closed
develar opened this issue Jun 9, 2016 · 1 comment · Fixed by #7248
Labels
inspector Issues and PRs related to the V8 inspector protocol

Comments

@develar
Copy link

develar commented Jun 9, 2016

If you try to connect to node inspector ws://127.0.0.1:9229/node, you get 400 Bad Request.

Because check

  } else if (!state->ws_key) {
    handshake_failed(inspector);
  }

in the inspector_socket.cc:523 failed.

Bug in the header_value_cb function — header Sec-WebSocket-Key is expected and it checked case-sensitive. But Netty sends this header as sec-websocket-key.

static int header_value_cb(http_parser* parser, const char* at, size_t length) {
      char SEC_WEBSOCKET_KEY_HEADER[] = "Sec-WebSocket-Key";
      struct http_parsing_state_s* state = (struct http_parsing_state_s*)
      (reinterpret_cast<inspector_socket_t*>(parser->data))->http_parsing_state;
  state->parsing_value = true;
  if (state->current_header && strncmp(state->current_header,
                                       SEC_WEBSOCKET_KEY_HEADER,
                                       sizeof(SEC_WEBSOCKET_KEY_HEADER)) == 0) {
    append(&state->ws_key, at, length);
  }
  return 0;
}
@develar develar changed the title v8_inspector: any netty-based WS client cannot connect to inspector because SEC_WEBSOCKET_KEY_HEADER is not case-insensitive v8_inspector: any netty-based WS client cannot connect to inspector because Sec-WebSocket-Key check is not case-insensitive Jun 9, 2016
@MylesBorins
Copy link
Contributor

@develar #7248 should fix this

@mscdex mscdex added the inspector Issues and PRs related to the V8 inspector protocol label Jun 9, 2016
MylesBorins added a commit to MylesBorins/node that referenced this issue Jun 14, 2016
Current case sensitive comparison is breaking netty-based WS clients.

replace strncmp with strncasecmp

Fixes: nodejs#7247
PR-URL: nodejs#7248
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Fishrock123 pushed a commit that referenced this issue Jul 5, 2016
Current case sensitive comparison is breaking netty-based WS clients.

replace strncmp with strncasecmp

Fixes: #7247
PR-URL: #7248
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inspector Issues and PRs related to the V8 inspector protocol
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants