Skip to content

migrate hyper to reqwest #11

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

rursprung
Copy link

@rursprung rursprung commented Apr 3, 2025

please see the individual commit messages for further details.

i realise that this is similar to #8 from @jakobbraun, however this PR completely removes hyper and it does not add an async version of the API. i think removing the outdated hyper dependency (and the vulnerabilities coming from it) is the most pressing issue.

note that this is a breaking change for consumers, so you might want to use that chance to just ditch the blocking API (currently the only one available) and replace it with an async API (most of the rust ecosystem - at the very least anything doing I/O - seems to have gone down that route, and for good reasons). i've raised #12 for this.
nevertheless i think that merging & releasing this PR would be a good first step, you can then just remove the blocking:: from the reqwest module name and add the necessary async/await keywords. having a last non-async release without the outdated hyper dependency would allow clients which are not (yet) async to at least get rid of the security warnings coming from the outdated hyper dependency, giving them a two-step migration path (first get rid of insecure dependencies and then switch to async).

fixes #9

@rursprung
Copy link
Author

@adnanademovic: it'd be great if you could review, merge & release this and then bump rosrust to the new release as that'd resolve security alerts (reported e.g. by dependabot) on rosrust & its downstream consumers.

this is also a good reason to not combine that release with switching to async: rosrust as the main (only?) consumer of this crate isn't async either (and it doesn't make sense to invest the time to make it async anymore seeing that ROS 1 will be EOL very soon)

rursprung added 2 commits May 15, 2025 10:29
`hyper` v0.10 is extremely outdated, the current release is v1.6. this
old version pulls in vulnerable dependencies (incl. `hyper` v0.10
itself).

rather than upgrading to `hyper` v1.6 i opted to replace it with
`reqwest` since this crate here is not `async` and `hyper` v1 is
completely `async`. due to the very limited amount of features needed
from `hyper` (just executing HTTP POST requests) it can instead be
replaced with the much simpler `reqwest` crate. while it hasn't hit v1.0
yet it is very widely used (nearly 200 million downloads on crates.io).

this is a breaking change for consumers since the URLs can now just be
passed as strings rather than having to call `.parse()?` on the string.

note that this is similar to, but not the same as, adnanademovic#8.

fixes adnanademovic#9
code changes done using `cargo fix --edition`
@rursprung rursprung force-pushed the migrate-hyper-to-reqwest branch from 215825c to eac04e6 Compare May 15, 2025 08:30
rursprung added a commit to rursprung/rosrust that referenced this pull request May 15, 2025
this updates to the latest `xml-rpc` version. note that at the moment
this does not yet build since it hasn't been released yet.
this requires adnanademovic/xml-rpc-rs#11 to be merged & released first.

for the time being you can use this by using `[patch.crates-io]` in your
`Cargo.toml` and overwrite both `rosrust` and `xml-rpc`.

this update is needed to resolve various security vulnerabilities coming
from outdated versions of `hyper` which are being pulled in via
`xml-rpc`. with this, `cargo-audit` is happy again.
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.

update to hyper v1
1 participant