Skip to content

Update url to min 2.5.4 #1128

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.

Sign up for GitHub

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

Jump to bottom
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
weihanglo
weihanglo approved these changes Feb 22, 2025
View reviewed changes
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 •
edited
Loading

@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?

git2-rs/git2-curl/Cargo.toml

Line 17 in 7b7be43

url = "2.0"

@ericswpark
Update url to 2.5.4
c6dd1ac 
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
facebook-github-bot pushed a commit to facebook/sapling that referenced this pull request May 15, 2025
@facebook-github-bot
update libgit2 0.20.0 to 0.20.2
7fcfb87 
Summary:
# Changelog

## 0.20.2 - 2025-05-05
[0.20.1...0.20.2](rust-lang/git2-rs@git2-0.20.1...git2-0.20.2)

### Added

- Added `Status::WT_UNREADABLE`.
  [#1151](rust-lang/git2-rs#1151)

### Fixed

- Added missing codes for `GIT_EDIRECTORY`, `GIT_EMERGECONFLICT`, `GIT_EUNCHANGED`, `GIT_ENOTSUPPORTED`, and `GIT_EREADONLY` to `Error::raw_code`.
  [#1153](rust-lang/git2-rs#1153)
- Fixed missing initialization in `Indexer::new`.
  [#1160](rust-lang/git2-rs#1160)

## 0.20.1 - 2025-03-17
[0.20.0...0.20.1](rust-lang/git2-rs@git2-0.20.0...git2-0.20.1)

### Added

- Added `Repository::branch_upstream_merge()`
  [#1131](rust-lang/git2-rs#1131)
- Added `Index::conflict_get()`
  [#1134](rust-lang/git2-rs#1134)
- Added `Index::conflict_remove()`
  [#1133](rust-lang/git2-rs#1133)
- Added `opts::set_cache_object_limit()`
  [#1118](rust-lang/git2-rs#1118)
- Added `Repo::merge_file_from_index()` and associated `MergeFileOptions` and `MergeFileResult`.
  [#1062](rust-lang/git2-rs#1062)

### Changed

- The `url` dependency minimum raised to 2.5.4
  [#1128](rust-lang/git2-rs#1128)
- Changed the tracing callback to abort the process if the callback panics instead of randomly detecting the panic in some other function.
  [#1121](rust-lang/git2-rs#1121)
- Credential helper config (loaded with `CredentialHelper::config`) now checks for helpers that start with something that looks like an absolute path, rather than checking for a `/` or `\` anywhere in the helper string (which resolves an issue if the helper had arguments with `/` or `\`).
  [#1137](rust-lang/git2-rs#1137)

### Fixed

- Fixed panic in `Remote::url_bytes` if the url is empty.
  [#1120](rust-lang/git2-rs#1120)
- Fixed incorrect lifetimes on `Patch::delta`, `Patch::hunk`, and `Patch::line_in_hunk`. The return values must not outlive the `Patch`.
  [#1141](rust-lang/git2-rs#1141)
- Bumped requirement to libgit2-sys 0.18.1, which fixes linking of advapi32 on Windows.
  [#1143](rust-lang/git2-rs#1143)

ignore-conflict-markers

Reviewed By: JakobDegen

Differential Revision: D74659779

fbshipit-source-id: a18bcd8f58bc62c7eedbfa5939a791002e18d7bc
facebook-github-bot pushed a commit to facebookincubator/reindeer that referenced this pull request May 15, 2025
@facebook-github-bot
update libgit2 0.20.0 to 0.20.2
cabe41f 
Summary:
# Changelog

## 0.20.2 - 2025-05-05
[0.20.1...0.20.2](rust-lang/git2-rs@git2-0.20.1...git2-0.20.2)

### Added

- Added `Status::WT_UNREADABLE`.
  [#1151](rust-lang/git2-rs#1151)

### Fixed

- Added missing codes for `GIT_EDIRECTORY`, `GIT_EMERGECONFLICT`, `GIT_EUNCHANGED`, `GIT_ENOTSUPPORTED`, and `GIT_EREADONLY` to `Error::raw_code`.
  [#1153](rust-lang/git2-rs#1153)
- Fixed missing initialization in `Indexer::new`.
  [#1160](rust-lang/git2-rs#1160)

## 0.20.1 - 2025-03-17
[0.20.0...0.20.1](rust-lang/git2-rs@git2-0.20.0...git2-0.20.1)

### Added

- Added `Repository::branch_upstream_merge()`
  [#1131](rust-lang/git2-rs#1131)
- Added `Index::conflict_get()`
  [#1134](rust-lang/git2-rs#1134)
- Added `Index::conflict_remove()`
  [#1133](rust-lang/git2-rs#1133)
- Added `opts::set_cache_object_limit()`
  [#1118](rust-lang/git2-rs#1118)
- Added `Repo::merge_file_from_index()` and associated `MergeFileOptions` and `MergeFileResult`.
  [#1062](rust-lang/git2-rs#1062)

### Changed

- The `url` dependency minimum raised to 2.5.4
  [#1128](rust-lang/git2-rs#1128)
- Changed the tracing callback to abort the process if the callback panics instead of randomly detecting the panic in some other function.
  [#1121](rust-lang/git2-rs#1121)
- Credential helper config (loaded with `CredentialHelper::config`) now checks for helpers that start with something that looks like an absolute path, rather than checking for a `/` or `\` anywhere in the helper string (which resolves an issue if the helper had arguments with `/` or `\`).
  [#1137](rust-lang/git2-rs#1137)

### Fixed

- Fixed panic in `Remote::url_bytes` if the url is empty.
  [#1120](rust-lang/git2-rs#1120)
- Fixed incorrect lifetimes on `Patch::delta`, `Patch::hunk`, and `Patch::line_in_hunk`. The return values must not outlive the `Patch`.
  [#1141](rust-lang/git2-rs#1141)
- Bumped requirement to libgit2-sys 0.18.1, which fixes linking of advapi32 on Windows.
  [#1143](rust-lang/git2-rs#1143)

ignore-conflict-markers

Reviewed By: JakobDegen

Differential Revision: D74659779

fbshipit-source-id: a18bcd8f58bc62c7eedbfa5939a791002e18d7bc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weihanglo weihanglo weihanglo approved these changes

Assignees
No one assigned
Labels
S-waiting-on-review Status: Waiting on review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ericswpark @weihanglo @rustbot