Work around '#' escaping bug in bip21 crate

bip21 dependency temporarily specified to use bugfix PR branch The `pj` parameter of the BIP 21 URL is itself a URL which contains a fragment. This is not escaped by bip21 during serialization, which according to RFC 3986 truncates the `pj` parameter, making everything that follows part of the fragment. Deserialization likewise ignores #, parsing it as part of the value which round trips with the incorrect serialization behavior. Temporarily depend on bugfix pr branch for bip21
payjoin · Dec 2, 2024 · 18dd004 · 18dd004
1 parent c54870d
commit 18dd004
Show file tree

Hide file tree

Showing 6 changed files with 39 additions and 30 deletions.
diff --git a/Cargo-minimal.lock b/Cargo-minimal.lock
@@ -208,8 +208,7 @@ dependencies = [
 [[package]]
 name = "bip21"
 version = "0.5.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "ebe7a7f5928d264879d5b65eb18a72ea1890c57f22d62ee2eba93f207a6a020b"
+source = "git+https://github.com/Kixunil/bip21.git?rev=refs/pull/26/head#413f845cc1a77dc85592e1a41a87624826052e1d"
 dependencies = [
  "bitcoin",
  "percent-encoding-rfc3986",

diff --git a/Cargo-recent.lock b/Cargo-recent.lock
@@ -208,8 +208,7 @@ dependencies = [
 [[package]]
 name = "bip21"
 version = "0.5.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "ebe7a7f5928d264879d5b65eb18a72ea1890c57f22d62ee2eba93f207a6a020b"
+source = "git+https://github.com/Kixunil/bip21.git?rev=refs/pull/26/head#413f845cc1a77dc85592e1a41a87624826052e1d"
 dependencies = [
  "bitcoin",
  "percent-encoding-rfc3986",

diff --git a/payjoin-cli/Cargo.toml b/payjoin-cli/Cargo.toml
@@ -28,7 +28,7 @@ v2 = ["payjoin/v2", "payjoin/io"]
 [dependencies]
 anyhow = "1.0.70"
 async-trait = "0.1"
-bip21 = "0.5.0"
+bip21 = { git = "https://github.com/Kixunil/bip21.git", rev = "refs/pull/26/head" }
 bitcoincore-rpc = "0.19.0"
 clap = { version = "~4.0.32", features = ["derive"] }
 config = "0.13.3"

diff --git a/payjoin/Cargo.toml b/payjoin/Cargo.toml
@@ -25,7 +25,7 @@ danger-local-https = ["io", "reqwest/rustls-tls", "rustls"]
 
 [dependencies]
 bitcoin = { version = "0.32.5", features = ["base64"] }
-bip21 = "0.5.0"
+bitcoin_uri = { git = "https://github.com/payjoin/bitcoin_uri.git" }
 hpke = { package = "bitcoin-hpke", version = "0.13.0", optional = true }
 log = { version = "0.4.14"}
 http = { version = "1", optional = true }

diff --git a/payjoin/src/uri/mod.rs b/payjoin/src/uri/mod.rs
@@ -100,8 +100,8 @@ impl PayjoinExtras {
     pub fn endpoint(&self) -> &Url { &self.endpoint }
 }
 
-pub type Uri<'a, NetworkValidation> = bip21::Uri<'a, NetworkValidation, MaybePayjoinExtras>;
-pub type PjUri<'a> = bip21::Uri<'a, NetworkChecked, PayjoinExtras>;
+pub type Uri<'a, NetworkValidation> = bitcoin_uri::Uri<'a, NetworkValidation, MaybePayjoinExtras>;
+pub type PjUri<'a> = bitcoin_uri::Uri<'a, NetworkChecked, PayjoinExtras>;
 
 mod sealed {
     use bitcoin::address::NetworkChecked;
@@ -115,22 +115,22 @@ mod sealed {
 pub trait UriExt<'a>: sealed::UriExt {
     // Error type is boxed to reduce the size of the Result
     // (See https://rust-lang.github.io/rust-clippy/master/index.html#result_large_err)
-    fn check_pj_supported(self) -> Result<PjUri<'a>, Box<bip21::Uri<'a>>>;
+    fn check_pj_supported(self) -> Result<PjUri<'a>, Box<bitcoin_uri::Uri<'a>>>;
 }
 
 impl<'a> UriExt<'a> for Uri<'a, NetworkChecked> {
-    fn check_pj_supported(self) -> Result<PjUri<'a>, Box<bip21::Uri<'a>>> {
+    fn check_pj_supported(self) -> Result<PjUri<'a>, Box<bitcoin_uri::Uri<'a>>> {
         match self.extras {
             MaybePayjoinExtras::Supported(payjoin) => {
-                let mut uri = bip21::Uri::with_extras(self.address, payjoin);
+                let mut uri = bitcoin_uri::Uri::with_extras(self.address, payjoin);
                 uri.amount = self.amount;
                 uri.label = self.label;
                 uri.message = self.message;
 
                 Ok(uri)
             }
             MaybePayjoinExtras::Unsupported => {
-                let mut uri = bip21::Uri::new(self.address);
+                let mut uri = bitcoin_uri::Uri::new(self.address);
                 uri.amount = self.amount;
                 uri.label = self.label;
                 uri.message = self.message;
@@ -220,11 +220,11 @@ impl PjUriBuilder {
 
     /// Build payjoin URI.
     ///
-    /// Constructs a `bip21::Uri` with PayjoinParams from the
+    /// Constructs a `bitcoin_uri::Uri` with PayjoinParams from the
     /// parameters set in the builder.
     pub fn build<'a>(self) -> PjUri<'a> {
         let extras = PayjoinExtras { endpoint: self.pj, disable_output_substitution: self.pjos };
-        let mut pj_uri = bip21::Uri::with_extras(self.address, extras);
+        let mut pj_uri = bitcoin_uri::Uri::with_extras(self.address, extras);
         pj_uri.amount = self.amount;
         pj_uri.label = self.label.map(Into::into);
         pj_uri.message = self.message.map(Into::into);
@@ -236,11 +236,11 @@ impl PayjoinExtras {
     pub fn is_output_substitution_disabled(&self) -> bool { self.disable_output_substitution }
 }
 
-impl bip21::de::DeserializationError for MaybePayjoinExtras {
+impl bitcoin_uri::de::DeserializationError for MaybePayjoinExtras {
     type Error = PjParseError;
 }
 
-impl bip21::de::DeserializeParams<'_> for MaybePayjoinExtras {
+impl bitcoin_uri::de::DeserializeParams<'_> for MaybePayjoinExtras {
     type DeserializationState = DeserializationState;
 }
 
@@ -250,7 +250,7 @@ pub struct DeserializationState {
     pjos: Option<bool>,
 }
 
-impl bip21::SerializeParams for &MaybePayjoinExtras {
+impl bitcoin_uri::SerializeParams for &MaybePayjoinExtras {
     type Key = &'static str;
     type Value = String;
     type Iterator = std::vec::IntoIter<(Self::Key, Self::Value)>;
@@ -263,7 +263,7 @@ impl bip21::SerializeParams for &MaybePayjoinExtras {
     }
 }
 
-impl bip21::SerializeParams for &PayjoinExtras {
+impl bitcoin_uri::SerializeParams for &PayjoinExtras {
     type Key = &'static str;
     type Value = String;
     type Iterator = std::vec::IntoIter<(Self::Key, Self::Value)>;
@@ -287,26 +287,26 @@ impl bip21::SerializeParams for &PayjoinExtras {
     }
 }
 
-impl bip21::de::DeserializationState<'_> for DeserializationState {
+impl bitcoin_uri::de::DeserializationState<'_> for DeserializationState {
     type Value = MaybePayjoinExtras;
 
     fn is_param_known(&self, param: &str) -> bool { matches!(param, "pj" | "pjos") }
 
     fn deserialize_temp(
         &mut self,
         key: &str,
-        value: bip21::Param<'_>,
+        value: bitcoin_uri::Param<'_>,
     ) -> std::result::Result<
-        bip21::de::ParamKind,
-        <Self::Value as bip21::DeserializationError>::Error,
+        bitcoin_uri::de::ParamKind,
+        <Self::Value as bitcoin_uri::DeserializationError>::Error,
     > {
         match key {
             "pj" if self.pj.is_none() => {
                 let endpoint = Cow::try_from(value).map_err(|_| InternalPjParseError::NotUtf8)?;
                 let url = Url::parse(&endpoint).map_err(|_| InternalPjParseError::BadEndpoint)?;
                 self.pj = Some(url);
 
-                Ok(bip21::de::ParamKind::Known)
+                Ok(bitcoin_uri::de::ParamKind::Known)
             }
             "pj" => Err(InternalPjParseError::DuplicateParams("pj").into()),
             "pjos" if self.pjos.is_none() => {
@@ -315,16 +315,17 @@ impl bip21::de::DeserializationState<'_> for DeserializationState {
                     "1" => self.pjos = Some(true),
                     _ => return Err(InternalPjParseError::BadPjOs.into()),
                 }
-                Ok(bip21::de::ParamKind::Known)
+                Ok(bitcoin_uri::de::ParamKind::Known)
             }
             "pjos" => Err(InternalPjParseError::DuplicateParams("pjos").into()),
-            _ => Ok(bip21::de::ParamKind::Unknown),
+            _ => Ok(bitcoin_uri::de::ParamKind::Unknown),
         }
     }
 
     fn finalize(
         self,
-    ) -> std::result::Result<Self::Value, <Self::Value as bip21::DeserializationError>::Error> {
+    ) -> std::result::Result<Self::Value, <Self::Value as bitcoin_uri::DeserializationError>::Error>
+    {
         match (self.pj, self.pjos) {
             (None, None) => Ok(MaybePayjoinExtras::Unsupported),
             (None, Some(_)) => Err(InternalPjParseError::MissingEndpoint.into()),

diff --git a/payjoin/src/uri/url_ext.rs b/payjoin/src/uri/url_ext.rs
@@ -160,9 +160,19 @@ mod tests {
     #[test]
     fn test_valid_v2_url_fragment_on_bip21() {
         let uri = "bitcoin:12c6DSiU4Rq3P4ZxziKxzrL5LmMBrzjrJX?amount=0.01\
-                   &pj=https://example.com\
-                   #OH1QYPM5JXYNS754Y4R45QWE336QFX6ZR8DQGVQCULVZTV20TFVEYDMFQC";
-        let uri = Uri::try_from(uri).unwrap().assume_checked().check_pj_supported().unwrap();
-        assert!(uri.extras.endpoint().ohttp().is_some());
+                   &pjos=0&pj=HTTPS://EXAMPLE.COM/\
+                   %23OH1QYPM5JXYNS754Y4R45QWE336QFX6ZR8DQGVQCULVZTV20TFVEYDMFQC";
+        let pjuri = Uri::try_from(uri).unwrap().assume_checked().check_pj_supported().unwrap();
+        assert!(pjuri.extras.endpoint().ohttp().is_some());
+        assert_eq!(format!("{}", pjuri), uri);
+
+        let reordered = "bitcoin:12c6DSiU4Rq3P4ZxziKxzrL5LmMBrzjrJX?amount=0.01\
+                   &pj=HTTPS://EXAMPLE.COM/\
+                   %23OH1QYPM5JXYNS754Y4R45QWE336QFX6ZR8DQGVQCULVZTV20TFVEYDMFQC\
+                   &pjos=0";
+        let pjuri =
+            Uri::try_from(reordered).unwrap().assume_checked().check_pj_supported().unwrap();
+        assert!(pjuri.extras.endpoint().ohttp().is_some());
+        assert_eq!(format!("{}", pjuri), uri);
     }
 }