Skip to content

Commit

Permalink
Propagate errors when retrieving exp parameter (#441)
Browse files Browse the repository at this point in the history 
Follow-up of #428 to propagate errors when retrieving the `exp`
parameter. This is the last of the parameter getters that need an update
to return a `Result`, so this will close #399
  • Loading branch information
@DanGould
DanGould authored Dec 14, 2024
2 parents 0891a5b + 4ea00db commit e8f1069
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 77 deletions.
2 changes: 1 addition & 1 deletion payjoin/src/send/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use bitcoin::transaction::Version;
use bitcoin::{AddressType, Sequence};

#[cfg(feature = "v2")]
use crate::uri::error::ParseReceiverPubkeyParamError;
use crate::uri::url_ext::ParseReceiverPubkeyParamError;

/// Error that may occur when the response from receiver is malformed.
///
Expand Down
4 changes: 2 additions & 2 deletions payjoin/src/send/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ impl Sender {
ohttp_relay: Url,
) -> Result<(Request, V2PostContext), CreateRequestError> {
use crate::uri::UrlExt;
if let Some(expiry) = self.endpoint.exp() {
if let Ok(expiry) = self.endpoint.exp() {
if std::time::SystemTime::now() > expiry {
return Err(InternalCreateRequestError::Expired(expiry).into());
}
Expand Down Expand Up @@ -331,7 +331,7 @@ impl Sender {
#[cfg(feature = "v2")]
fn extract_rs_pubkey(
&self,
) -> Result<HpkePublicKey, crate::uri::error::ParseReceiverPubkeyParamError> {
) -> Result<HpkePublicKey, crate::uri::url_ext::ParseReceiverPubkeyParamError> {
use crate::uri::UrlExt;
self.endpoint.receiver_pubkey()
}
Expand Down
57 changes: 0 additions & 57 deletions payjoin/src/uri/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,63 +11,6 @@ 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 ParseReceiverPubkeyParamError {
MissingPubkey,
InvalidHrp(bitcoin::bech32::Hrp),
DecodeBech32(bitcoin::bech32::primitives::decode::CheckedHrpstringError),
InvalidPubkey(crate::hpke::HpkeError),
}

#[cfg(feature = "v2")]
impl std::fmt::Display for ParseReceiverPubkeyParamError {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
use ParseReceiverPubkeyParamError::*;

match &self {
MissingPubkey => write!(f, "receiver public key is missing"),
InvalidHrp(h) => write!(f, "incorrect hrp for receiver key: {}", h),
DecodeBech32(e) => write!(f, "receiver public is not valid base64: {}", e),
InvalidPubkey(e) =>
write!(f, "receiver public key does not represent a valid pubkey: {}", e),
}
}
}

#[cfg(feature = "v2")]
impl std::error::Error for ParseReceiverPubkeyParamError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
use ParseReceiverPubkeyParamError::*;

match &self {
MissingPubkey => None,
InvalidHrp(_) => None,
DecodeBech32(error) => Some(error),
InvalidPubkey(error) => Some(error),
}
}
}

impl From<InternalPjParseError> for PjParseError {
fn from(value: InternalPjParseError) -> Self { PjParseError(value) }
}
Expand Down
137 changes: 120 additions & 17 deletions payjoin/src/uri/url_ext.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use bitcoin::consensus::encode::Decodable;
use bitcoin::consensus::Encodable;
use url::Url;

use super::error::{ParseOhttpKeysParamError, ParseReceiverPubkeyParamError};
use crate::hpke::HpkePublicKey;
use crate::ohttp::OhttpKeys;

Expand All @@ -15,7 +14,7 @@ pub(crate) trait UrlExt {
fn set_receiver_pubkey(&mut self, exp: HpkePublicKey);
fn ohttp(&self) -> Result<OhttpKeys, ParseOhttpKeysParamError>;
fn set_ohttp(&mut self, ohttp: OhttpKeys);
fn exp(&self) -> Option<std::time::SystemTime>;
fn exp(&self) -> Result<std::time::SystemTime, ParseExpParamError>;
fn set_exp(&mut self, exp: std::time::SystemTime);
}

Expand Down Expand Up @@ -60,22 +59,23 @@ impl UrlExt for Url {
fn set_ohttp(&mut self, ohttp: OhttpKeys) { set_param(self, "OH1", &ohttp.to_string()) }

/// Retrieve the exp parameter from the URL fragment
fn exp(&self) -> Option<std::time::SystemTime> {
get_param(self, "EX1", |value| {
let (hrp, bytes) = crate::bech32::nochecksum::decode(value).ok()?;
fn exp(&self) -> Result<std::time::SystemTime, ParseExpParamError> {
let value =
get_param(self, "EX1", |v| Some(v.to_owned())).ok_or(ParseExpParamError::MissingExp)?;

let ex_hrp: Hrp = Hrp::parse("EX").unwrap();
if hrp != ex_hrp {
return None;
}
let (hrp, bytes) =
crate::bech32::nochecksum::decode(&value).map_err(ParseExpParamError::DecodeBech32)?;

let mut cursor = &bytes[..];
u32::consensus_decode(&mut cursor)
.map(|timestamp| {
std::time::UNIX_EPOCH + std::time::Duration::from_secs(timestamp as u64)
})
.ok()
})
let ex_hrp: Hrp = Hrp::parse("EX").unwrap();
if hrp != ex_hrp {
return Err(ParseExpParamError::InvalidHrp(hrp));
}

u32::consensus_decode(&mut &bytes[..])
.map(|timestamp| {
std::time::UNIX_EPOCH + std::time::Duration::from_secs(timestamp as u64)
})
.map_err(ParseExpParamError::InvalidExp)
}

/// Set the exp parameter in the URL fragment
Expand Down Expand Up @@ -130,6 +130,87 @@ fn set_param(url: &mut Url, prefix: &str, param: &str) {
url.set_fragment(if fragment.is_empty() { None } else { Some(&fragment) });
}

#[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 ParseExpParamError {
MissingExp,
InvalidHrp(bitcoin::bech32::Hrp),
DecodeBech32(bitcoin::bech32::primitives::decode::CheckedHrpstringError),
InvalidExp(bitcoin::consensus::encode::Error),
}

#[cfg(feature = "v2")]
impl std::fmt::Display for ParseExpParamError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
use ParseExpParamError::*;

match &self {
MissingExp => write!(f, "exp is missing"),
InvalidHrp(h) => write!(f, "incorrect hrp for exp: {}", h),
DecodeBech32(d) => write!(f, "exp is not valid bech32: {}", d),
InvalidExp(i) =>
write!(f, "exp param does not contain a bitcoin consensus encoded u32: {}", i),
}
}
}

#[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 ParseReceiverPubkeyParamError {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
use ParseReceiverPubkeyParamError::*;

match &self {
MissingPubkey => write!(f, "receiver public key is missing"),
InvalidHrp(h) => write!(f, "incorrect hrp for receiver key: {}", h),
DecodeBech32(e) => write!(f, "receiver public is not valid base64: {}", e),
InvalidPubkey(e) =>
write!(f, "receiver public key does not represent a valid pubkey: {}", e),
}
}
}

