From 9e60019694d10724406e373b8ce45094142929dc Mon Sep 17 00:00:00 2001 From: Shing Him Ng Date: Tue, 3 Dec 2024 22:31:35 -0600 Subject: [PATCH 1/2] Propagate errors when retrieving ohttp parameter --- payjoin/src/send/mod.rs | 4 ++-- payjoin/src/uri/error.rs | 19 +++++++++++++++++++ payjoin/src/uri/url_ext.rs | 35 +++++++++++++++++++++++++++-------- 3 files changed, 48 insertions(+), 10 deletions(-) diff --git a/payjoin/src/send/mod.rs b/payjoin/src/send/mod.rs index 9fe2742b..60a21302 100644 --- a/payjoin/src/send/mod.rs +++ b/payjoin/src/send/mod.rs @@ -306,7 +306,7 @@ impl Sender { ) .map_err(InternalCreateRequestError::Hpke)?; let mut ohttp = - self.endpoint.ohttp().ok_or(InternalCreateRequestError::MissingOhttpConfig)?; + self.endpoint.ohttp().map_err(|_| InternalCreateRequestError::MissingOhttpConfig)?; let (body, ohttp_ctx) = ohttp_encapsulate(&mut ohttp, "POST", url.as_str(), Some(&body)) .map_err(InternalCreateRequestError::OhttpEncapsulation)?; log::debug!("ohttp_relay_url: {:?}", ohttp_relay); @@ -418,7 +418,7 @@ impl V2GetContext { ) .map_err(InternalCreateRequestError::Hpke)?; let mut ohttp = - self.endpoint.ohttp().ok_or(InternalCreateRequestError::MissingOhttpConfig)?; + self.endpoint.ohttp().map_err(|_| InternalCreateRequestError::MissingOhttpConfig)?; let (body, ohttp_ctx) = ohttp_encapsulate(&mut ohttp, "GET", url.as_str(), Some(&body)) .map_err(InternalCreateRequestError::OhttpEncapsulation)?; diff --git a/payjoin/src/uri/error.rs b/payjoin/src/uri/error.rs index f4ef7fab..4728020e 100644 --- a/payjoin/src/uri/error.rs +++ b/payjoin/src/uri/error.rs @@ -11,6 +11,25 @@ pub(crate) enum InternalPjParseError { UnsecureEndpoint, } +#[cfg(feature = "v2")] +#[derive(Debug)] +pub(crate) enum ParseOhttpKeysParamError { + MissingOhttpKeys, + InvalidOhttpKeys(crate::ohttp::ParseOhttpKeysError), +} + +#[cfg(feature = "v2")] +impl std::fmt::Display for ParseOhttpKeysParamError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + use ParseOhttpKeysParamError::*; + + match &self { + MissingOhttpKeys => write!(f, "ohttp keys are missing"), + InvalidOhttpKeys(o) => write!(f, "invalid ohttp keys: {}", o), + } + } +} + #[cfg(feature = "v2")] #[derive(Debug)] pub(crate) enum ParseReceiverPubkeyError { diff --git a/payjoin/src/uri/url_ext.rs b/payjoin/src/uri/url_ext.rs index 15c63ee4..67a95921 100644 --- a/payjoin/src/uri/url_ext.rs +++ b/payjoin/src/uri/url_ext.rs @@ -5,15 +5,15 @@ use bitcoin::consensus::encode::Decodable; use bitcoin::consensus::Encodable; use url::Url; -use super::error::ParseReceiverPubkeyError; +use super::error::{ParseOhttpKeysParamError, ParseReceiverPubkeyError}; use crate::hpke::HpkePublicKey; -use crate::OhttpKeys; +use crate::ohttp::OhttpKeys; /// Parse and set fragment parameters from `&pj=` URI parameter URLs pub(crate) trait UrlExt { fn receiver_pubkey(&self) -> Result; fn set_receiver_pubkey(&mut self, exp: HpkePublicKey); - fn ohttp(&self) -> Option; + fn ohttp(&self) -> Result; fn set_ohttp(&mut self, ohttp: OhttpKeys); fn exp(&self) -> Option; fn set_exp(&mut self, exp: std::time::SystemTime); @@ -50,8 +50,10 @@ impl UrlExt for Url { } /// Retrieve the ohttp parameter from the URL fragment - fn ohttp(&self) -> Option { - get_param(self, "OH1", |value| OhttpKeys::from_str(value).ok()) + fn ohttp(&self) -> Result { + let value = get_param(self, "OH1", |v| Some(v.to_owned())) + .ok_or(ParseOhttpKeysParamError::MissingOhttpKeys)?; + OhttpKeys::from_str(&value).map_err(ParseOhttpKeysParamError::InvalidOhttpKeys) } /// Set the ohttp parameter in the URL fragment @@ -142,7 +144,24 @@ mod tests { url.set_ohttp(ohttp_keys.clone()); assert_eq!(url.fragment(), Some(serialized)); - assert_eq!(url.ohttp(), Some(ohttp_keys)); + assert_eq!(url.ohttp().unwrap(), ohttp_keys); + } + + #[test] + fn test_errors_when_parsing_ohttp() { + let missing_ohttp_url = Url::parse("https://example.com").unwrap(); + assert!(matches!( + missing_ohttp_url.ohttp(), + Err(ParseOhttpKeysParamError::MissingOhttpKeys) + )); + + let invalid_ohttp_url = + Url::parse("https://example.com?pj=https://test-payjoin-url#OH1invalid_bech_32") + .unwrap(); + assert!(matches!( + invalid_ohttp_url.ohttp(), + Err(ParseOhttpKeysParamError::InvalidOhttpKeys(_)) + )); } #[test] @@ -163,7 +182,7 @@ mod tests { &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!(pjuri.extras.endpoint().ohttp().is_ok()); assert_eq!(format!("{}", pjuri), uri); let reordered = "bitcoin:12c6DSiU4Rq3P4ZxziKxzrL5LmMBrzjrJX?amount=0.01\ @@ -172,7 +191,7 @@ mod tests { &pjos=0"; let pjuri = Uri::try_from(reordered).unwrap().assume_checked().check_pj_supported().unwrap(); - assert!(pjuri.extras.endpoint().ohttp().is_some()); + assert!(pjuri.extras.endpoint().ohttp().is_ok()); assert_eq!(format!("{}", pjuri), uri); } } From accdaca5b603a6749add92fb97b96c9c8bb2ad1c Mon Sep 17 00:00:00 2001 From: Shing Him Ng Date: Thu, 5 Dec 2024 15:45:06 -0600 Subject: [PATCH 2/2] Rename ParseReceiverPubkeyError to ParseReceiverPubkeyParamError This change will differentiate between parsing a receiver pubkey in the context of a URL and other formats --- payjoin/src/send/error.rs | 8 ++++---- payjoin/src/send/mod.rs | 2 +- payjoin/src/uri/error.rs | 10 +++++----- payjoin/src/uri/url_ext.rs | 14 +++++++------- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/payjoin/src/send/error.rs b/payjoin/src/send/error.rs index 7de18a1f..78d20765 100644 --- a/payjoin/src/send/error.rs +++ b/payjoin/src/send/error.rs @@ -5,7 +5,7 @@ use bitcoin::transaction::Version; use bitcoin::{AddressType, Sequence}; #[cfg(feature = "v2")] -use crate::uri::error::ParseReceiverPubkeyError; +use crate::uri::error::ParseReceiverPubkeyParamError; /// Error that may occur when the response from receiver is malformed. /// @@ -203,7 +203,7 @@ pub(crate) enum InternalCreateRequestError { #[cfg(feature = "v2")] OhttpEncapsulation(crate::ohttp::OhttpEncapsulationError), #[cfg(feature = "v2")] - ParseReceiverPubkey(ParseReceiverPubkeyError), + ParseReceiverPubkey(ParseReceiverPubkeyParamError), #[cfg(feature = "v2")] MissingOhttpConfig, #[cfg(feature = "v2")] @@ -287,8 +287,8 @@ impl From for CreateRequestError { } #[cfg(feature = "v2")] -impl From for CreateRequestError { - fn from(value: ParseReceiverPubkeyError) -> Self { +impl From for CreateRequestError { + fn from(value: ParseReceiverPubkeyParamError) -> Self { CreateRequestError(InternalCreateRequestError::ParseReceiverPubkey(value)) } } diff --git a/payjoin/src/send/mod.rs b/payjoin/src/send/mod.rs index 60a21302..735b1039 100644 --- a/payjoin/src/send/mod.rs +++ b/payjoin/src/send/mod.rs @@ -331,7 +331,7 @@ impl Sender { #[cfg(feature = "v2")] fn extract_rs_pubkey( &self, - ) -> Result { + ) -> Result { use crate::uri::UrlExt; self.endpoint.receiver_pubkey() } diff --git a/payjoin/src/uri/error.rs b/payjoin/src/uri/error.rs index 4728020e..34a05481 100644 --- a/payjoin/src/uri/error.rs +++ b/payjoin/src/uri/error.rs @@ -32,7 +32,7 @@ impl std::fmt::Display for ParseOhttpKeysParamError { #[cfg(feature = "v2")] #[derive(Debug)] -pub(crate) enum ParseReceiverPubkeyError { +pub(crate) enum ParseReceiverPubkeyParamError { MissingPubkey, InvalidHrp(bitcoin::bech32::Hrp), DecodeBech32(bitcoin::bech32::primitives::decode::CheckedHrpstringError), @@ -40,9 +40,9 @@ pub(crate) enum ParseReceiverPubkeyError { } #[cfg(feature = "v2")] -impl std::fmt::Display for ParseReceiverPubkeyError { +impl std::fmt::Display for ParseReceiverPubkeyParamError { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - use ParseReceiverPubkeyError::*; + use ParseReceiverPubkeyParamError::*; match &self { MissingPubkey => write!(f, "receiver public key is missing"), @@ -55,9 +55,9 @@ impl std::fmt::Display for ParseReceiverPubkeyError { } #[cfg(feature = "v2")] -impl std::error::Error for ParseReceiverPubkeyError { +impl std::error::Error for ParseReceiverPubkeyParamError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - use ParseReceiverPubkeyError::*; + use ParseReceiverPubkeyParamError::*; match &self { MissingPubkey => None, diff --git a/payjoin/src/uri/url_ext.rs b/payjoin/src/uri/url_ext.rs index 67a95921..8ad080a9 100644 --- a/payjoin/src/uri/url_ext.rs +++ b/payjoin/src/uri/url_ext.rs @@ -5,13 +5,13 @@ use bitcoin::consensus::encode::Decodable; use bitcoin::consensus::Encodable; use url::Url; -use super::error::{ParseOhttpKeysParamError, ParseReceiverPubkeyError}; +use super::error::{ParseOhttpKeysParamError, ParseReceiverPubkeyParamError}; use crate::hpke::HpkePublicKey; use crate::ohttp::OhttpKeys; /// Parse and set fragment parameters from `&pj=` URI parameter URLs pub(crate) trait UrlExt { - fn receiver_pubkey(&self) -> Result; + fn receiver_pubkey(&self) -> Result; fn set_receiver_pubkey(&mut self, exp: HpkePublicKey); fn ohttp(&self) -> Result; fn set_ohttp(&mut self, ohttp: OhttpKeys); @@ -21,20 +21,20 @@ pub(crate) trait UrlExt { impl UrlExt for Url { /// Retrieve the receiver's public key from the URL fragment - fn receiver_pubkey(&self) -> Result { + fn receiver_pubkey(&self) -> Result { let value = get_param(self, "RK1", |v| Some(v.to_owned())) - .ok_or(ParseReceiverPubkeyError::MissingPubkey)?; + .ok_or(ParseReceiverPubkeyParamError::MissingPubkey)?; let (hrp, bytes) = crate::bech32::nochecksum::decode(&value) - .map_err(ParseReceiverPubkeyError::DecodeBech32)?; + .map_err(ParseReceiverPubkeyParamError::DecodeBech32)?; let rk_hrp: Hrp = Hrp::parse("RK").unwrap(); if hrp != rk_hrp { - return Err(ParseReceiverPubkeyError::InvalidHrp(hrp)); + return Err(ParseReceiverPubkeyParamError::InvalidHrp(hrp)); } HpkePublicKey::from_compressed_bytes(&bytes[..]) - .map_err(ParseReceiverPubkeyError::InvalidPubkey) + .map_err(ParseReceiverPubkeyParamError::InvalidPubkey) } /// Set the receiver's public key in the URL fragment