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

Add support for server-side websocket upgrade #500

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

quinnj
Copy link
Contributor

@quinnj quinnj commented Feb 21, 2025

The main idea here is a new aws_websocket_upgrade function that allows creating an aws_websocket instance from a server-side stream. In order for the stream to correctly be marked as having switched protocols in s_stream_complete, I added a is_switching_protocols bool field to the aws_h1_encoder_message struct that is checked in s_stream_complete.

Usage would then allow calling aws_websocket_upgrade from the server-side on_complete function of aws_http_request_handler_options and using the aws_websocket instance as desired as a server websocket.

I recognize that there may be a cleaner or more desirable way to track the is_switching_protocols, but after playing around with a few things, this approach seemed pretty simple. If we'd prefer to track this on the aws_h1_stream instead, or in some other way, I'm open to ideas.

I'm also open to other approaches on how to handle/allow the overall aws_websocket_upgrade. The other thought I had was perhaps you could pass something like the proposed aws_websocket_server_upgrade_options to the server configuration options itself and then aws_websocket_upgrade would be called automatically if an incoming websocket upgrade request was detected? i.e. teh websocket response would automatically be sent, the websocket upgraded, and then two aws_websocket_on_connect and aws_websocket_on_connect_shutdown callback functions would be required? That seemed a little more involved and I wasn't sure if it would even work dependency wise with server bringing in websocket definitions.

I also realize a discussion/issue could have been opened around this, but I've also just been personally curious to dive in and try something out in the project and this has been a fun way to get to know all the http (h1 at least) lifecycle code. Happy to restart if there are ideas on how to do something like this cleaner/simpler starting from scratch.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@quinnj
Copy link
Contributor Author

quinnj commented Feb 21, 2025

(motivation-wise, this is one of the last pieces of functionality we are hoping to support to have the Julia HTTP.jl package be able to replace it's internal implementation with aws-c-http functionality: JuliaWeb/HTTP.jl#1213)

@quinnj
Copy link
Contributor Author

quinnj commented Feb 21, 2025

(also, obviously additional documentation and tests can/would be added depending on how initial reviews of the proposed code go)

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.

1 participant