#[cfg(feature = "v2")]
impl std::error::Error for ParseReceiverPubkeyParamError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
use ParseReceiverPubkeyParamError::*;

match &self {
MissingPubkey => None,
InvalidHrp(_) => None,
DecodeBech32(error) => Some(error),
InvalidPubkey(error) => Some(error),
}
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -173,7 +254,29 @@ mod tests {
url.set_exp(exp_time);
assert_eq!(url.fragment(), Some("EX1C4UC6ES"));

assert_eq!(url.exp(), Some(exp_time));
assert_eq!(url.exp().unwrap(), exp_time);
}

#[test]
fn test_errors_when_parsing_exp() {
let missing_exp_url = Url::parse("http://example.com").unwrap();
assert!(matches!(missing_exp_url.exp(), Err(ParseExpParamError::MissingExp)));

let invalid_bech32_exp_url =
Url::parse("http://example.com?pj=https://test-payjoin-url#EX1invalid_bech_32")
.unwrap();
assert!(matches!(invalid_bech32_exp_url.exp(), Err(ParseExpParamError::DecodeBech32(_))));

// Since the HRP is everything to the left of the right-most separator, the invalid url in
// this test would have it's HRP being parsed as EX101 instead of the expected EX1
let invalid_hrp_exp_url =
Url::parse("http://example.com?pj=https://test-payjoin-url#EX1010").unwrap();
assert!(matches!(invalid_hrp_exp_url.exp(), Err(ParseExpParamError::InvalidHrp(_))));

// Not enough data to decode into a u32
let invalid_timestamp_exp_url =
Url::parse("http://example.com?pj=https://test-payjoin-url#EX10").unwrap();
assert!(matches!(invalid_timestamp_exp_url.exp(), Err(ParseExpParamError::InvalidExp(_))))
}

#[test]
Expand Down

0 comments on commit e8f1069

Please sign in to comment.