From 1e32c64d1c2cc8cd6e26a489d47ceff350587669 Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Sun, 1 Sep 2024 17:25:35 +0100 Subject: [PATCH] Fix multi-method query-style lookup quoters Broken-by: ac8aeb5485a8 --- src/src/lookupapi.h | 1 + src/src/lookups/testdb.c | 49 ++++++++++++++++++++++++++++++++--- src/src/search.c | 7 +++-- src/src/store.c | 14 +++++----- src/src/store.h | 2 +- test/confs/3213 | 2 +- test/runtest | 1 - test/scripts/3200-testdb/3200 | 12 +++++++++ test/stderr/2610 | 12 ++++++--- test/stderr/2620 | 6 +++-- test/stderr/3200 | 1 + test/stdout/3200 | 11 +++++++- 12 files changed, 94 insertions(+), 24 deletions(-) diff --git a/src/src/lookupapi.h b/src/src/lookupapi.h index af7bd51f6..524a2fc1d 100644 --- a/src/src/lookupapi.h +++ b/src/src/lookupapi.h @@ -7,6 +7,7 @@ /* See the file NOTICE for conditions of use and distribution. */ /* SPDX-License-Identifier: GPL-2.0-or-later */ +#define LOOKUPAPI_H /* The "type" field in each item is a set of bit flags: diff --git a/src/src/lookups/testdb.c b/src/src/lookups/testdb.c index 8de7090b7..2319291d5 100644 --- a/src/src/lookups/testdb.c +++ b/src/src/lookups/testdb.c @@ -61,6 +61,17 @@ return OK; } +/* Don't actually alter the string; just copy to quoted-pool memory */ + +static uschar * +testdb_quote(uschar * s, uschar * opt, unsigned idx) +{ +int len = Ustrlen(s) + 1; +uschar * quoted = store_get_quoted(len, s, idx, US"testdb"); +memcpy(quoted, s, len); +return quoted; +} + /************************************************* * Version reporting entry point * *************************************************/ @@ -79,7 +90,7 @@ return g; } -static lookup_info _lookup_info = { +static lookup_info testdb_lookup_info = { .name = US"testdb", /* lookup name */ .type = lookup_querystyle, /* query-style lookup */ .open = testdb_open, /* open function */ @@ -87,7 +98,31 @@ static lookup_info _lookup_info = { .find = testdb_find, /* find function */ .close = NULL, /* no close function */ .tidy = NULL, /* no tidy function */ - .quote = NULL, /* no quoting function */ + .quote = testdb_quote, /* quoting function */ + .version_report = testdb_version_report /* version reporting */ +}; + +static lookup_info testdb2_lookup_info = { + .name = US"testdb2", /* lookup name */ + .type = lookup_querystyle, /* query-style lookup */ + .open = testdb_open, /* open function */ + .check = NULL, /* check function */ + .find = testdb_find, /* find function */ + .close = NULL, /* no close function */ + .tidy = NULL, /* no tidy function */ + .quote = testdb_quote, /* same quoting function */ + .version_report = testdb_version_report /* version reporting */ +}; + +static lookup_info testdb3_lookup_info = { + .name = US"testdb_nq", /* lookup name */ + .type = lookup_querystyle, /* query-style lookup */ + .open = testdb_open, /* open function */ + .check = NULL, /* check function */ + .find = testdb_find, /* find function */ + .close = NULL, /* no close function */ + .tidy = NULL, /* no tidy function */ + .quote = NULL, /* NO quoting function */ .version_report = testdb_version_report /* version reporting */ }; @@ -95,7 +130,13 @@ static lookup_info _lookup_info = { #define testdb_lookup_module_info _lookup_module_info #endif -static lookup_info *_lookup_list[] = { &_lookup_info }; -lookup_module_info testdb_lookup_module_info = { LOOKUP_MODULE_INFO_MAGIC, _lookup_list, 1 }; +static lookup_info *_lookup_list[] = { + &testdb_lookup_info, + &testdb2_lookup_info, + &testdb3_lookup_info +}; + +lookup_module_info testdb_lookup_module_info = { + LOOKUP_MODULE_INFO_MAGIC, _lookup_list, 3 }; /* End of lookups/testdb.c */ diff --git a/src/src/search.c b/src/src/search.c index ad55e8780..981c1a73d 100644 --- a/src/src/search.c +++ b/src/src/search.c @@ -536,7 +536,6 @@ search_cache * c = (search_cache *)(t->data.ptr); const lookup_info * li = c->li; expiring_data * e = NULL; /* compiler quietening */ uschar * data = NULL; -int required_quoter_id = li->acq_num; int old_pool = store_pool; /* Lookups that return DEFER may not always set an error message. So that @@ -603,7 +602,7 @@ else */ if ( !filename && li->quote - && is_tainted(keystring) && !is_quoted_like(keystring, required_quoter_id)) + && is_tainted(keystring) && !is_quoted_like(keystring, li)) { const uschar * ks = keystring; uschar * loc = acl_current_verb(); @@ -638,8 +637,8 @@ else const uschar * quoter_name; int q = quoter_for_address(ks, "er_name); - debug_printf_indent("required_quoter_id %d (%s) quoting %d (%s)\n", - required_quoter_id, li->name, + debug_printf_indent("required_quoter_id (%s) quoting %d (%s)\n", + li->name, q, quoter_name); } #endif diff --git a/src/src/store.c b/src/src/store.c index 4824b5c54..a609e5c61 100644 --- a/src/src/store.c +++ b/src/src/store.c @@ -659,20 +659,22 @@ found: There is extra complexity to handle lookup providers with multiple find variants but shared quote functions. */ BOOL -is_quoted_like(const void * p, unsigned quoter) +is_quoted_like(const void * p, const void * v_q_li) { -const uschar * p_name, * q_name = NULL; -const lookup_info * p_li, * q_li; +const uschar * p_name; +const lookup_info * p_li, * q_li = v_q_li; void * p_qfn, * q_qfn; (void) quoter_for_address(p, &p_name); -(void) pool_for_quoter(quoter, &q_name); -if (!p_name || !q_name) return FALSE; +if (!p_name) + { + DEBUG(D_any) debug_printf("No quoter name for addr\n"); + return FALSE; + } p_li = search_findtype(p_name, Ustrlen(p_name)); p_qfn = p_li ? p_li->quote : NULL; -q_li = search_findtype(q_name, Ustrlen(q_name)); q_qfn = q_li ? q_li->quote : NULL; BOOL y = p_qfn == q_qfn; diff --git a/src/src/store.h b/src/src/store.h index f088647f1..e54d45fc0 100644 --- a/src/src/store.h +++ b/src/src/store.h @@ -83,7 +83,7 @@ extern rmark store_reset_3(rmark, const char *, int); #define GET_TAINTED (const void *)1 extern int quoter_for_address(const void *, const uschar **); -extern BOOL is_quoted_like(const void *, unsigned); +extern BOOL is_quoted_like(const void *, const void *); extern BOOL is_real_quoter(int); extern void debug_print_taint(const void * p); diff --git a/test/confs/3213 b/test/confs/3213 index 5752f91ed..77c0f34ef 100644 --- a/test/confs/3213 +++ b/test/confs/3213 @@ -14,7 +14,7 @@ acl_smtp_mail = check_mail begin ACL check_mail: - accept senders = testdb;<$sender_address> + accept senders = testdb;<${quote_testdb:$sender_address}> # End diff --git a/test/runtest b/test/runtest index ae227810c..70499312d 100755 --- a/test/runtest +++ b/test/runtest @@ -1475,7 +1475,6 @@ RESET_AFTER_EXTRA_LINE_READ: } # Different builds will have different lookup types included - s/required_quoter_id \K\d+ \((\w+)\) quoting -1 \(NULL\)$/NN ($1) quoting -1 (NULL)/; # and different numbers of lookup types result in different type-code letters, # so convert them all to "0" s%(? match_ip: 12 true > match_ip: 13 -> \ No newline at end of file +> > variable m0 set +> variable m1 set +> variable m2 set +> variable m3 set +> missing quoter: expected_fail +> proper quoter: barbaz +> missing (but cached): barbaz +> shared quoter: foo_2 +> quoter notreq: foo_3 +> -- 2.30.2