Propagate ParseError when retrieving ohttp parameter (payjoin#428)

This PR updates the `ohttp()` getter to propagate the parse errors
as part of payjoin#399.

Author: DanGould

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<crate::psbt::AddressTypeError> for CreateRequestError {
 }
 
 #[cfg(feature = "v2")]
-impl From<ParseReceiverPubkeyError> for CreateRequestError {
-    fn from(value: ParseReceiverPubkeyError) -> Self {
+impl From<ParseReceiverPubkeyParamError> for CreateRequestError {
+    fn from(value: ParseReceiverPubkeyParamError) -> Self {
         CreateRequestError(InternalCreateRequestError::ParseReceiverPubkey(value))
     }
 }

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);
@@ -331,7 +331,7 @@ impl Sender {
     #[cfg(feature = "v2")]
     fn extract_rs_pubkey(
         &self,
-    ) -> Result<HpkePublicKey, crate::uri::error::ParseReceiverPubkeyError> {
+    ) -> Result<HpkePublicKey, crate::uri::error::ParseReceiverPubkeyParamError> {
         use crate::uri::UrlExt;
         self.endpoint.receiver_pubkey()
     }
@@ -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)?;
 

payjoin/src/uri/error.rs

@@ -13,17 +13,36 @@ pub(crate) enum InternalPjParseError {
 
 #[cfg(feature = "v2")]
 #[derive(Debug)]
-pub(crate) enum ParseReceiverPubkeyError {
+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 ParseReceiverPubkeyParamError {
     MissingPubkey,
     InvalidHrp(bitcoin::bech32::Hrp),
     DecodeBech32(bitcoin::bech32::primitives::decode::CheckedHrpstringError),
     InvalidPubkey(crate::hpke::HpkeError),
 }
 
 #[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"),
@@ -36,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,

payjoin/src/uri/url_ext.rs

@@ -5,36 +5,36 @@ use bitcoin::consensus::encode::Decodable;
 use bitcoin::consensus::Encodable;
 use url::Url;
 
-use super::error::ParseReceiverPubkeyError;
+use super::error::{ParseOhttpKeysParamError, ParseReceiverPubkeyParamError};
 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<HpkePublicKey, ParseReceiverPubkeyError>;
+    fn receiver_pubkey(&self) -> Result<HpkePublicKey, ParseReceiverPubkeyParamError>;
     fn set_receiver_pubkey(&mut self, exp: HpkePublicKey);
-    fn ohttp(&self) -> Option<OhttpKeys>;
+    fn ohttp(&self) -> Result<OhttpKeys, ParseOhttpKeysParamError>;
     fn set_ohttp(&mut self, ohttp: OhttpKeys);
     fn exp(&self) -> Option<std::time::SystemTime>;
     fn set_exp(&mut self, exp: std::time::SystemTime);
 }
 
 impl UrlExt for Url {
     /// Retrieve the receiver's public key from the URL fragment
-    fn receiver_pubkey(&self) -> Result<HpkePublicKey, ParseReceiverPubkeyError> {
+    fn receiver_pubkey(&self) -> Result<HpkePublicKey, ParseReceiverPubkeyParamError> {
         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
@@ -50,8 +50,10 @@ impl UrlExt for Url {
     }
 
     /// Retrieve the ohttp parameter from the URL fragment
-    fn ohttp(&self) -> Option<OhttpKeys> {
-        get_param(self, "OH1", |value| OhttpKeys::from_str(value).ok())
+    fn ohttp(&self) -> Result<OhttpKeys, ParseOhttpKeysParamError> {
+        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);
     }
 }