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

Update url to min 2.5.4 #1128

Merged
merged 1 commit into from
Feb 22, 2025
Merged

Update url to min 2.5.4 #1128

merged 1 commit into from
Feb 22, 2025

Conversation

ericswpark
Copy link
Contributor

This also resolves the security warning that comes from idna 0.5.0 being vulnerable.

@rustbot rustbot added the S-waiting-on-review Status: Waiting on review label Feb 21, 2025
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

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

While this looks legit, git2 doesn't pin the version of url crate. Packages depending on git2 should be able to get the latest url without problems.

What is the actual security warning from?

@weihanglo weihanglo self-requested a review February 22, 2025 00:22
@ericswpark
Copy link
Contributor Author

ericswpark commented Feb 22, 2025

@weihanglo you're right. I initially received an update from Dependabot telling me one of my projects was vulnerable due to idna, and tracked down the dependency graph until I landed on git2. Dependabot couldn't create a patch automatically, so I assumed 2.0 on cargo.toml locked versions to 2.0.x. After running cargo update I can verify url is updated to 2.5.4.

I can edit the patch to say 2.5 instead of 2.5.4 so it goes up to the version before 3.x, or this PR can be closed if it is unnecessary.

@weihanglo
Copy link
Member

I can edit the patch to say 2.5 instead of 2.5.4 so it goes up to the version before 3.x, or this PR can be closed if it is unnecessary.

Specifying url = "2.5.4" doesn't lock the dependency to 2.5.4 either. See https://doc.rust-lang.org/nightly/cargo/reference/specifying-dependencies.html#default-requirements.

@ericswpark
Copy link
Contributor Author

I can edit the patch to say 2.5 instead of 2.5.4 so it goes up to the version before 3.x, or this PR can be closed if it is unnecessary.

Specifying url = "2.5.4" doesn't lock the dependency to 2.5.4 either. See doc.rust-lang.org/nightly/cargo/reference/specifying-dependencies.html#default-requirements.

@weihanglo thanks for the link, I assumed putting on the patch version would lock it to future patch releases (2.5.x), though it seems I would have to use tilde requirements for that.

@weihanglo
Copy link
Member

To clarify, url = "2.0" has no problem for downstream users, as they will always get the latest 2.y.z version if they have no pre-existent Cargo.lock, or run cargo update.

Your original commit e31ba6a is a correct fix that downstream users will no longer get any version older than 2.5.4, moving the ecosystem away from the vulnerability. Could you also help update git2-curl and squash into one commit?

url = "2.0"

This also resolves the security warning that comes from idna 0.5.0 being
vulnerable.
@ericswpark
Copy link
Contributor Author

@weihanglo sounds good, I've dropped the last commit and updated git2-curl as well.

@ericswpark ericswpark changed the title Update url to 2.5.4 Update url to min 2.5.4 Feb 22, 2025
@weihanglo weihanglo added this pull request to the merge queue Feb 22, 2025
Merged via the queue into rust-lang:master with commit 64017cd Feb 22, 2025
7 checks passed
@ericswpark ericswpark deleted the idna-patch branch February 22, 2025 04:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Waiting on review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants