Propagate errors when retrieving ohttp parameter

payjoin · Dec 5, 2024 · fe6ecad · fe6ecad
1 parent 52d19ae
commit fe6ecad
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 10 deletions.
diff --git 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
@@ -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
@@ -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<HpkePublicKey, ParseReceiverPubkeyError>;
     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);
@@ -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,16 @@ 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 +174,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 +183,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);
     }
 }