Skip to content

NIOTypedHTTPClientUpgradeHandler will write Upgrade request on handlerAdded if active#3473

Merged
Lukasa merged 5 commits intoapple:mainfrom
adam-fowler:websocket-upgrade-on-add
Jan 14, 2026
Merged

NIOTypedHTTPClientUpgradeHandler will write Upgrade request on handlerAdded if active#3473
Lukasa merged 5 commits intoapple:mainfrom
adam-fowler:websocket-upgrade-on-add

Conversation

@adam-fowler
Copy link
Copy Markdown
Contributor

NIOTypedHTTPClientUpgradeHandler.handlerAdded will write upgrade request if the channel is already active

Motivation:

This has been added to make it easier to implement a websocket client that supports proxies. If the upgrade handler is added after the proxy connect has been processed previously nothing would happen.

Modifications:

  • Added new function NIOTypedHTTPClientUpgradeHandler.writeUpgradeRequest() function. Call this from channelActive and also from handlerAdded if the channel is already active.
  • Added test testUpgradeHappensAfterHandlerAdded

Result:

NIOTypedHTTPClientUpgradeHandler now also works if it is added to a channel after the channel is active

Copy link
Copy Markdown
Contributor

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

This is a good change, but needs a slight tweak to be a bit more defensive.

It is currently possible to crash this handler. To do it, you could write a test like the following:

  1. Add a handler to the pipeline that, when it receives channelActive, will add the upgrade handler to the pipeline, and then forwards on channelActive.
  2. Activate the channel.

In this flow, the handler will activate itself on handlerAdded, but then receive channelActive and crash.

The solution here is to enhance the state machine to tolerate activating twice, and return none as the action for future activations.

@adam-fowler adam-fowler requested a review from Lukasa January 10, 2026 06:58
@Lukasa Lukasa added the 🔨 semver/patch No public API change. label Jan 14, 2026
@Lukasa Lukasa enabled auto-merge (squash) January 14, 2026 11:17
@Lukasa Lukasa merged commit 61053a0 into apple:main Jan 14, 2026
55 checks passed
@adam-fowler adam-fowler deleted the websocket-upgrade-on-add branch January 14, 2026 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🔨 semver/patch No public API change.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants