-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
inspector: report client-visible host and port #19664
Conversation
src/inspector_socket_server.cc
Outdated
} | ||
std::string FormatAddress(const std::string& host, | ||
const std::string& target_id, | ||
bool include_protocol) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you line these two arguments up with the first one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
I’m not sure what to feel about this. It feels almost like an XSS vector, but using HTTP headers. |
Can you clarify? In my opinion, it is the opposite - less information (that the remote party may not already know) is provided. Before this patch, the response would show an actual IP in case of remote connection. |
Fair enough. |
Did a second CI run: https://ci.nodejs.org/job/node-test-commit/17358/ No relevant failures detected (one failure from the first run was not detected in the second run and seems unlikely to have been caused by the change) |
Node instance may not know the real host and port user sees when debug frontend connects through the SSH tunnel. This change fixes '/json/list' response by using the value client provided in the host header. PR-URL: #19664 Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Node instance may not know the real host and port user sees when debug frontend connects through the SSH tunnel. This change fixes '/json/list' response by using the value client provided in the host header. PR-URL: #19664 Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Node instance may not know the real host and port user sees when
debug frontend connects through the SSH tunnel. This change fixes
'/json/list' response by using the value client provided in the host
header.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes