fix(search_family): Fix crash when no SEPARATOR is specified in the F…

…T.CREATE command (#4205) fix(search_family): fix(search_family): Fix crash when no SEPARATOR is specified in the FT.CREATE command Signed-off-by: Stepan Bagritsevich <[email protected]>
dragonflydb · Nov 27, 2024 · bd143e4 · bd143e4
1 parent 1ceca47
commit bd143e4
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 3 deletions.
diff --git a/src/server/search/search_family.cc b/src/server/search/search_family.cc
@@ -72,11 +72,18 @@ search::SchemaField::VectorParams ParseVectorParams(CmdArgParser* parser) {
   return params;
 }
 
-search::SchemaField::TagParams ParseTagParams(CmdArgParser* parser) {
+std::optional<search::SchemaField::TagParams> ParseTagParams(CmdArgParser* parser,
+                                                             SinkReplyBuilder* builder) {
   search::SchemaField::TagParams params{};
   while (parser->HasNext()) {
     if (parser->Check("SEPARATOR")) {
-      string_view separator = parser->Next();
+      std::string_view separator = parser->NextOrDefault();
+      if (separator.size() != 1) {
+        builder->SendError(
+            absl::StrCat("Tag separator must be a single character. Got `", separator, "`"),
+            kSyntaxErrType);
+        return std::nullopt;
+      }
       params.separator = separator.front();
       continue;
     }
@@ -127,7 +134,11 @@ optional<search::Schema> ParseSchemaOrReply(DocIndex::DataType type, CmdArgParse
     // Vector fields include: {algorithm} num_args args...
     search::SchemaField::ParamsVariant params(monostate{});
     if (type == search::SchemaField::TAG) {
-      params = ParseTagParams(&parser);
+      auto tag_params = ParseTagParams(&parser, builder);
+      if (!tag_params) {
+        return std::nullopt;
+      }
+      params = tag_params.value();
     } else if (type == search::SchemaField::VECTOR) {
       auto vector_params = ParseVectorParams(&parser);
       if (parser.HasError()) {

diff --git a/src/server/search/search_family_test.cc b/src/server/search/search_family_test.cc
@@ -332,6 +332,11 @@ TEST_F(SearchFamilyTest, Errors) {
 
   // Wrong field type
   EXPECT_THAT(Run({"ft.search", "i1", "@foo:lol"}), ErrArg("Wrong access type for field: foo"));
+
+  // ft.create index on json schema $.sometag AS sometag TAG SEPARATOR
+  EXPECT_THAT(Run({"ft.create", "i2", "ON", "JSON", "SCHEMA", "$.sometag", "AS", "sometag", "TAG",
+                   "SEPARATOR"}),
+              ErrArg("Tag separator must be a single character. Got ``"));
 }
 
 TEST_F(SearchFamilyTest, NoPrefix) {