mu-query-parser: handle corner case / complexity

Some improvements:

- Fix or/xor chains (use left-associativity).
- Fix quote handling
- Make parsing O(n) rather than quadratic; limit recursion depth

And update tests.
This commit is contained in:
Dirk-Jan C. Binnema
2026-07-16 16:30:05 +03:00
committed by Seth Ladygo
parent 809b9e4f4a
commit 500bf400c3
4 changed files with 168 additions and 141 deletions

View File

@ -33,13 +33,6 @@
using namespace Mu;
// Sexp extensions...
static Sexp&
prepend(Sexp& s, Sexp&& e)
{
s.list().insert(s.list().begin(), std::move(e));
return s;
}
static Option<Sexp&>
second(Sexp& s)
{
@ -71,8 +64,35 @@ looks_like_matcher(const Sexp& sexp)
}
struct ParseContext {
bool expand;
std::vector<std::string> warnings;
bool expand{};
size_t depth{}; /* current parenthesis-nesting depth */
};
/* parsing is best-effort; deeper nesting than this is ignored
* (this also caps the recursion depth) */
constexpr size_t MaxDepth{100};
/**
* A cursor over the flat token-list, so popping is O(1)
*/
struct TokenStream {
explicit TokenStream(Sexp& tokens): toks_{tokens.list()} {}
bool empty() const { return pos_ >= toks_.size(); }
Option<Sexp&> head() {
if (empty())
return Nothing;
else
return toks_[pos_];
}
bool head_symbolp(const Sexp::Symbol& sym) const {
return pos_ < toks_.size() && toks_[pos_].symbolp(sym);
}
void pop_front() { ++pos_; }
private:
Sexp::List& toks_;
size_t pos_{};
};
/**
@ -118,21 +138,12 @@ phrasify(const Field& field, const Sexp& val)
* matcher
*/
static Sexp query(Sexp& tokens, ParseContext& ctx);
static Sexp query(TokenStream& tokens, ParseContext& ctx);
static Sexp
matcher(Sexp& tokens, ParseContext& ctx)
finalize_matcher(Sexp&& val, ParseContext& ctx)
{
if (tokens.empty())
return {};
auto val{*tokens.head()};
tokens.pop_front();
/* special case: if we find some non-matcher type here, we need to second-guess the token */
if (!looks_like_matcher(val))
val = Sexp{placeholder_sym, val.symbol().name};
const auto fieldsym{val.front().symbol()};
// Note the _expand_ case is what we use when processing the query 'for real';
@ -145,12 +156,10 @@ matcher(Sexp& tokens, ParseContext& ctx)
if (ctx.expand) { /* should we expand meta-fields? */
auto fields = fields_from_name(fieldsym == placeholder_sym ? "" : fieldsym.name);
if (!fields.empty()) {
if (!fields.empty() && second(val)) {
Sexp vals{};
vals.add(or_sym);
for (auto&& field: fields) {
if (!second(val))
continue;
if (auto&& phrase{phrasify(field, *second(val))}; phrase)
vals.add(std::move(*phrase));
else
@ -163,23 +172,49 @@ matcher(Sexp& tokens, ParseContext& ctx)
}
if (auto&& field{field_from_name(fieldsym.name)}; field) {
if (auto&& phrase(phrasify(*field, *second(val))); phrase)
val = std::move(*phrase);
if (auto&& v{second(val)}; v)
if (auto&& phrase{phrasify(*field, *v)}; phrase)
val = std::move(*phrase);
}
return val;
return std::move(val);
}
static Sexp
unit(Sexp& tokens, ParseContext& ctx)
matcher(TokenStream& tokens, ParseContext& ctx)
{
if (tokens.empty())
return {};
auto val{*tokens.head()};
tokens.pop_front();
/* special case: if we find some non-matcher type here, we need to second-guess the token */
if (!looks_like_matcher(val))
val = Sexp{placeholder_sym, val.symbol().name};
return finalize_matcher(std::move(val), ctx);
}
static Sexp
unit(TokenStream& tokens, ParseContext& ctx)
{
if (tokens.head_symbolp(not_sym)) { /* NOT */
tokens.pop_front();
/* handle (chains of) NOTs iteratively; parity decides */
bool neg{};
while (tokens.head_symbolp(not_sym)) {
tokens.pop_front();
neg = !neg;
}
Sexp sub{unit(tokens, ctx)};
/* special case: interpret "not" as a matcher instead; */
if (sub.empty())
return matcher(prepend(tokens, Sexp{placeholder_sym, not_sym.name}), ctx);
/* special case: interpret a trailing "not" as a matcher instead */
if (sub.empty()) {
sub = finalize_matcher(Sexp{placeholder_sym, not_sym.name}, ctx);
neg = !neg;
}
if (!neg)
return sub;
/* we try to optimize: double negations are removed */
if (sub.head_symbolp(not_sym))
@ -189,12 +224,13 @@ unit(Sexp& tokens, ParseContext& ctx)
} else if (tokens.head_symbolp(open_sym)) { /* ( sub) */
tokens.pop_front();
if (ctx.depth >= MaxDepth) /* nested too deeply; bail out */
return {};
++ctx.depth;
Sexp sub{query(tokens, ctx)};
--ctx.depth;
if (tokens.head_symbolp(close_sym))
tokens.pop_front();
else {
//g_warning("expected <)>");
}
return sub;
}
@ -204,7 +240,7 @@ unit(Sexp& tokens, ParseContext& ctx)
static Sexp
factor(Sexp& tokens, ParseContext& ctx)
factor(TokenStream& tokens, ParseContext& ctx)
{
Sexp un = unit(tokens, ctx);
@ -245,44 +281,30 @@ factor(Sexp& tokens, ParseContext& ctx)
}
static Sexp
query(Sexp& tokens, ParseContext& ctx)
query(TokenStream& tokens, ParseContext& ctx)
{
/* note: we flatten (or (or ( or ...)) etc. here;
* for optimization (since Xapian likes flat trees) */
/* process a left-associative chain of factors, separated by
* <OR>/<XOR>. Chains of the same operator are flattened, i.e.
* (or (or a b) c) => (or a b c), since Xapian likes flat trees */
Sexp fact = factor(tokens, ctx);
Sexp or_factors, xor_factors;
while (true) {
auto factors = std::invoke([&]()->Option<Sexp&> {
if (tokens.head_symbolp(or_sym))
return or_factors;
else if (tokens.head_symbolp(xor_sym))
return xor_factors;
else
return Nothing;
});
if (!factors)
const Sexp::Symbol* opsym{};
if (tokens.head_symbolp(or_sym))
opsym = &or_sym;
else if (tokens.head_symbolp(xor_sym))
opsym = &xor_sym;
else
break;
tokens.pop_front();
factors->add(factor(tokens, ctx));
}
Sexp rhs = factor(tokens, ctx);
if (rhs.empty())
break; /* trailing op; ignore */
// a bit clumsy...
if (!or_factors.empty() && xor_factors.empty()) {
fact = Sexp{or_sym, std::move(fact)};
fact.add_list(std::move(or_factors));
} else if (or_factors.empty() && !xor_factors.empty()) {
fact = Sexp{xor_sym, std::move(fact)};
fact.add_list(std::move(xor_factors));
} else if (!or_factors.empty() && !xor_factors.empty()) {
fact = Sexp{or_sym, std::move(fact)};
fact.add_list(std::move(or_factors));
prepend(xor_factors, xor_sym);
fact.add(std::move(xor_factors));
if (!fact.head_symbolp(*opsym))
fact = Sexp{*opsym, std::move(fact)};
fact.add(std::move(rhs));
}
return fact;
@ -294,10 +316,12 @@ Mu::parse_query(const std::string& expr, bool expand)
ParseContext context;
context.expand = expand;
if (auto&& items = process_query(expr); !items.listp())
auto items = process_query(expr);
if (!items.listp())
throw std::runtime_error("tokens must be a list-sexp");
else
return query(items, context);
TokenStream tokens{items};
return query(tokens, context);
}
@ -352,10 +376,15 @@ test_parser_basic()
TestCase{R"(a and b and c)", R"((and (_ "a") (_ "b") (_ "c")))"},
// a or b
TestCase{R"(a or b)", R"((or (_ "a") (_ "b")))"},
// or-chains are flattened
TestCase{R"(a or b or c)", R"((or (_ "a") (_ "b") (_ "c")))"},
// a or b and c
TestCase{R"(a or b and c)", R"((or (_ "a") (and (_ "b") (_ "c"))))"},
// a and b or c
TestCase{R"(a and b or c)", R"((or (and (_ "a") (_ "b")) (_ "c")))"},
// mixed or/xor associate to the left
TestCase{R"(a or b xor c)", R"((xor (or (_ "a") (_ "b")) (_ "c")))"},
TestCase{R"(a xor b or c)", R"((or (xor (_ "a") (_ "b")) (_ "c")))"},
// not a
TestCase{R"(not a)", R"((not (_ "a")))"},
// lone not
@ -387,6 +416,10 @@ test_parser_recover()
TestCase{R"(a and ()", R"((_ "a"))"},
// missing end )
TestCase{R"(a and (b)", R"((and (_ "a") (_ "b")))"},
// trailing operator is dropped
TestCase{R"(a or)", R"((_ "a"))"},
// quoted operators are matchers, not operators
TestCase{R"(foo "and" bar)", R"((and (_ "foo") (_ "and") (_ "bar")))"},
};
for (auto&& test: cases) {
@ -395,6 +428,24 @@ test_parser_recover()
}
}
static void
test_parser_pathological()
{
// pathological queries parse (possibly partially) without
// crashes or quadratic slow-down.
std::string parens(10000, '(');
parens += "a";
parens.append(10000, ')');
g_assert_true(parse_query(parens).listp());
std::string nots;
for (auto i = 0; i != 10000; ++i)
nots += "not ";
nots += "a"; // even number of nots
assert_equal(parse_query(nots).to_string(), R"((_ "a"))");
}
static void
test_parser_fields()
@ -475,6 +526,7 @@ main(int argc, char* argv[])
g_test_add_func("/query-parser/basic", test_parser_basic);
g_test_add_func("/query-parser/recover", test_parser_recover);
g_test_add_func("/query-parser/pathological", test_parser_pathological);
g_test_add_func("/query-parser/fields", test_parser_fields);
g_test_add_func("/query-parser/range", test_parser_range);
g_test_add_func("/query-parser/expand", test_parser_expand);

View File

@ -1,5 +1,5 @@
/*
** Copyright (C) 2023-2024 Dirk-Jan C. Binnema <djcb@djcbsoftware.nl>
** Copyright (C) 2023-2026 Dirk-Jan C. Binnema <djcb@djcbsoftware.nl>
**
** This program is free software; you can redistribute it and/or modify it
** under the terms of the GNU General Public License as published by the
@ -16,6 +16,9 @@
** Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
**
*/
#ifndef MU_QUERY_PARSER_HH__
#define MU_QUERY_PARSER_HH__
#include <string>
#include "mu-xapian-db.hh"
@ -41,8 +44,6 @@ inline const auto and_sym = "and"_sym;
inline const auto or_sym = "or"_sym;
inline const auto xor_sym = "xor"_sym;
inline const auto not_sym = "not"_sym;
inline const auto and_not_sym = "and-not"_sym;
/*
* We take a query, then parse it into a human-readable s-expression and then
@ -113,3 +114,5 @@ Result<Xapian::Query> make_xapian_query(const Store& store, const std::string& e
MU_ENABLE_BITOPS(ParserFlags);
} // namespace Mu
#endif /*MU_QUERY_PARSER_HH__*/

View File

@ -54,7 +54,7 @@ using namespace Mu;
*/
struct Element {
enum struct Bracket { Open, Close} ;
enum struct Op { And, Or, Xor, Not, AndNot };
enum struct Op { And, Or, Xor, Not };
template<typename ValueType>
struct FieldValue {
@ -95,7 +95,7 @@ struct Element {
Element(Bracket b): value{b} {}
Element(Op op): value{op} {}
Element(const std::string& val): value{val} {}
Element(const std::string& val, bool q=false): value{val}, quoted{q} {}
template<typename T>
Option<T&> get_opt() {
@ -133,8 +133,6 @@ struct Element {
return xor_sym;
case Op::Not:
return not_sym;
case Op::AndNot:
return and_not_sym;
default:
throw std::logic_error("invalid op type");
}
@ -155,6 +153,8 @@ struct Element {
}
ValueType value;
bool quoted{}; /**< the value was (at least partially) quoted;
* quoted values are not promoted to Ops */
};
using Elements = std::vector<Element>;
@ -162,60 +162,24 @@ using Elements = std::vector<Element>;
/**
* Remove first character from string and return it.
* Get the next element from the string, advancing pos
*
* @param[in,out] str a string
* @param[in,out] pos position in _original_ string
* @param str the query string
* @param[in,out] pos position where to start scanning
*
* @return a char or 0 if there is none.
*/
static char
read_char(std::string& str, size_t& pos)
{
if (str.empty())
return {};
auto kar{str.at(0)};
str.erase(0, 1);
++pos;
return kar;
}
/**
* Restore kar at the beginning of the string
*
* @param[in,out] str a string
* @param[in,out] pos position in _original_ string
* @param kar a character
*/
static void
unread_char(std::string& str, size_t& pos, char kar)
{
str = kar + str;
--pos;
}
/**
* Remove the next element from the string and return it
*
* @param[in,out] str a string
* @param[in,out] pos position in _original_ string *
*
* @return an Element or Nothing
* @return an Element or Nothing when the input is exhausted
*/
static Option<Element>
next_element(std::string& str, size_t& pos)
next_element(std::string_view str, size_t& pos)
{
bool quoted{}, escaped{};
bool quoted{}, escaped{}, had_quote{};
std::string value{};
auto is_separator = [](char c) { return c == ' '|| c == '(' || c == ')'; };
while (!str.empty()) {
while (pos != str.size()) {
auto kar = read_char(str, pos);
auto kar = str[pos++];
if (kar == '\\') {
escaped = !escaped;
@ -224,8 +188,9 @@ next_element(std::string& str, size_t& pos)
}
if (kar == '"' && !escaped) {
if (!escaped && quoted)
return Element{value};
had_quote = true;
if (quoted)
return Element{value, had_quote};
else {
quoted = true;
continue;
@ -234,20 +199,17 @@ next_element(std::string& str, size_t& pos)
if (!quoted && !escaped && is_separator(kar)) {
if (!value.empty()) {
unread_char(str, pos, kar);
return Element{value};
--pos; // leave the separator for next time
return Element{value, had_quote};
}
if (quoted || kar == ' ')
continue;
switch (kar) {
case '(':
return Element{Element::Bracket::Open};
case ')':
return Element{Element::Bracket::Close};
default:
break;
default: // some space
continue;
}
}
@ -258,7 +220,7 @@ next_element(std::string& str, size_t& pos)
if (value.empty())
return Nothing;
else
return Element{value};
return Element{value, had_quote};
}
@ -266,7 +228,7 @@ static Option<Element>
opify(Element&& element)
{
auto&& str{element.get_opt<std::string>()};
if (!str)
if (!str || element.quoted) // quoted values are not operators
return element;
static const std::unordered_map<std::string, Element::Op> ops = {
@ -274,7 +236,6 @@ opify(Element&& element)
{ "or", Element::Op::Or},
{ "xor", Element::Op::Xor },
{ "not", Element::Op::Not },
// AndNot only appears during parsing.
};
if (auto&& it = ops.find(utf8_flatten(*str)); it != ops.end())
@ -296,7 +257,9 @@ basify(Element&& element)
return element;
}
const auto fname{str->substr(0, pos)};
auto fname{str->substr(0, pos)};
for (auto& c: fname) // field names are case-insensitive
c = to_ascii_lower(c);
if (auto&& field{field_from_name(fname)}; field) {
auto val{str->substr(pos + 1)};
if (field == Field::Id::Flags) {
@ -422,15 +385,15 @@ static Elements
process(const std::string& expr)
{
Elements elements{};
size_t offset{0};
size_t pos{0};
/* all control chars become SPC */
std::string str{expr};
for (auto& c: str)
c = is_ascii_cntrl(c) ? ' ' : c;
while(!str.empty()) {
auto&& element = next_element(str, offset)
while (pos != str.size()) {
auto&& element = next_element(str, pos)
.and_then(opify)
.and_then(basify)
.and_then(regexpify)
@ -495,7 +458,11 @@ test_processor()
// basics
TestCase{R"(hello world)", R"(((_ "hello") (_ "world")))"},
TestCase{R"(maildir:/"hello world")", R"(((maildir "/hello world")))"},
TestCase{R"(flag:deleted)", R"(((_ "flag:deleted")))"} // non-existing flags
TestCase{R"(flag:deleted)", R"(((_ "flag:deleted")))"}, // non-existing flags
// quoted operators are not operators
TestCase{R"(foo "and" bar)", R"(((_ "foo") (_ "and") (_ "bar")))"},
// field-names are case-insensitive
TestCase{R"(Subject:foo)", R"(((subject "foo")))"}
};
for (auto&& test: cases) {

View File

@ -1,5 +1,5 @@
/*
** Copyright (C) 2023 Dirk-Jan C. Binnema <djcb@djcbsoftware.nl>
** Copyright (C) 2023-2026 Dirk-Jan C. Binnema <djcb@djcbsoftware.nl>
**
** This program is free software; you can redistribute it and/or modify it
** under the terms of the GNU General Public License as published by the
@ -152,6 +152,8 @@ range(const Field& field, Sexp&& s)
// iso -> time_t
r0 = iso_to_lexnum(*r0);
r1 = iso_to_lexnum(*r1);
if (!r0 || !r1)
return Err(Error::Code::InvalidArgument, "invalid date range");
} else if (field == Field::Id::Size) {
if (!r0->empty())
r0 = to_lexnum(::atoll(r0->c_str()));
@ -213,7 +215,6 @@ parse_logop(const Store& store, Xapian::Query::op op, Sexp&& args, Mu::ParserFla
switch(op) {
case Xapian::Query::OP_AND_NOT:
// TODO: optimize AND_NOT
if (qs.size() != 1)
return Err(Error::Code::InvalidArgument,
"expected single argument for NOT");
@ -370,10 +371,14 @@ xapian_query_classic(const std::string& expr, Mu::ParserFlags flags)
Result<Xapian::Query>
Mu::make_xapian_query(const Store& store, const std::string& expr, Mu::ParserFlags flags) noexcept
{
if (any_of(flags & Mu::ParserFlags::XapianParser))
return xapian_query_classic(expr, flags);
// note: the try-wrapper, since Xapian may throw (e.g. when the
// database was modified while we're querying)
return xapian_try_result([&]()->Result<Xapian::Query> {
if (any_of(flags & Mu::ParserFlags::XapianParser))
return Ok(xapian_query_classic(expr, flags));
return parse(store, Mu::parse_query(expr, true/*expand*/), flags);
return parse(store, Mu::parse_query(expr, true/*expand*/), flags);
});
}