Skip to content

Commit 6a26e09

Browse files
Copilotmakr-code
andcommitted
fix(api): address code review – AQL injection escaping, test dedup, formatting
- Add aqlEscape() helper to sanitize user input before AQL string embedding - Apply aqlEscape() to collection + query/sparse_query in HybridSearch and FullTextSearch - Remove duplicate ThemisDBGrpcService construction in ConstructWithWiredComponents test - Remove extra trailing newline in test file Co-authored-by: makr-code <150588092+makr-code@users.noreply.github.com> Agent-Logs-Url: https://github.com/makr-code/ThemisDB/sessions/794e2fa9-9d77-4d33-b543-df42c5e31930
1 parent 97cd900 commit 6a26e09

2 files changed

Lines changed: 24 additions & 7 deletions

File tree

src/api/themisdb_grpc_service.cpp

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,23 @@
5656
# define THEMIS_HAS_JSON 0
5757
#endif
5858

59+
namespace {
60+
61+
/// Escape a string for safe embedding inside an AQL single-quoted literal.
62+
/// Replaces backslashes and single-quotes to prevent AQL injection.
63+
std::string aqlEscape(const std::string& raw) {
64+
std::string out;
65+
out.reserve(raw.size() + 4);
66+
for (char c : raw) {
67+
if (c == '\\') { out += "\\\\"; }
68+
else if (c == '\'') { out += "\\'"; }
69+
else { out += c; }
70+
}
71+
return out;
72+
}
73+
74+
} // namespace
75+
5976
namespace themis {
6077
namespace api {
6178

@@ -525,9 +542,10 @@ class ThemisDBGrpcService::Impl {
525542

526543
// Sparse component (AQL full-text via engine) – simplified delegation
527544
if (aql_engine_ && !req->sparse_query().empty()) {
545+
// Escape user input before embedding in AQL string literal
528546
const std::string aql =
529-
"FOR doc IN " + req->collection() +
530-
" FILTER FULLTEXT(doc, 'text', '" + req->sparse_query() + "') "
547+
"FOR doc IN " + aqlEscape(req->collection()) +
548+
" FILTER FULLTEXT(doc, 'text', '" + aqlEscape(req->sparse_query()) + "')"
531549
" LIMIT " + std::to_string(k) + " RETURN doc";
532550
auto result = aql_engine_->execute(aql);
533551
if (result) {
@@ -566,9 +584,10 @@ class ThemisDBGrpcService::Impl {
566584
}
567585

568586
const int limit = req->max_results() > 0 ? req->max_results() : 10;
587+
// Escape user input before embedding in AQL string literal
569588
const std::string aql =
570-
"FOR doc IN " + req->collection() +
571-
" FILTER FULLTEXT(doc, 'text', '" + req->query() + "') "
589+
"FOR doc IN " + aqlEscape(req->collection()) +
590+
" FILTER FULLTEXT(doc, 'text', '" + aqlEscape(req->query()) + "')"
572591
" LIMIT " + std::to_string(limit) + " RETURN doc";
573592

574593
auto result = aql_engine_->execute(aql);

tests/test_themisdb_grpc_service.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,9 +128,8 @@ TEST(ThemisDBGrpcServiceTest, ConstructWithWiredComponents) {
128128

129129
ASSERT_NO_THROW({
130130
ThemisDBGrpcService svc(nullptr, nullptr, engine, index);
131+
(void)svc.service();
131132
});
132-
ThemisDBGrpcService svc(nullptr, nullptr, engine, index);
133-
SUCCEED(); // construction succeeds; service() will return valid ptr if stubs present
134133
}
135134

136135
TEST(ThemisDBGrpcServiceTest, ServicePointerStableAfterWiring) {
@@ -199,4 +198,3 @@ TEST(ThemisDBGrpcServiceFactoryTest, FactoryIsReusable) {
199198
EXPECT_NE(svc1.get(), svc2.get());
200199
}
201200

202-

0 commit comments

Comments
 (0)