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

Websocket support in Easy APIs / curl-sys #579

Open
frextrite opened this issue Sep 21, 2024 · 4 comments
Open

Websocket support in Easy APIs / curl-sys #579

frextrite opened this issue Sep 21, 2024 · 4 comments

Comments

@frextrite
Copy link

frextrite commented Sep 21, 2024

I am exploring usage of websockes with curl-rust and looking for pointers on the same. Specifically I'm trying out the connect only model mentioned in libcurl documentation.

But from the looks of it curl::easy::connect_only(enable: bool) does not allow settings an integer value (specifically 2 in case of Websockets).

Since the high-level Easy2 APIs didn't support this, I explored the lower-level curl-sys ffi APIs which do support this even though it looks like I'm reinventing the wheel and using a lot of unsafe rust. On exploring further I wasn't able to find the core websockets functions - curl_ws_recv and curl_ws_send in curl-sys even though it is based on libcurl 8.10.1 which definitely supports these APIs.

Am I missing any configuration to enable the usage of curl_ws_recv and curl_ws_send functions or is it something that is currently not supported by the crate?

Edit: README mentions the bindings are developed using curl 7.24.0 but the latest package references libcurl 8.10.1. What gives?

@sagebind
Copy link
Collaborator

Am I missing any configuration to enable the usage of curl_ws_recv and curl_ws_send functions or is it something that is currently not supported by the crate?

curl-rust generally hasn't had the goal to be an exhaustive binding to the complete libcurl API, but rather only including things that people need. Nobody has asked for websocket support before which is why it is not currently implemented.

That said, if it were me, I would probably not seek out curl as my first choice of library for doing websockets in Rust, unless I was already using it in my project.

Edit: README mentions the bindings are developed using curl 7.24.0 but the latest package references libcurl 8.10.1. What gives?

curl-rust dynamically links to libcurl by default, which means in order to be backwards compatible with older versions of libcurl that may be present on older systems, by default curl-rust does not hard link to any libcurl functions which were introduced more recently. If curl-sys contained symbols only present in newer libcurl versions, then curl-sys would produce a linking error on systems that don't have the latest version of libcurl installed system-wide, which is probably a large number of systems.

@frextrite
Copy link
Author

Thanks for your insights @sagebind. I have a few followups.

curl-rust generally hasn't had the goal to be an exhaustive binding to the complete libcurl API, but rather only including things that people need. Nobody has asked for websocket support before which is why it is not currently implemented.

I understand this completely for the curl-rust crate. But is there a way to enable websocket support at least with curl-sys crate? Doesn't curl-sys being on 8.10.1, imply all APIs in 8.10.1 should be supported?

That said, if it were me, I would probably not seek out curl as my first choice of library for doing websockets in Rust, unless I was already using it in my project.

Thanks for the heads up! My current use case for libcurl is mostly for monitoring purposes, making an HTTP request and getting the timing information. libcurl supports multiple protocols OOB, and making a websocket (or in the future any other protocol) request, is simply a change of few lines.

If curl-sys contained symbols only present in newer libcurl versions, then curl-sys would produce a linking error on systems that don't have the latest version of libcurl installed system-wide, which is probably a large number of systems.

Just a thought but wouldn't versioning and having separate versions for curl-rust and curl-sys help here? For example, current curl-rust 0.4.46 depends on curl-sys 0.4.72 with libcurl 8.6.0. If I want to use newer symbols, I should be able to bump up curl-sys to 0.4.76 separately and get bindings for libcurl 8.10.1. But I don't think that's possible at the moment.

However, if I'm working directly with curl-sys 0.4.76, I as a developer will make sure the libcurl version I'm linking against is at least 8.10.1 so that the bindings work properly but currently that's not a good option since bindings themselves are absent.

I do see reference to websockets.h in curl-sys, which does have the curl_ws_* functions but I'm not sure what's next to support them.

@sagebind
Copy link
Collaborator

Doesn't curl-sys being on 8.10.1, imply all APIs in 8.10.1 should be supported?

Just a thought but wouldn't versioning and having separate versions for curl-rust and curl-sys help here? For example, current curl-rust 0.4.46 depends on curl-sys 0.4.72 with libcurl 8.6.0. If I want to use newer symbols, I should be able to bump up curl-sys to 0.4.76 separately and get bindings for libcurl 8.10.1. But I don't think that's possible at the moment.

As of writing, the current version of curl-sys is 0.4.76. This versioning is specific to this project and doesn't relate to curl's version scheme. There is a curl version included in the full version string (0.4.76+curl-8.10.1), but it is after the + character which denotes build metadata by SemVer1. This is not used for any version comparisons, so you could not, for example, specify a version range to prevent a minor bump in curl version as specified in that build metadata.

The reason for the build metadata portion of curl-sys' semver string is for a different purpose. You see, curl-sys supports both dynamic linking and static linking. This is for two reasons:

  1. Some people really want static linking of curl, and others really want dynamic linking. So we offer a crate feature which leaves that choice up to the user, rather than us making that choice for everyone.
  2. If a system-wide curl is not available, we fall back toward static linking, as a preference toward at least having a successful build rather than failing the build.

