search

gftea / amqprs

Async & Lock-free RabbitMQ Rust Client, Easy-to-use API
MIT License
214 stars 27 forks source link

Upgrade rustls #141

Closed LockedThread closed 3 months ago

LockedThread commented 3 months ago

Description

I am working to upgrade to the latest version of rustls in order to not be affected by the security advisory linked below. https://github.com/rustls/rustls/security/advisories/GHSA-6g7w-8wpp-frhj

Resources

Breaking Changes:

LockedThread commented 3 months ago

@gftea All tests passed on my end. Let me know if you would like any changes.

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 85.68%. Comparing base (f0260c8) to head (50b550b).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #141 +/- ## ========================================== + Coverage 85.55% 85.68% +0.13% ========================================== Files 40 40 Lines 6540 6560 +20 ========================================== + Hits 5595 5621 +26 + Misses 945 939 -6 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

LockedThread commented 3 months ago

Regarding your comments, I tried what you said but it was constant fighting regarding lifetimes. With the rustls breaking changes it may be difficult to maintain backwards compatibility. I will try again tomorrow and get back to you.

I appreciate your fast review.

so-schen commented 3 months ago

Regarding your comments, I tried what you said but it was constant fighting regarding lifetimes. With the rustls breaking changes it may be difficult to maintain backwards compatibility. I will try again tomorrow and get back to you.

I appreciate your fast review.

If there are multiple input reference, you would need to annotate which lifetime to bind

LockedThread commented 3 months ago

Regarding your comments, I tried what you said but it was constant fighting regarding lifetimes. With the rustls breaking changes it may be difficult to maintain backwards compatibility. I will try again tomorrow and get back to you. I appreciate your fast review.

If there are multiple input reference, you would need to annotate which lifetime to bind

Thanks for the help. I try to stay away from lifetimes as much as possible. I think I got it working now. I have not run the tests yet, I will later tonight.

LockedThread commented 3 months ago

All tests are passing.

gftea commented 3 months ago

thanks @LockedThread for the changes. CI has clippy error and cargo check also fail in check_msrv, can you fix them?

LockedThread commented 3 months ago

thanks @LockedThread for the changes. CI has clippy error and cargo check also fail in check_msrv, can you fix them?

I fixed the clippy and check_msrv errors. Additionally, I upgraded rustls-pemfile to 2.0.

michaelklishin commented 3 months ago

@LockedThread thank you for taking the time to contribute!

LockedThread commented 3 months ago

@LockedThread thank you for taking the time to contribute!

I am happy too. This was my first real contribution to an open source rust project, it has been fun.

LockedThread commented 3 months ago

That is a weird error, it worked for me locally a few days ago... How would you like to proceed? Increment MSRV?

gftea commented 3 months ago

@LockedThread Nice work! it is almost there. there is a new error regarding rustc version in ci

gftea commented 3 months ago

@LockedThread Thanks for the PR, and I add some fix to CI and lift msrv to 1.64.