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

Websockets #8995

Closed
wants to merge 11 commits into from
Closed

Websockets #8995

wants to merge 11 commits into from

Conversation

bagder
Copy link
Member

@bagder bagder commented Jun 12, 2022

Warning: regular rebase + force-pushes are still performed on this branch.

Initial design thoughts

  • setopt documentation
  • configure
  • decide to not rely on any external websocket library
  • Send HTTP request
  • Send HTTP request when issued with hyper
  • websocket URL parser (+ tests for it)
  • curl_version_info() protocols[] fixed
  • HTTP Upgrade response check
  • extended CURLOPT_CONNECT_ONLY to offer upgrade-only
  • curl_ws_send documentation
  • curl_ws_recv documentation
  • curl_ws_send implementation
  • curl_ws_recv implementation
  • decide how to do the test server: extend sws
  • do proper non-blocking buffer management when reading slow/a lot
  • test server support for websockets (partial)
  • test cases running (ws test infrastructure working)
  • document curl_ws_meta();
  • implement curl_ws_meta();
  • add test using curl_ws_meta();
  • fail gracefully if not getting the expected101 response code + test it
  • allow overriding request headers in the ws initial request

Left for follow-up work

  • make websockets work with hyper
  • do the sha1 response verification
  • consider a curl_ws_poll()
  • make nc-like functionality for the curl tool
  • make sure ws code paths are fuzzed
  • disable PING-PONG automation
  • CURLWS_COMPRESS

@bagder bagder added feature-window A merge of this requires an open feature window libcurl API labels Jun 12, 2022
@bagder bagder force-pushed the WebSockets branch 3 times, most recently from 7a66eeb to 8364ca8 Compare June 15, 2022 09:52
@bagder bagder force-pushed the WebSockets branch 3 times, most recently from 9376d25 to 93e9419 Compare June 30, 2022 08:52
@bagder bagder force-pushed the WebSockets branch 2 times, most recently from 4f782d9 to 9e015a3 Compare July 4, 2022 08:46
@calvin2021y
Copy link

calvin2021y commented Jul 4, 2022

Hi @bagder, thanks for the great work.

I plan to use libcurl with event loop to replace libwebsockets. The new implement will support non-blocked frame recv ?

I guess call curl_ws_recv will blocked, so some thing like CURLOPT_WS_RECVFUNCTION, CURLOPT_WS_RECVDATA should exists to handle the callback inside a event loop. (each call should return a full frame with type: ping, pong, binary, text)

@bagder
Copy link
Member Author

bagder commented Jul 4, 2022

I think maybe it might be better to take this to the discussions rather than to have this "side conversation" here in the PR.

I plan to use libcurl with event loop to replace libwebsockets. The new implement will support non-blocked frame recv ?

I'm not sure what that means. The proposed API is documented in the wiki and in somewhat more detail in the docs section in the files in this PR.

I guess call curl_ws_recv will blocked

It will return a full frame is there is one, I think it might not return anything if there isn't a complete frame to return.

so some thing like CURLOPT_WS_RECVFUNCTION, CURLOPT_WS_RECVDATA should exists to handle the callback inside a event loop.

This has been discussed at length and I am currently providing the websockets data using the "regular" write callback CURLOPT_WRITEFUNCTION. Additional details about the delivered frame will be available from within the callback using the curl_ws_meta() function.

@bagder

This comment was marked as outdated.

@seanmonstar

This comment was marked as outdated.

@bagder

This comment was marked as outdated.

@seanmonstar

This comment was marked as outdated.

@bagder

This comment was marked as outdated.

@bagder bagder removed the feature-window A merge of this requires an open feature window label Sep 5, 2022
@bagder bagder marked this pull request as ready for review September 5, 2022 15:19
@bagder
Copy link
Member Author

bagder commented Sep 9, 2022

I intend to merge this soon.

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

Successfully merging this pull request may close these issues.

3 participants