From bd143e4b81f693ecda9251c03f7d387452a4f910 Mon Sep 17 00:00:00 2001
From: Stepan Bagritsevich
 <43710058+BagritsevichStepan@users.noreply.github.com>
Date: Wed, 27 Nov 2024 16:30:36 +0400
Subject: [PATCH] fix(search_family): Fix crash when no SEPARATOR is specified
 in the FT.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 <stefan@dragonflydb.io>
---
 src/server/search/search_family.cc      | 17 ++++++++++++++---
 src/server/search/search_family_test.cc |  5 +++++
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/src/server/search/search_family.cc b/src/server/search/search_family.cc
index c7c12176a45c..f1151dc60a67 100644
--- 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
index 046b76976c49..9fa68bd66757 100644
--- 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) {