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

Upgrade reqwest to 0.12 #2015

Merged
merged 2 commits into from
Jun 3, 2024
Merged

Upgrade reqwest to 0.12 #2015

merged 2 commits into from
Jun 3, 2024

Conversation

legoktm
Copy link
Member

@legoktm legoktm commented May 17, 2024

Status

Ready for review

Description

There's a small change in how reqwest errors are printed (see seanmonstar/reqwest#2199), so handle that in our code and update one test accordingly.

Import a number of audits and trust markers, and the rest have been audited.

Fixes #1985.

Test Plan

  • CI passes
  • Visual review

Checklist

If these changes modify code paths involving cryptography, the opening of files in VMs or network (via the RPC service) traffic, Qubes testing in the staging environment is required. For fine tuning of the graphical user interface, testing in any environment in Qubes is required. Please check as applicable:

  • These changes should not need testing in Qubes

If these changes add or remove files other than client code, the AppArmor profile may need to be updated. Please check as applicable:

  • No update to the AppArmor profile is required for these changes

If these changes modify the database schema, you should include a database migration. Please check as applicable:

  • No database schema changes are needed

There's a small change in how reqwest errors are printed (see
<seanmonstar/reqwest#2199>), so handle that in
our code and update one test accordingly.

Import a number of audits and trust markers, a few audits will still be
needed.

Fixes #1985.
And prune imported audits that are no longer necessary.
@legoktm legoktm marked this pull request as ready for review May 28, 2024 18:26
@legoktm legoktm requested a review from a team as a code owner May 28, 2024 18:26
@rocodes rocodes self-assigned this Jun 3, 2024
@rocodes
Copy link
Contributor

rocodes commented Jun 3, 2024

@legoktm this looks ready to me, I assume the WIP Status is a holdover from before your last commit?

@legoktm
Copy link
Member Author

legoktm commented Jun 3, 2024

@rocodes oops yep, all set to go here

Copy link
Contributor

@rocodes rocodes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

  • Went through the update + cargo vet process, confirmed the same hashes and version numbers as in this PR (except hyper-util, on version 0.1.5 as of recently)
  • Confirmed the @legoktm-audited crates are the ones for whom we don't have a trusted contributor - thanks for auditing :)
  • One thing that mildly puzzled me, thinking of performing this process in the future, is that smallvec doesn't show up when I run cargo vet, even though it is added to the lock file (and I see that you've vetted it @legoktm), and I would have expected to see it flagged there.

@rocodes rocodes merged commit f5f0522 into main Jun 3, 2024
53 checks passed
@rocodes rocodes deleted the reqwest-0.12 branch June 3, 2024 20:49
@legoktm
Copy link
Member Author

legoktm commented Jun 3, 2024

Re: smallvec, if I remove my audit from the toml file and re-run cargo vet, then I see:

diff --git a/supply-chain/imports.lock b/supply-chain/imports.lock
index d6cd7f29..23f2f9c7 100644
--- a/supply-chain/imports.lock
+++ b/supply-chain/imports.lock
@@ -1394,6 +1394,12 @@ criteria = "safe-to-deploy"
 delta = "0.1.22 -> 0.1.23"
 aggregated-from = "https://raw.githubusercontent.com/zcash/zcash/master/qa/supply-chain/audits.toml"
 
+[[audits.zcash.audits.smallvec]]
+who = "Daira-Emma Hopwood <daira@jacaranda.org>"
+criteria = "safe-to-deploy"
+delta = "1.11.1 -> 1.13.2"
+aggregated-from = "https://raw.githubusercontent.com/zcash/librustzcash/main/supply-chain/audits.toml"
+
 [[audits.zcash.audits.tinyvec_macros]]
 who = "Jack Grigg <jack@z.cash>"
 criteria = "safe-to-deploy"

which I assume they published between my audit and now.

@rocodes
Copy link
Contributor

rocodes commented Jun 4, 2024

🤦 That makes sense, of course - even had a hint with the hyper-util version bump and didn't clue in. ty!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Update reqwest to 0.12
2 participants