feat: SourceIdAsUrl is aware of URL encoding

Function `SourceId::as_encoded_url()` is added for that purpose.

Co-authored-by: Gabriel de Perthuis <[email protected]>

Author: weihanglo

src/cargo/core/source/source_id.rs

@@ -195,6 +195,15 @@ impl SourceId {
     pub fn as_url(&self) -> SourceIdAsUrl<'_> {
         SourceIdAsUrl {
             inner: &*self.inner,
+            encoded: false,
+        }
+    }
+
+    /// Like [`Self::as_url`] but with URL parameters encoded.
+    pub fn as_encoded_url(&self) -> SourceIdAsUrl<'_> {
+        SourceIdAsUrl {
+            inner: &*self.inner,
+            encoded: true,
         }
     }
 
@@ -566,7 +575,7 @@ impl fmt::Display for SourceId {
                 // Don't replace the URL display for git references,
                 // because those are kind of expected to be URLs.
                 write!(f, "{}", self.inner.url)?;
-                if let Some(pretty) = reference.pretty_ref() {
+                if let Some(pretty) = reference.pretty_ref(false) {
                     write!(f, "?{}", pretty)?;
                 }
 
@@ -714,6 +723,7 @@ impl Ord for SourceKind {
 /// A `Display`able view into a `SourceId` that will write it as a url
 pub struct SourceIdAsUrl<'a> {
     inner: &'a SourceIdInner,
+    encoded: bool,
 }
 
 impl<'a> fmt::Display for SourceIdAsUrl<'a> {
@@ -731,7 +741,7 @@ impl<'a> fmt::Display for SourceIdAsUrl<'a> {
                 ..
             } => {
                 write!(f, "git+{}", url)?;
-                if let Some(pretty) = reference.pretty_ref() {
+                if let Some(pretty) = reference.pretty_ref(self.encoded) {
                     write!(f, "?{}", pretty)?;
                 }
                 if let Some(precise) = precise.as_ref() {
@@ -771,27 +781,49 @@ impl<'a> fmt::Display for SourceIdAsUrl<'a> {
 impl GitReference {
     /// Returns a `Display`able view of this git reference, or None if using
     /// the head of the default branch
-    pub fn pretty_ref(&self) -> Option<PrettyRef<'_>> {
+    pub fn pretty_ref(&self, url_encoded: bool) -> Option<PrettyRef<'_>> {
         match self {
             GitReference::DefaultBranch => None,
-            _ => Some(PrettyRef { inner: self }),
+            _ => Some(PrettyRef {
+                inner: self,
+                url_encoded,
+            }),
         }
     }
 }
 
 /// A git reference that can be `Display`ed
 pub struct PrettyRef<'a> {
     inner: &'a GitReference,
+    url_encoded: bool,
 }
 
 impl<'a> fmt::Display for PrettyRef<'a> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match *self.inner {
-            GitReference::Branch(ref b) => write!(f, "branch={}", b),
-            GitReference::Tag(ref s) => write!(f, "tag={}", s),
-            GitReference::Rev(ref s) => write!(f, "rev={}", s),
+        let value: &str;
+        match self.inner {
+            GitReference::Branch(s) => {
+                write!(f, "branch=")?;
+                value = s;
+            }
+            GitReference::Tag(s) => {
+                write!(f, "tag=")?;
+                value = s;
+            }
+            GitReference::Rev(s) => {
+                write!(f, "rev=")?;
+                value = s;
+            }
             GitReference::DefaultBranch => unreachable!(),
         }
+        if self.url_encoded {
+            for value in url::form_urlencoded::byte_serialize(value.as_bytes()) {
+                write!(f, "{value}")?;
+            }
+        } else {
+            write!(f, "{value}")?;
+        }
+        Ok(())
     }
 }
 
@@ -905,6 +937,27 @@ mod tests {
         assert_eq!(formatted, "sparse+https://my-crates.io/");
         assert_eq!(source_id, deserialized);
     }
+
+    #[test]
+    fn gitrefs_roundtrip() {
+        let base = "https://host/path".into_url().unwrap();
+        let branch = GitReference::Branch("*-._+20%30 Z/z#foo=bar&zap[]?to\\()'\"".to_string());
+        let s1 = SourceId::for_git(&base, branch).unwrap();
+        let ser1 = format!("{}", s1.as_encoded_url());
+        let s2 = SourceId::from_url(&ser1).expect("Failed to deserialize");
+        let ser2 = format!("{}", s2.as_encoded_url());
+        // Serializing twice should yield the same result
+        assert_eq!(ser1, ser2, "Serialized forms don't match");
+        // SourceId serializing the same should have the same semantics
+        // This used to not be the case (# was ambiguous)
+        assert_eq!(s1, s2, "SourceId doesn't round-trip");
+        // Freeze the format to match an x-www-form-urlencoded query string
+        // https://url.spec.whatwg.org/#application/x-www-form-urlencoded
+        assert_eq!(
+            ser1,
+            "git+https://host/path?branch=*-._%2B20%2530+Z%2Fz%23foo%3Dbar%26zap%5B%5D%3Fto%5C%28%29%27%22"
+        );
+    }
 }
 
 /// Check if `url` equals to the overridden crates.io URL.

src/cargo/sources/git/source.rs

@@ -164,7 +164,7 @@ impl<'cfg> Debug for GitSource<'cfg> {
     fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
         write!(f, "git repo at {}", self.remote.url())?;
 
-        match self.manifest_reference.pretty_ref() {
+        match self.manifest_reference.pretty_ref(false) {
             Some(s) => write!(f, " ({})", s),
             None => Ok(()),
         }

src/cargo/util/toml_mut/dependency.rs

@@ -881,7 +881,7 @@ impl GitSource {
 impl std::fmt::Display for GitSource {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         let git_ref = self.git_ref();
-        if let Some(pretty_ref) = git_ref.pretty_ref() {
+        if let Some(pretty_ref) = git_ref.pretty_ref(false) {
             write!(f, "{}?{}", self.git, pretty_ref)
         } else {
             write!(f, "{}", self.git)