When linking statically, we usually link to a bundled distribution of the libcurl source that is included within releases of curl-sys. This is explicitly opted into via the static-curl crate feature. Since therefore, curl-sys releases include a bundled release of curl within it, we include the version of that bundled release in the semver build metadata section as a helpful moniker/reminder of which version of curl is bundled with which version of curl-sys.

This does mean though that while enabling the static-curl feature guarantees the version of libcurl being used at runtime will match the version specified in curl-sys' version string, when that feature is not enabled (which it is disabled by default), it doesn't tell us much about what version of libcurl will be used at runtime. It might link to an older version, or a newer version, than the one that was bundled.

This means for the sake of this project's adherence to semver, if we were to bump the minimum required version of libcurl required by curl-sys for dynamic linking, then that would be at least a minor version bump to version 0.5, as it would be technically breaking to existing builds that use older versions of libcurl.

To see why this is the case, suppose that we did release a new version of curl-sys that required libcurl 8.10.1, and released that as curl-sys version 0.4.77. This means, if anyone is currently depending on curl = "0.4.*", which in turn depends on curl-sys = "^0.4.72"2, then when they cargo update, this would resolve the latest version in their dependency tree to the new curl-sys 0.4.77, without them even needing to change their version range for curl. Assuming they're targeting an older system with libcurl 7, this would break their builds, and they wouldn't have any way of fixing it other than pinning an exact patch version in their Cargo.lock, which would be pretty annoying, and not feel very "SemVer" of us.

Also, just to nip another idea in the bud, making curl-sys' version always match the libcurl version would also be kind of gross, as we'd never have any way of releasing a new version of curl-sys with bugfixes without also bumping the libcurl version at the same time. The semver guarantees of libcurl are different than the semver guarantees of curl-sys, and there's not really a way around that.


However, if I'm working directly with curl-sys 0.4.76, I as a developer will make sure the libcurl version I'm linking against is at least 8.10.1 so that the bindings work properly but currently that's not a good option since bindings themselves are absent.

You will, but someone who asked for curl-sys 0.4.56 two years ago didn't or won't, as at that time, the build metadata was curl-7.83.13. Because most of the time people write curl-sys = "0.4.56" and not curl-sys = "=0.4.56" (which arguably is a good thing), which is what would be required to not silently opt-in to major version bumps of libcurl being required on patch upgrades of curl-sys.


I do see reference to websockets.h in curl-sys, which does have the curl_ws_* functions but I'm not sure what's next to support them.

Yes, this is because we use a Git submodule to include the bundled version of libcurl we want to include, and we have to kinda tailor the build script to build libcurl in the most compatible way with curl-sys. It does add some maintenance burden though.

So hopefully after writing all these words, I haven't scared you away! Because actually I think this is a fine addition to curl-sys if someone is willing to put in the work. We actually already have a sort-of established pattern for adding new bindings that require newer libcurl versions, by using crate features. Two example features of this are upkeep_7_62_0 and poll_7_68_04. By putting the newer bindings behind a non-default feature name that declares the minimum libcurl version, it allows a developer to programmatically declare that they want those bindings and are aware of the minimum version requirement therein, while other developers who don't care about that feature aren't also forced to upgrade.

So what I would recommend if you're still interested in implementing this would be to create a new websockets_7_86_0 feature (which appears to be the libcurl version where these functions were added5) and then add those bindings to curl-sys somewhere behind #[cfg(feature = "websockets_7_86_0")]. Here's an example of some existing conditional bindings:

curl-rust/curl-sys/lib.rs

Lines 1127 to 1137 in a09b66e

#[cfg(feature = "poll_7_68_0")]
pub fn curl_multi_poll(
multi_handle: *mut CURLM,
extra_fds: *mut curl_waitfd,
extra_nfds: c_uint,
timeout_ms: c_int,
ret: *mut c_int,
) -> CURLMcode;
#[cfg(feature = "poll_7_68_0")]
pub fn curl_multi_wakeup(multi_handle: *mut CURLM) -> CURLMcode;

The bindings are written by hand (we're not using anything like bindgen) so the process usually involves inspecting the C headers provided by curl, and then translating it to the equivalent types and declarations in Rust.

Hopefully this helps!

Footnotes

  1. https://semver.org/#spec-item-10

  2. https://crates.io/crates/curl/0.4.46/dependencies

  3. https://crates.io/crates/curl-sys/0.4.56+curl-7.83.1

  4. https://github.com/alexcrichton/curl-rust/blob/a09b66e0f2e84843e1f6b58dc349f0c40dd18fd3/curl-sys/Cargo.toml#L55-L56

  5. https://curl.se/ch/7.86.0.html

@frextrite
Copy link
Author

frextrite commented Oct 16, 2024

So hopefully after writing all these words, I haven't scared you away!

Contrary to what it might seem, no, not at all! I've just been busy the past few weeks. Btw I really appreciate you taking the time to go into the depths and explaining each and every detail so nicely!

I unfortunately have other commitments for the time being and cannot dedicate time to add bindings for websockets. However, given how eloquently you've explained everything, if I (or someone else) get some time, your post above should have everything to get started.

Once again, thanks for this, it was very helpful!

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

No branches or pull requests

2 participants