From e5d3ce3b1f1373f6b4e136f5272739a38b92c56d Mon Sep 17 00:00:00 2001 From: niftynei Date: Tue, 19 Jul 2022 17:04:40 +0930 Subject: [PATCH] bkpr incomestmt: properly escape things for the CSVs First off, when we pull data out of JSON, unescape it so we don't end up with extraneous escapes in our bookkeeping data. I promise, it's worth it. Then, when we print descriptions out to the csvs, we gotta wrap everything in quotes... but also we have to change all the double-quotes to singles so that adding the quotes doesn't do anything untoward. We also just pass it thru json_escape to get rid of linebreaks etc. Note that in the tests we do a byte comparison instead of converting the CSV dumps to strings because python will escape the strings on conversion... --- plugins/bkpr/bookkeeper.c | 5 ++++- plugins/bkpr/incomestmt.c | 26 +++++++++++++++++++++++--- tests/test_bookkeeper.py | 20 ++++++++++++++++++-- 3 files changed, 45 insertions(+), 6 deletions(-) diff --git a/plugins/bkpr/bookkeeper.c b/plugins/bkpr/bookkeeper.c index 5321fd87e..95a5fe8da 100644 --- a/plugins/bkpr/bookkeeper.c +++ b/plugins/bkpr/bookkeeper.c @@ -1,6 +1,7 @@ #include "config.h" #include #include +#include #include #include #include @@ -1178,7 +1179,9 @@ listinvoice_done(struct command *cmd, const char *buf, if (desc) { db_begin_transaction(db); - add_payment_hash_desc(db, payment_hash, desc); + add_payment_hash_desc(db, payment_hash, + json_escape_unescape(cmd, + (struct json_escape *)desc)); db_commit_transaction(db); } else plugin_log(cmd->plugin, LOG_DBG, diff --git a/plugins/bkpr/incomestmt.c b/plugins/bkpr/incomestmt.c index 49d0121c3..f20880249 100644 --- a/plugins/bkpr/incomestmt.c +++ b/plugins/bkpr/incomestmt.c @@ -105,6 +105,26 @@ static struct income_event *onchainfee_to_income(const tal_t *ctx, return inc; } +/* CSVs don't like ',' in the middle. We short circuit this + * by wrapping the desc in double-quotes ("). But what if + * there's already double-quotes? Well we swap these to + * single-quotes (') and then use the json_escape function */ +static char *csv_safe_str(const tal_t *ctx, char *input TAKES) +{ + struct json_escape *esc; + char *dupe; + + /* Update the double-quotes in place */ + dupe = tal_strdup(ctx, input); + for (size_t i = 0; dupe[i] != '\0'; i++) { + if (dupe[i] == '"') + dupe[i] = '\''; + } + + esc = json_escape(ctx, dupe); + return tal_fmt(ctx, "\"%s\"", esc->s); +} + static struct income_event *maybe_chain_income(const tal_t *ctx, struct db *db, struct account *acct, @@ -589,7 +609,7 @@ static void koinly_entry(const tal_t *ctx, FILE *csvf, struct income_event *ev) /* Description */ if (ev->desc) - fprintf(csvf, "%s", ev->desc); + fprintf(csvf, "%s", csv_safe_str(ev, ev->desc)); fprintf(csvf, ","); /* TxHash */ @@ -733,7 +753,7 @@ static void harmony_entry(const tal_t *ctx, FILE *csvf, struct income_event *ev) fprintf(csvf, ","); /* ",Note" description (may be NULL) */ - fprintf(csvf, "%s", ev->desc ? ev->desc : ""); + fprintf(csvf, "%s", ev->desc ? csv_safe_str(ev, ev->desc) : ""); } static void quickbooks_header(FILE *csvf) @@ -766,7 +786,7 @@ static void quickbooks_entry(const tal_t *ctx, FILE *csvf, struct income_event * /* Description */ fprintf(csvf, "%s (%s) %s: %s", ev->tag, ev->acct_name, ev->currency, - ev->desc ? ev->desc : "no desc"); + ev->desc ? csv_safe_str(ev, ev->desc) : "no desc"); fprintf(csvf, ","); /* Credit */ diff --git a/tests/test_bookkeeper.py b/tests/test_bookkeeper.py index 1cd013d8f..0f10272fe 100644 --- a/tests/test_bookkeeper.py +++ b/tests/test_bookkeeper.py @@ -386,7 +386,7 @@ def test_bookkeeping_descriptions(node_factory, bitcoind, chainparams): l1, l2 = node_factory.line_graph(2, opts={'experimental-offers': None}) # Send l2 funds via the channel - bolt11_desc = "test bolt11 description" + bolt11_desc = 'test "bolt11" description, 🥰🪢' l1.pay(l2, 11000000, label=bolt11_desc) l1.daemon.wait_for_log('coin_move .* [(]invoice[)] 0msat -11000000msat') l2.daemon.wait_for_log('coin_move .* [(]invoice[)] 11000000msat') @@ -402,7 +402,7 @@ def test_bookkeeping_descriptions(node_factory, bitcoind, chainparams): assert inv['description'] == bolt11_desc # Make an offer (l1) - bolt12_desc = "test bolt12 description" + bolt12_desc = 'test "bolt12" description, 🥰🪢' offer = l1.rpc.call('offer', [100, bolt12_desc]) invoice = l2.rpc.call('fetchinvoice', {'offer': offer['bolt12']}) paid = l2.rpc.pay(invoice['invoice']) @@ -418,3 +418,19 @@ def test_bookkeeping_descriptions(node_factory, bitcoind, chainparams): l2_inc_ev = l2.rpc.bkpr_listincome()['income_events'] inv = only_one([ev for ev in l2_inc_ev if 'payment_id' in ev and ev['payment_id'] == paid['payment_hash'] and ev['tag'] == 'invoice']) assert inv['description'] == bolt12_desc + + # Check the CSVs look groovy + l1.rpc.bkpr_dumpincomecsv('koinly', 'koinly.csv') + l2.rpc.bkpr_dumpincomecsv('koinly', 'koinly.csv') + koinly_path = os.path.join(l1.daemon.lightning_dir, TEST_NETWORK, 'koinly.csv') + l1_koinly_csv = open(koinly_path, 'rb').read() + bolt11_exp = bytes('invoice,"test \'bolt11\' description, 🥰🪢",', 'utf-8') + bolt12_exp = bytes('invoice,"test \'bolt12\' description, 🥰🪢",', 'utf-8') + + assert l1_koinly_csv.find(bolt11_exp) >= 0 + assert l1_koinly_csv.find(bolt12_exp) >= 0 + + koinly_path = os.path.join(l2.daemon.lightning_dir, TEST_NETWORK, 'koinly.csv') + l2_koinly_csv = open(koinly_path, 'rb').read() + assert l2_koinly_csv.find(bolt11_exp) >= 0 + assert l2_koinly_csv.find(bolt12_exp) >= 0