Fix LIKE operator to escape wildcards in user-supplied values

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<SqlValue> 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");

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<Person> 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<Person>(&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<Person> 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<Person>(&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<Person> 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<Person>(&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<Person> 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<Person>(&fetch_condition);
+
+    ASSERT_EQ(1, fetched.size());
+    EXPECT_EQ(1, fetched[0].id);
+}
+
+TEST_F(DatabaseTest, LikeWithoutWildcardsStillMatchesSubstring) {
+    const auto db = Database::Instance();
+
+    std::vector<Person> 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<Person>(&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();
 

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);
 }