Skip to content

Commit

Permalink
fix: restrict which characters can be used in fingerprint components
Browse files Browse the repository at this point in the history
  • Loading branch information
huin committed Jul 6, 2024
1 parent 0e09b56 commit b785b13
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 29 deletions.
35 changes: 32 additions & 3 deletions src/fingerprint.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use anyhow::{bail, Result};
use byteorder::{BigEndian, ByteOrder};
use chrono::{Datelike, NaiveDate, NaiveTime, Timelike};
use lazy_static::lazy_static;
use ledger_parser::Amount;
use regex::Regex;
use sha1::{Digest, Sha1};

use crate::tags;
Expand Down Expand Up @@ -53,13 +56,39 @@ pub struct FingerprintBuilder {
}

impl FingerprintBuilder {
pub fn new(algorithm_name: &'static str, algorithm_version: i64, user_namespace: &str) -> Self {
Self {
pub fn new(
algorithm_name: &'static str,
algorithm_version: i64,
user_namespace: &str,
) -> Result<Self> {
lazy_static! {
// This regex should only allow characters that don't conflict with
// the tag structure generated by [Fingerprint::tag].
// Including '+' and '/' allow for standard base64 encoding.
static ref VALID_FP_PART_RX: Regex = Regex::new(r#"^[a-zA-Z0-9_+/]*$"#).unwrap();
}

if !VALID_FP_PART_RX.is_match(&algorithm_name) {
bail!(
"fingerprint algorithm name {:?} must match regex {:?}",
algorithm_name,
VALID_FP_PART_RX.as_str()
);
}
if !VALID_FP_PART_RX.is_match(user_namespace) {
bail!(
"fingerprint user namespace {:?} must match regex {:?}",
user_namespace,
VALID_FP_PART_RX.as_str()
);
}

Ok(Self {
acc: Accumulator::new(),
algorithm_name,
algorithm_version,
user_namespace: user_namespace.to_string(),
}
})
}

pub fn build(self) -> Fingerprint {
Expand Down
37 changes: 24 additions & 13 deletions src/importers/nationwide_csv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ impl PostingFormer for RecordFive {
_ => bail!("expected *either* paid in or paid out"),
};
let halves = self_and_peer_account_amount(self_amount, ASSETS_UNKNOWN.to_string());
let fp_v1 = self.fingerprint_v1(fp_namespace, date_counter);
let fp_v1 = self.fingerprint_v1(fp_namespace, date_counter)?;
let mut self_comment = Comment::builder()
.with_tag(tags::UNKNOWN_ACCOUNT)
.with_value_tag(tags::ACCOUNT, account_name)
Expand Down Expand Up @@ -281,15 +281,15 @@ impl PostingFormer for RecordSix {
.with_value_tag(tags::BANK, BANK_NAME)
.with_value_tag(TRANSACTION_TYPE_TAG, self.type_.clone());
let mut peer_comment = self_comment.clone();
let fp_v1 = self.fingerprint_v1(fp_namespace, date_counter);
let fp_v1 = self.fingerprint_v1(fp_namespace, date_counter)?;
self_comment = self_comment
.with_tag(fp_v1.self_.tag())
.with_tag(tags::IMPORT_SELF.to_string());
peer_comment = peer_comment
.with_tag(fp_v1.peer.tag())
.with_tag(tags::IMPORT_PEER.to_string());
if include_legacy_fingerprint {
let fp_legacy = self.fingerprint_legacy(fp_namespace, date_counter, &halves);
let fp_legacy = self.fingerprint_legacy(fp_namespace, date_counter, &halves)?;
self_comment = self_comment.with_tag(fp_legacy.self_.legacy_tag());
peer_comment = peer_comment.with_tag(fp_legacy.peer.legacy_tag());
}
Expand Down Expand Up @@ -318,7 +318,7 @@ mod de {
use std::fmt;
use std::str::FromStr;

use anyhow::{bail, Result};
use anyhow::{bail, Context, Result};
use chrono::NaiveDate;
use lazy_static::lazy_static;
use ledger_parser::{Amount, Commodity, CommodityPosition};
Expand All @@ -345,16 +345,21 @@ mod de {
}

impl RecordFive {
pub fn fingerprint_v1(&self, fp_namespace: &str, date_counter: i32) -> FingerprintHalves {
self_and_peer_fingerprints(
pub fn fingerprint_v1(
&self,
fp_namespace: &str,
date_counter: i32,
) -> Result<FingerprintHalves> {
Ok(self_and_peer_fingerprints(
FingerprintBuilder::new("nwcsv5", 1, fp_namespace)
.with_context(|| "building v1 fingerprint")?
.with(self.date.0)
.with(date_counter)
.with(self.transactions.as_str())
.with(self.location.as_str())
.with(self.paid_out.as_ref())
.with(self.paid_in.as_ref()),
)
))
}
}

Expand All @@ -378,8 +383,9 @@ mod de {
fp_namespace: &str,
date_counter: i32,
halves: &TransactionHalves,
) -> FingerprintHalves {
) -> Result<FingerprintHalves> {
let fpb_legacy = FingerprintBuilder::new("", 0, fp_namespace)
.with_context(|| "building legacy fingerprint")?
.with(self.type_.as_str())
.with(self.date.0)
// Description should have been included in the legacy fingerprint, but a
Expand All @@ -396,23 +402,28 @@ mod de {
.with(halves.peer.account.as_str())
.with(&halves.peer.amount);

FingerprintHalves {
Ok(FingerprintHalves {
self_: self_fp.build(),
peer: peer_fp.build(),
}
})
}

pub fn fingerprint_v1(&self, fp_namespace: &str, date_counter: i32) -> FingerprintHalves {
self_and_peer_fingerprints(
pub fn fingerprint_v1(
&self,
fp_namespace: &str,
date_counter: i32,
) -> Result<FingerprintHalves> {
Ok(self_and_peer_fingerprints(
FingerprintBuilder::new("nwcsv6", 1, fp_namespace)
.with_context(|| "building v1 fingerprint")?
.with(self.type_.as_str())
.with(self.date.0)
.with(date_counter)
.with(self.description.as_str())
.with(self.paid_out.as_ref())
.with(self.paid_in.as_ref())
.with(&self.balance),
)
))
}
}

Expand Down
25 changes: 13 additions & 12 deletions src/importers/nationwide_pdf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ impl TransactionImporter for NationwidePdf {
}
}

let transactions = acc.build();
let transactions = acc.build()?;
Ok(Import {
user_fp_namespace,
transactions,
Expand Down Expand Up @@ -208,7 +208,7 @@ impl TransactionsAccumulator {
}
(Some(payment), None) => {
// Start of new payment transaction.
self.flush_transaction();
self.flush_transaction()?;
if trn_line.implied_date != self.prev_date {
self.date_counter = 0;
} else {
Expand All @@ -224,7 +224,7 @@ impl TransactionsAccumulator {
}
(None, Some(receipt)) => {
// Start of new receipt transaction.
self.flush_transaction();
self.flush_transaction()?;
if trn_line.implied_date != self.prev_date {
self.date_counter = 0;
} else {
Expand Down Expand Up @@ -276,15 +276,16 @@ impl TransactionsAccumulator {
Ok(())
}

fn flush_transaction(&mut self) {
fn flush_transaction(&mut self) -> Result<()> {
if let Some(pending) = self.cur_trn_opt.take() {
self.trns.push(pending.build(&self.fp_ns));
self.trns.push(pending.build(&self.fp_ns)?);
}
Ok(())
}

fn build(mut self) -> Vec<Transaction> {
self.flush_transaction();
self.trns
fn build(mut self) -> Result<Vec<Transaction>> {
self.flush_transaction()?;
Ok(self.trns)
}
}

Expand Down Expand Up @@ -332,8 +333,8 @@ impl TransactionBuilder {
})
}

fn build(self, fp_ns: &str) -> Transaction {
let record_fpb = FingerprintBuilder::new("nwpdf", 1, fp_ns)
fn build(self, fp_ns: &str) -> Result<Transaction> {
let record_fpb = FingerprintBuilder::new("nwpdf", 1, fp_ns)?
.with(self.date)
.with(self.date_counter)
.with(self.description.as_str());
Expand All @@ -357,7 +358,7 @@ impl TransactionBuilder {
.with(halves.peer.account.as_str())
.with(&halves.peer.amount);

Transaction {
Ok(Transaction {
date: self.date,
effective_date: self.effective_date,
status: None,
Expand Down Expand Up @@ -391,7 +392,7 @@ impl TransactionBuilder {
.into_opt_comment(),
},
],
}
})
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/importers/paypal_csv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ impl Record {
quantity: v.balance,
commodity,
};
let partial_fp = FingerprintBuilder::new("ppcsv", 1, fp_ns)
let partial_fp = FingerprintBuilder::new("ppcsv", 1, fp_ns)?
.with(v.date.0)
.with(v.time.0)
.with(v.time_zone.as_str())
Expand Down

0 comments on commit b785b13

Please sign in to comment.