Skip to content

Commit

Permalink
replace bip21 crate with bitcoin_uri
Browse files Browse the repository at this point in the history 
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.

The buggy deserialization likewise ignores #, parsing it as part of the
value which round trips with the incorrect serialization behavior.
  • Loading branch information
@nothingmuch
nothingmuch committed Dec 2, 2024
1 parent c54870d commit d822f02
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 50 deletions.
23 changes: 11 additions & 12 deletions Cargo-minimal.lock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,16 +205,6 @@ dependencies = [
"url",
]

[[package]]
name = "bip21"
version = "0.5.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "ebe7a7f5928d264879d5b65eb18a72ea1890c57f22d62ee2eba93f207a6a020b"
dependencies = [
"bitcoin",
"percent-encoding-rfc3986",
]

[[package]]
name = "bitcoin"
version = "0.32.5"
Expand Down Expand Up @@ -312,6 +302,16 @@ dependencies = [
"serde",
]

[[package]]
name = "bitcoin_uri"
version = "0.1.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "0e0a228e083d1702f83389b0ac71eb70078dc8d7fcbb6cde864d1cbca145f5cc"
dependencies = [
"bitcoin",
"percent-encoding-rfc3986",
]

[[package]]
name = "bitcoincore-rpc"
version = "0.19.0"
Expand Down Expand Up @@ -1580,10 +1580,10 @@ name = "payjoin"
version = "0.20.0"
dependencies = [
"bhttp",
"bip21",
"bitcoin",
"bitcoin-hpke",
"bitcoin-ohttp",
"bitcoin_uri",
"bitcoind",
"http",
"log",
Expand All @@ -1609,7 +1609,6 @@ version = "0.0.9-alpha"
dependencies = [
"anyhow",
"async-trait",
"bip21",
"bitcoincore-rpc",
"bitcoind",
"clap",
Expand Down
23 changes: 11 additions & 12 deletions Cargo-recent.lock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,16 +205,6 @@ dependencies = [
"url",
]

[[package]]
name = "bip21"
version = "0.5.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "ebe7a7f5928d264879d5b65eb18a72ea1890c57f22d62ee2eba93f207a6a020b"
dependencies = [
"bitcoin",
"percent-encoding-rfc3986",
]

[[package]]
name = "bitcoin"
version = "0.32.5"
Expand Down Expand Up @@ -312,6 +302,16 @@ dependencies = [
"serde",
]

[[package]]
name = "bitcoin_uri"
version = "0.1.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "0e0a228e083d1702f83389b0ac71eb70078dc8d7fcbb6cde864d1cbca145f5cc"
dependencies = [
"bitcoin",
"percent-encoding-rfc3986",
]

[[package]]
name = "bitcoincore-rpc"
version = "0.19.0"
Expand Down Expand Up @@ -1580,10 +1580,10 @@ name = "payjoin"
version = "0.20.0"
dependencies = [
"bhttp",
"bip21",
"bitcoin",
"bitcoin-hpke",
"bitcoin-ohttp",
"bitcoin_uri",
"bitcoind",
"http",
"log",
Expand All @@ -1609,7 +1609,6 @@ version = "0.0.9-alpha"
dependencies = [
"anyhow",
"async-trait",
"bip21",
"bitcoincore-rpc",
"bitcoind",
"clap",
Expand Down
1 change: 0 additions & 1 deletion payjoin-cli/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ v2 = ["payjoin/v2", "payjoin/io"]
[dependencies]
anyhow = "1.0.70"
async-trait = "0.1"
bip21 = "0.5.0"
bitcoincore-rpc = "0.19.0"
clap = { version = "~4.0.32", features = ["derive"] }
config = "0.13.3"
Expand Down
2 changes: 1 addition & 1 deletion payjoin/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 = "0.1.0"
hpke = { package = "bitcoin-hpke", version = "0.13.0", optional = true }
log = { version = "0.4.14"}
http = { version = "1", optional = true }
Expand Down
41 changes: 21 additions & 20 deletions payjoin/src/uri/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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;
}

Expand All @@ -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)>;
Expand All @@ -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)>;
Expand All @@ -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() => {
Expand All @@ -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()),
Expand Down
18 changes: 14 additions & 4 deletions payjoin/src/uri/url_ext.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

0 comments on commit d822f02

Please sign in to comment.