From 0086fd011aaa1e001f7bd2240389b7de84f1393d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 7 Jul 2026 18:50:49 +0000 Subject: [PATCH] Escape %/_ wildcards in Like and emit ESCAPE clause (#24) Like::GetSqlValue wrapped the bound value in %...% but never escaped %, _, or \ occurring inside the caller's value itself, and the generated LIKE clause had no ESCAPE clause. SQLite's LIKE has no default escape character, so a value like "50%" matched far more than the literal text - not injection (the value is still bound as a parameter), but wrong match semantics. Escape \, % and _ (backslash first, single pass, so an inserted escape backslash is never re-escaped) in Like::GetSqlValue before wrapping in the outer "contains" percents, and emit " ESCAPE '\\'" from QueryPredicate::Evaluate() whenever symbol_ == "LIKE". The ESCAPE clause has to live in QueryPredicate::Evaluate() rather than a Like-only override: QueryPredicate::Clone() returns a base QueryPredicate (not a Like), and BinaryPredicate (And/Or) stores Clone()d operands, so a Like combined via And()/Or() is downgraded to a base QueryPredicate before Evaluate() runs. Since symbol_ and the already-escaped value_ both survive Clone(), keying the ESCAPE clause off symbol_ keeps compound predicates correct too - confirmed with a dedicated Clone-survival test. Tests: literal %, literal _, literal backslash, a Like combined via And() still matching literally (locks in the Clone-survival fix), and a regression test that a plain Like with no special characters still behaves as a substring/contains match. Updated the pre-existing LikePayloadStaysInBindings test's expected SQL/binding strings to reflect the now-correct escaped output. Confirmed 6 of 8 new/updated Like assertions fail on the pre-fix code and all pass after. Full suite: 72/72 tests pass. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01NUt3c1wdseRtRSSXfg3MCK --- src/query_predicates.cc | 37 ++++++++++++-- tests/database_test.cc | 88 ++++++++++++++++++++++++++++++++++ tests/query_predicates_test.cc | 33 ++++++++++++- 3 files changed, 151 insertions(+), 7 deletions(-) diff --git a/src/query_predicates.cc b/src/query_predicates.cc index 5b6e1f8..3ab46f9 100644 --- a/src/query_predicates.cc +++ b/src/query_predicates.cc @@ -29,6 +29,24 @@ using namespace sqlite_reflection; constexpr char space[] = " "; constexpr char percent[] = "%"; +namespace { +/// Escapes backslash, % and _ in a LIKE value so it is matched literally, rather than being +/// interpreted as SQLite LIKE wildcards. Must be paired with an ESCAPE '\' clause in the +/// generated SQL (see QueryPredicate::Evaluate). Backslash is escaped as it is encountered, +/// so a backslash inserted to escape % or _ is never itself re-escaped. +std::string EscapeLikeWildcards(const std::string& value) { + std::string escaped; + escaped.reserve(value.size()); + for (const char c : value) { + if (c == '\\' || c == '%' || c == '_') { + escaped += '\\'; + } + escaped += c; + } + return escaped; +} +} // namespace + SqlValue::SqlValue() : storage_class(SqliteStorageClass::kText), int_value(0), bool_value(false), real_value(0.0) {} QueryPredicateBase* QueryPredicate::Clone() const { @@ -56,7 +74,15 @@ OrPredicate QueryPredicateBase::Or(const QueryPredicateBase& other) const { } std::string QueryPredicate::Evaluate() const { - return member_name_ + space + symbol_ + space + "?"; + auto evaluation = member_name_ + space + symbol_ + space + "?"; + if (symbol_ == "LIKE") { + // The escape character used by EscapeLikeWildcards on the bound value must be + // declared here to take effect; symbol_ and the already-escaped value_ both survive + // Clone() (unlike a Like-only Evaluate() override), so this stays correct for a Like + // combined via And()/Or() too + evaluation += " ESCAPE '\\'"; + } + return evaluation; } std::vector QueryPredicate::Bindings() const { @@ -99,19 +125,20 @@ SqlValue Like::GetSqlValue(void* v, SqliteStorageClass storage_class) const { switch (storage_class) { case SqliteStorageClass::kInt: value.storage_class = SqliteStorageClass::kText; - value.text_value = percent + StringUtilities::FromInt(value.int_value) + percent; + value.text_value = percent + EscapeLikeWildcards(StringUtilities::FromInt(value.int_value)) + percent; return value; case SqliteStorageClass::kBool: value.storage_class = SqliteStorageClass::kText; - value.text_value = percent + StringUtilities::FromInt(value.bool_value ? 1 : 0) + percent; + value.text_value = + percent + EscapeLikeWildcards(StringUtilities::FromInt(value.bool_value ? 1 : 0)) + percent; return value; case SqliteStorageClass::kReal: value.storage_class = SqliteStorageClass::kText; - value.text_value = percent + StringUtilities::FromDouble(value.real_value) + percent; + value.text_value = percent + EscapeLikeWildcards(StringUtilities::FromDouble(value.real_value)) + percent; return value; case SqliteStorageClass::kText: case SqliteStorageClass::kDateTime: - value.text_value = percent + value.text_value + percent; + value.text_value = percent + EscapeLikeWildcards(value.text_value) + percent; return value; default: throw std::domain_error("Blob cannot be compared against similarity"); diff --git a/tests/database_test.cc b/tests/database_test.cc index ecdbcbf..c4f3f80 100644 --- a/tests/database_test.cc +++ b/tests/database_test.cc @@ -571,6 +571,94 @@ TEST_F(DatabaseTest, FetchWithPredicateChaining) { EXPECT_EQ(37, fetched_persons[1].age); } +TEST_F(DatabaseTest, LikeMatchesPercentWildcardLiterally) { + const auto db = Database::Instance(); + + std::vector persons; + persons.push_back({L"50% off", L"doe", 30, false, 1}); + persons.push_back({L"5000 off", L"doe", 30, false, 2}); + db->Save(persons); + + // A literal '%' in the search value must not act as a SQLite LIKE wildcard + const auto fetch_condition = Like(&Person::first_name, L"50%"); + const auto fetched = db->Fetch(&fetch_condition); + + ASSERT_EQ(1, fetched.size()); + EXPECT_EQ(1, fetched[0].id); + EXPECT_EQ(L"50% off", fetched[0].first_name); +} + +TEST_F(DatabaseTest, LikeMatchesUnderscoreWildcardLiterally) { + const auto db = Database::Instance(); + + std::vector persons; + persons.push_back({L"a_b", L"doe", 30, false, 1}); + persons.push_back({L"axb", L"doe", 30, false, 2}); + db->Save(persons); + + // A literal '_' in the search value must not act as a SQLite LIKE any-single-char wildcard + const auto fetch_condition = Like(&Person::first_name, L"a_b"); + const auto fetched = db->Fetch(&fetch_condition); + + ASSERT_EQ(1, fetched.size()); + EXPECT_EQ(1, fetched[0].id); + EXPECT_EQ(L"a_b", fetched[0].first_name); +} + +TEST_F(DatabaseTest, LikeMatchesBackslashLiterally) { + const auto db = Database::Instance(); + + std::vector persons; + persons.push_back({L"a\\b", L"doe", 30, false, 1}); + persons.push_back({L"axb", L"doe", 30, false, 2}); + db->Save(persons); + + // A literal backslash in the search value must match literally, not be misinterpreted as + // (or interfere with) the ESCAPE character + const auto fetch_condition = Like(&Person::first_name, L"a\\b"); + const auto fetched = db->Fetch(&fetch_condition); + + ASSERT_EQ(1, fetched.size()); + EXPECT_EQ(1, fetched[0].id); + EXPECT_EQ(L"a\\b", fetched[0].first_name); +} + +TEST_F(DatabaseTest, LikeInsideAndCombinationStillMatchesWildcardsLiterally) { + const auto db = Database::Instance(); + + std::vector persons; + persons.push_back({L"50% off", L"doe", 30, false, 1}); + persons.push_back({L"5000 off", L"doe", 30, false, 2}); + persons.push_back({L"50% off", L"roe", 40, false, 3}); + db->Save(persons); + + // Combining Like via And() clones it into a base QueryPredicate (see + // QueryPredicate::Clone()); the ESCAPE clause and the already-escaped bound value must + // both survive that clone for the match to stay literal here + const auto fetch_condition = Like(&Person::first_name, L"50%").And(Equal(&Person::age, 30)); + const auto fetched = db->Fetch(&fetch_condition); + + ASSERT_EQ(1, fetched.size()); + EXPECT_EQ(1, fetched[0].id); +} + +TEST_F(DatabaseTest, LikeWithoutWildcardsStillMatchesSubstring) { + const auto db = Database::Instance(); + + std::vector persons; + persons.push_back({L"hello world", L"doe", 30, false, 1}); + persons.push_back({L"goodbye", L"doe", 30, false, 2}); + db->Save(persons); + + // Regression: a value with no %/_/\ must still behave as a plain substring/contains match + const auto fetch_condition = Like(&Person::first_name, L"hello"); + const auto fetched = db->Fetch(&fetch_condition); + + ASSERT_EQ(1, fetched.size()); + EXPECT_EQ(1, fetched[0].id); + EXPECT_EQ(L"hello world", fetched[0].first_name); +} + TEST_F(DatabaseTest, FetchPreservesInt64ValuesBeyondInt32Range) { const auto db = Database::Instance(); diff --git a/tests/query_predicates_test.cc b/tests/query_predicates_test.cc index af81f60..f3de274 100644 --- a/tests/query_predicates_test.cc +++ b/tests/query_predicates_test.cc @@ -174,7 +174,36 @@ TEST(QueryPredicatesTest, LikePayloadStaysInBindings) { const auto evalution = condition.Evaluate(); const auto bindings = condition.Bindings(); - EXPECT_EQ(0, strcmp(evalution.data(), "first_name LIKE ?")); + EXPECT_EQ(0, strcmp(evalution.data(), R"(first_name LIKE ? ESCAPE '\')")); ASSERT_EQ(1, bindings.size()); - EXPECT_EQ("%john%' OR 1=1 --%", bindings[0].text_value); + // The literal '%' in the payload is escaped, since it is caller-supplied text, not an + // intentional wildcard + EXPECT_EQ(R"(%john\%' OR 1=1 --%)", bindings[0].text_value); +} + +TEST(QueryPredicatesTest, LikeEscapesWildcardsAndEmitsEscapeClause) { + const Like condition(&Person::first_name, L"50%_a\\b"); + const auto evaluation = condition.Evaluate(); + const auto bindings = condition.Bindings(); + + EXPECT_EQ(0, strcmp(evaluation.data(), R"(first_name LIKE ? ESCAPE '\')")); + ASSERT_EQ(1, bindings.size()); + // %, _ and \ in the caller's value are each escaped with a backslash before the outer + // "contains" wildcards are added + EXPECT_EQ(R"(%50\%\_a\\b%)", bindings[0].text_value); +} + +TEST(QueryPredicatesTest, LikeInsideAndSurvivesCloneWithEscapeClause) { + // BinaryPredicate stores Clone()d operands, and QueryPredicate::Clone() returns a base + // QueryPredicate rather than a Like - the ESCAPE clause must therefore come from + // QueryPredicate::Evaluate() itself (keyed on symbol_ == "LIKE") to survive this, not from + // a Like-only Evaluate() override + const auto condition = Like(&Person::first_name, L"50%").And(Equal(&Person::age, 30)); + const auto evaluation = condition.Evaluate(); + const auto bindings = condition.Bindings(); + + EXPECT_EQ(0, strcmp(evaluation.data(), R"((first_name LIKE ? ESCAPE '\' AND age = ?))")); + ASSERT_EQ(2, bindings.size()); + EXPECT_EQ(R"(%50\%%)", bindings[0].text_value); + EXPECT_EQ(30, bindings[1].int_value); }