Skip to content

Internal build for !7304 - #7331

Draft
ketanhwr wants to merge 1 commit into
mainfrom
ketan/trubini/connect_remote_addr_workerd
Draft

ketanhwr wants to merge 1 commit into
mainfrom
ketan/trubini/connect_remote_addr_workerd

Conversation

@ketanhwr

Copy link
Copy Markdown
Member

false, // isDynamicDispatch
kj::mv(accessInfo), kj::mv(metadata.restoredSelfTokenFactory), metadata.fromPersistentStub);
kj::mv(accessInfo), kj::mv(metadata.restoredSelfTokenFactory), metadata.fromPersistentStub,
kj::mv(metadata.clientIp));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes Socket.opened.remoteAddress appear on inbound sockets at every compatibility date; it was previously undefined. Please add a dedicated compatibility flag and cover both states so existing handlers retain their current result. Once that flag exists, gate this argument:

Suggested change
kj::mv(metadata.clientIp));
worker->getIsolate().getApi().getFeatureFlags().getConnectHandlerRemoteAddress()
? kj::mv(metadata.clientIp)
: kj::none);

@ask-bonk

ask-bonk Bot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

I'm Bonk, and I've done a quick review of your PR.

Plumbs the TCP peer address into inbound CONNECT handler sockets.

  1. HIGH src/workerd/server/server.c++:3969 exposes Socket.opened.remoteAddress for all compatibility dates, changing existing handler behavior. Posted one inline suggestion to gate it behind a new compatibility flag.

github run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants