Skip to content

SourceId serialization is ambiguous due to lack of escaping #11085

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Closed
g2p opened this issue Sep 14, 2022 · 0 comments · Fixed by #12280
Closed

SourceId serialization is ambiguous due to lack of escaping #11085

g2p opened this issue Sep 14, 2022 · 0 comments · Fixed by #12280
Labels
A-lockfile Area: Cargo.lock issues C-bug Category: bug S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.

Comments

@g2p
Copy link
Contributor

g2p commented Sep 14, 2022 •
edited
Loading

Problem

I noticed this when cargo update (in compare_dependency_graphs) kept notifying me of spurious changes when referring to a branch name with a '+' character. Due to poor serialisation, deserializing a branch name from Cargo.lock gives a branch that doesn't match the one in Cargo.toml.

It's also possible to give a branch name that will spill into the 'precise' commit hash when deserializing.

Steps

No response

Possible Solution(s)

Will send a PR that fixes serialization to use the expected encoding, which is unambiguous.

Notes

No response

Version

cargo 1.65.0 (bd99c043c 2022-09-09)
release: 1.65.0
commit-hash: bd99c043ceeaa0318f1ce4633f796110ae182002
commit-date: 2022-09-09
host: x86_64-unknown-linux-gnu
libgit2: 1.5.0 (sys:0.15.0 vendored)
libcurl: 7.81.0 (sys:0.4.56+curl-7.83.1 system ssl:NSS/3.68.2)
os: Ubuntu 22.04 (jammy) [64-bit]
@g2p g2p added the C-bug Category: bug label Sep 14, 2022
g2p added a commit to g2p/cargo that referenced this issue Sep 14, 2022
@g2p
SourceId: some git branches aren't properly serialized
fb553d5 
This adds a failing test for rust-lang#11085.

For some branch names (or tags or refs), generate_lockfile will
serialize poorly.  Parsing the Cargo.lock will refer to a branch
that wasn't the intended one in Cargo.toml.

Because a branch name can contain the '#' character, the precise
reference could be overriden through a branch/tag/ref as well.

This is because serialisation is done with a custom Display
implementation that concatenates strings as-is and is unaware
that some characters may need escaping; on the other hand,
deserialization uses url::Url::query_pairs which assumes
<https://url.spec.whatwg.org/#application/x-www-form-urlencoded>.
g2p added a commit to g2p/cargo that referenced this issue Sep 14, 2022
@g2p
SourceId: Fix ambiguous serialization of Git references
4650ae1 
Use x-www-form-urlencoded which is the standard for query strings,
matching what deserialization expects.
https://url.spec.whatwg.org/#application/x-www-form-urlencoded

Fixes rust-lang#11085.
g2p added a commit to g2p/cargo that referenced this issue Sep 14, 2022
@g2p
SourceId: some git branches aren't properly serialized
da2f573 
This adds a failing test for rust-lang#11085.

For some branch names (or tags or refs), generate_lockfile will
serialize poorly.  Parsing the Cargo.lock will refer to a branch
that wasn't the intended one in Cargo.toml.

Because a branch name can contain the '#' character, the precise
reference could be corrupted through a branch/tag/ref as well.

This is because serialisation is done with a custom Display
implementation that concatenates strings as-is and is unaware
that some characters may need escaping; on the other hand,
deserialization uses url::Url::query_pairs which assumes
<https://url.spec.whatwg.org/#application/x-www-form-urlencoded>.
g2p added a commit to g2p/cargo that referenced this issue Sep 14, 2022
@g2p
SourceId: Fix ambiguous serialization of Git references
f8d7f5b 
Use x-www-form-urlencoded which is the standard for query strings,
matching what the deserializer expects.
https://url.spec.whatwg.org/#application/x-www-form-urlencoded

Fixes rust-lang#11085.
@g2p g2p mentioned this issue Sep 14, 2022
Closed
g2p added a commit to g2p/cargo that referenced this issue Sep 14, 2022
@g2p
SourceId: some git branches aren't properly serialized
ec3ed33 
This adds a failing test for rust-lang#11085.

For some branch names (or tags or refs), generate_lockfile will
serialize poorly.  Parsing the Cargo.lock will refer to a branch
that wasn't the intended one in Cargo.toml.

Because a branch name can contain the '#' character, the precise
reference could be corrupted through a branch/tag/ref as well.

This is because serialisation is done with a custom Display
implementation that concatenates strings as-is and is unaware
that some characters may need escaping; on the other hand,
deserialization uses url::Url::query_pairs which assumes
<https://url.spec.whatwg.org/#application/x-www-form-urlencoded>.
g2p added a commit to g2p/cargo that referenced this issue Sep 14, 2022
@g2p
SourceId: Fix ambiguous serialization of Git references
9486e76 
Use x-www-form-urlencoded which is the standard for query strings,
matching what the deserializer expects.
https://url.spec.whatwg.org/#application/x-www-form-urlencoded

Fixes rust-lang#11085.
@weihanglo weihanglo added the A-lockfile Area: Cargo.lock issues label Sep 15, 2022
g2p added a commit to g2p/cargo that referenced this issue Sep 15, 2022
@g2p
SourceId: Fix ambiguous serialization of Git references
78c456d 
Use x-www-form-urlencoded which is the standard for query strings,
matching what the deserializer expects.
https://url.spec.whatwg.org/#application/x-www-form-urlencoded

Fixes rust-lang#11085.
g2p added a commit to g2p/cargo that referenced this issue Sep 15, 2022
@g2p
SourceId: some git branches aren't properly serialized
2c7ae3d 
This adds a failing test for rust-lang#11085.

For some branch names (or tags or refs), generate_lockfile will
serialize poorly.  Parsing the Cargo.lock will refer to a branch
that wasn't the intended one in Cargo.toml.

Because a branch name can contain the '#' character, the precise
reference could be corrupted through a branch/tag/ref as well.

This is because serialisation is done with a custom Display
implementation that concatenates strings as-is and is unaware
that some characters may need escaping; on the other hand,
deserialization uses url::Url::query_pairs which assumes
<https://url.spec.whatwg.org/#application/x-www-form-urlencoded>.
g2p added a commit to g2p/cargo that referenced this issue Sep 15, 2022
@g2p
SourceId: Fix ambiguous serialization of Git references
3476441 
Use x-www-form-urlencoded which is the standard for query strings,
matching what the deserializer expects.
https://url.spec.whatwg.org/#application/x-www-form-urlencoded

Fixes rust-lang#11085.
@weihanglo weihanglo mentioned this issue Mar 2, 2023
Closed
@weihanglo weihanglo added the S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. label Apr 25, 2023
@weihanglo weihanglo mentioned this issue May 10, 2023
Open
@weihanglo weihanglo mentioned this issue Jun 17, 2023
Merged
@weihanglo weihanglo mentioned this issue Aug 21, 2023
Closed
@bruceg bruceg mentioned this issue Aug 21, 2023
Merged
@jszwedko jszwedko mentioned this issue Aug 22, 2023
Closed
@weihanglo weihanglo mentioned this issue Sep 23, 2024
Closed
@epage epage mentioned this issue Nov 4, 2024
Closed
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees
No one assigned
Labels
A-lockfile Area: Cargo.lock issues C-bug Category: bug S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.
Projects
None yet
Milestone
No milestone
2 participants
@g2p @weihanglo