Skip to content

Commit

Permalink
Fail Unified QR URI parsing if any known param fails
Browse files Browse the repository at this point in the history
Previously, we decided to continue parsing any fields if we failed to
parse a known (i.e., `lightning` or `lno`) parameter failed to parse.
This however just hides the error and is a bit anti-idiomatic even
though allowing to use *some* URI fields even in the face of
incompatible formats for others.

Here we therefore opt to fail parsing the URI if any field fails.
  • Loading branch information
tnull committed Jan 13, 2025
1 parent 71dffa7 commit 0d18f90
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 21 deletions.
21 changes: 9 additions & 12 deletions src/payment/unified_qr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,22 +256,19 @@ impl<'a> bip21::de::DeserializationState<'a> for DeserializationState {
"lightning" => {
let bolt11_value =
String::try_from(value).map_err(|_| Error::UriParameterParsingFailed)?;
if let Ok(invoice) = bolt11_value.parse::<Bolt11Invoice>() {
self.bolt11_invoice = Some(invoice);
Ok(bip21::de::ParamKind::Known)
} else {
Ok(bip21::de::ParamKind::Unknown)
}
let invoice = bolt11_value
.parse::<Bolt11Invoice>()
.map_err(|_| Error::UriParameterParsingFailed)?;
self.bolt11_invoice = Some(invoice);
Ok(bip21::de::ParamKind::Known)
},
"lno" => {
let bolt12_value =
String::try_from(value).map_err(|_| Error::UriParameterParsingFailed)?;
if let Ok(offer) = bolt12_value.parse::<Offer>() {
self.bolt12_offer = Some(offer);
Ok(bip21::de::ParamKind::Known)
} else {
Ok(bip21::de::ParamKind::Unknown)
}
let offer =
bolt12_value.parse::<Offer>().map_err(|_| Error::UriParameterParsingFailed)?;
self.bolt12_offer = Some(offer);
Ok(bip21::de::ParamKind::Known)
},
_ => Ok(bip21::de::ParamKind::Unknown),
}
Expand Down
15 changes: 6 additions & 9 deletions tests/integration_tests_rust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -868,11 +868,10 @@ fn unified_qr_send_receive() {

expect_payment_successful_event!(node_a, Some(offer_payment_id), None);

// Removed one character from the offer to fall back on to invoice.
// Still needs work
let uri_str_with_invalid_offer = &uri_str[..uri_str.len() - 1];
// Cut off the BOLT12 part to fallback to BOLT11.
let uri_str_without_offer = uri_str.split("&lno=").next().unwrap();
let invoice_payment_id: PaymentId =
match node_a.unified_qr_payment().send(uri_str_with_invalid_offer) {
match node_a.unified_qr_payment().send(uri_str_without_offer) {
Ok(QrPaymentResult::Bolt12 { payment_id: _ }) => {
panic!("Expected Bolt11 payment but got Bolt12");
},
Expand All @@ -893,11 +892,9 @@ fn unified_qr_send_receive() {
let onchain_uqr_payment =
node_b.unified_qr_payment().receive(expect_onchain_amount_sats, "asdf", 4_000).unwrap();

// Removed a character from the offer, so it would move on to the other parameters.
let txid = match node_a
.unified_qr_payment()
.send(&onchain_uqr_payment.as_str()[..onchain_uqr_payment.len() - 1])
{
// Cut off any lightning part to fallback to on-chain only.
let uri_str_without_lightning = onchain_uqr_payment.split("&lightning=").next().unwrap();
let txid = match node_a.unified_qr_payment().send(&uri_str_without_lightning) {
Ok(QrPaymentResult::Bolt12 { payment_id: _ }) => {
panic!("Expected on-chain payment but got Bolt12")
},
Expand Down

0 comments on commit 0d18f90

Please sign in to comment.