From: Brian Aker <brian@tangent.org>
Date: Wed, 23 Mar 2011 17:02:56 +0000 (-0700)
Subject: Fixes issue where if illegal options caused parsing to fail an incorrect
X-Git-Tag: 0.51~15^2~75^2~8
X-Git-Url: https://git.m6w6.name/?a=commitdiff_plain;h=ecf02b501d387ee5761eb31db6f760de60ac09a1;p=m6w6%2Flibmemcached

Fixes issue where if illegal options caused parsing to fail an incorrect
message would still be returned.
---

diff --git a/libmemcached/options.cc b/libmemcached/options.cc
index dde4ccbe..35e8b783 100644
--- a/libmemcached/options.cc
+++ b/libmemcached/options.cc
@@ -86,7 +86,7 @@ memcached_return_t libmemcached_check_configuration(const char *option_string, s
   memcached_return_t rc= memcached_parse_configuration(memc_ptr, option_string, length);
   if (rc != MEMCACHED_SUCCESS && error_buffer && error_buffer_size)
   {
-    strncpy(error_buffer, error_buffer_size, memcached_last_error_message(memc_ptr));
+    strncpy(error_buffer, memcached_last_error_message(memc_ptr), error_buffer_size);
   }
 
   if (rc== MEMCACHED_SUCCESS && memcached_behavior_get(memc_ptr, MEMCACHED_BEHAVIOR_LOAD_FROM_FILE))
@@ -96,7 +96,7 @@ memcached_return_t libmemcached_check_configuration(const char *option_string, s
 
     if (rc != MEMCACHED_SUCCESS && error_buffer && error_buffer_size)
     {
-      strncpy(error_buffer, error_buffer_size, memcached_last_error_message(memc_ptr));
+      strncpy(error_buffer, memcached_last_error_message(memc_ptr), error_buffer_size);
     }
   }
 
@@ -111,14 +111,12 @@ memcached_return_t memcached_parse_configuration(memcached_st *self, char const
   if (! self)
     return memcached_set_error(self, MEMCACHED_INVALID_ARGUMENTS, NULL);
 
-  Context context(option_string, length, self);
+  memcached_return_t rc;
+  Context context(option_string, length, self, rc);
 
-  bool success= libmemcached_parse(&context, context.scanner)  == 0;
+  libmemcached_parse(&context, context.scanner);
 
-  if (not success)
-    return MEMCACHED_PARSE_ERROR;
-
-  return MEMCACHED_SUCCESS;
+  return rc;
 }
 
 void memcached_set_configuration_file(memcached_st *self, const char *filename, size_t filename_length)
diff --git a/libmemcached/options/context.h b/libmemcached/options/context.h
index 562d3dc1..f7c6d592 100644
--- a/libmemcached/options/context.h
+++ b/libmemcached/options/context.h
@@ -42,16 +42,19 @@
 class Context
 {
 public:
-  Context(const char *option_string, size_t option_string_length, memcached_st *memc_arg) :
+  Context(const char *option_string, size_t option_string_length, memcached_st *memc_arg,
+          memcached_return_t &rc_arg) :
     scanner(NULL),
     begin(NULL),
     pos(0),
-    memc(NULL)
+    memc(NULL),
+    rc(rc_arg)
   {
     buf= option_string;
     length= option_string_length;
     memc= memc_arg;
     init_scanner();
+    rc= MEMCACHED_SUCCESS;
   }
 
   ~Context()
@@ -65,6 +68,7 @@ public:
   size_t pos;
   size_t length;
   memcached_st *memc;
+  memcached_return_t &rc;
 
 protected:
   void init_scanner();   
diff --git a/libmemcached/options/parser.yy b/libmemcached/options/parser.yy
index a9021d0b..abd832b5 100644
--- a/libmemcached/options/parser.yy
+++ b/libmemcached/options/parser.yy
@@ -26,7 +26,7 @@
 %defines "libmemcached/options/parser.h"
 %lex-param { yyscan_t *scanner }
 %name-prefix="libmemcached_"
-%parse-param { Context *parser }
+%parse-param { Context *context }
 %parse-param { yyscan_t *scanner }
 %locations
 %pure-parser
@@ -55,10 +55,11 @@ int libmemcached_lex(YYSTYPE* lvalp, YYLTYPE* llocp, void* scanner);
 inline void libmemcached_error(YYLTYPE *locp, Context *context, yyscan_t *scanner, const char *error)
 {
   memcached_string_t local_string;
-  std::cerr << " Error " << error << std::endl;
   local_string.size= strlen(context->begin);
   local_string.c_str= context->begin;
-  memcached_set_error(context->memc, MEMCACHED_PARSE_ERROR, &local_string);
+  if (context->rc == MEMCACHED_SUCCESS)
+    context->rc= MEMCACHED_PARSE_ERROR;
+  memcached_set_error(context->memc, context->rc, &local_string);
 }
 
 %}
@@ -161,15 +162,17 @@ statement:
 expression:
           SERVER '=' server
           { 
-            if (memcached_server_add_parsed(parser->memc, $3.c_str, $3.length, $3.port, 0) != MEMCACHED_SUCCESS)
+            if ((context->rc= memcached_server_add_parsed(context->memc, $3.c_str, $3.length, $3.port, 0)) != MEMCACHED_SUCCESS)
+            {
               YYERROR;
+            }
           }
         | SERVERS '=' server_list
           {
           }
         | CONFIGURE_FILE '=' string
           {
-            memcached_set_configuration_file(parser->memc, $3.c_str, $3.length);
+            memcached_set_configuration_file(context->memc, $3.c_str, $3.length);
           }
         | behaviors
         ;
@@ -177,67 +180,43 @@ expression:
 behaviors:
           PREFIX_KEY '=' string
           {
-            memcached_return_t rc;
-            if ((rc= memcached_callback_set(parser->memc, MEMCACHED_CALLBACK_PREFIX_KEY, std::string($3.c_str, $3.length).c_str())) != MEMCACHED_SUCCESS)
+            if ((context->rc= memcached_callback_set(context->memc, MEMCACHED_CALLBACK_PREFIX_KEY, std::string($3.c_str, $3.length).c_str())) != MEMCACHED_SUCCESS)
             {
-              std::stringstream ss;
-              ss << "--PREFIX-KEY" << $3;
-              memcached_set_error_string(parser->memc, rc, ss.str().c_str(), ss.str().length());
               YYERROR;
             }
           }
         | DISTRIBUTION '=' distribution
           {
-            memcached_return_t rc;
-            if ((rc= memcached_behavior_set(parser->memc, MEMCACHED_BEHAVIOR_DISTRIBUTION, $3)) != MEMCACHED_SUCCESS)
+            if ((context->rc= memcached_behavior_set(context->memc, MEMCACHED_BEHAVIOR_DISTRIBUTION, $3)) != MEMCACHED_SUCCESS)
             {
-              std::stringstream ss;
-              ss << "--DISTRIBUTION=" << libmemcached_string_distribution($3);
-              memcached_set_error_string(parser->memc, rc, ss.str().c_str(), ss.str().length());
               YYERROR;
             }
           }
         | HASH '=' hash
           {
-            memcached_return_t rc;
-            if ((rc= memcached_behavior_set(parser->memc, MEMCACHED_BEHAVIOR_HASH, $3)) != MEMCACHED_SUCCESS)
+            if ((context->rc= memcached_behavior_set(context->memc, MEMCACHED_BEHAVIOR_HASH, $3)) != MEMCACHED_SUCCESS)
             {
-              std::stringstream ss;
-              ss << "--HASH=" << libmemcached_string_hash($3);
-              memcached_set_error_string(parser->memc, rc, ss.str().c_str(), ss.str().length());
               YYERROR; 
             }
           }
         | KETAMA_HASH '=' hash
           {
-            memcached_return_t rc;
-            if ((rc= memcached_behavior_set(parser->memc, MEMCACHED_BEHAVIOR_KETAMA_HASH, $3)) != MEMCACHED_SUCCESS)
+            if ((context->rc= memcached_behavior_set(context->memc, MEMCACHED_BEHAVIOR_KETAMA_HASH, $3)) != MEMCACHED_SUCCESS)
             {
-              std::stringstream ss;
-              ss << "--KETAMA-HASH=" << libmemcached_string_hash($3);
-              memcached_set_error_string(parser->memc, rc, ss.str().c_str(), ss.str().length());
               YYERROR;
             }
           }
         | behavior_number '=' NUMBER
           {
-            memcached_return_t rc;
-            if ((rc= memcached_behavior_set(parser->memc, $1, $3)) != MEMCACHED_SUCCESS)
+            if ((context->rc= memcached_behavior_set(context->memc, $1, $3)) != MEMCACHED_SUCCESS)
             {
-              std::stringstream ss;
-              ss << "--" << libmemcached_string_behavior($1) << "=" << $3;
-              memcached_set_error_string(parser->memc, rc, ss.str().c_str(), ss.str().length());
               YYERROR;
             }
           }
         | behavior_boolean
           {
-            memcached_return_t rc;
-            if ((rc= memcached_behavior_set(parser->memc, $1, true)) != MEMCACHED_SUCCESS)
+            if ((context->rc= memcached_behavior_set(context->memc, $1, true)) != MEMCACHED_SUCCESS)
             {
-              std::stringstream ss;
-              ss << "--" << libmemcached_string_behavior($1);
-              memcached_set_error_string(parser->memc, rc, ss.str().c_str(), ss.str().length());
               YYERROR;
             }
           }
@@ -371,23 +350,15 @@ behavior_boolean:
 server_list:
            server
           {
-            memcached_return_t rc;
-            if ((rc= memcached_server_add_parsed(parser->memc, $1.c_str, $1.length, $1.port, 0)) != MEMCACHED_SUCCESS)
+            if ((context->rc= memcached_server_add_parsed(context->memc, $1.c_str, $1.length, $1.port, 0)) != MEMCACHED_SUCCESS)
             {
-              std::stringstream ss;
-              ss << "--SERVER=" << $1;
-              memcached_set_error_string(parser->memc, rc, ss.str().c_str(), ss.str().length());
               YYERROR;
             }
           }
         | server_list ',' server
           {
-            memcached_return_t rc;
-            if ((rc= memcached_server_add_parsed(parser->memc, $3.c_str, $3.length, $3.port, 0)) != MEMCACHED_SUCCESS)
+            if ((context->rc= memcached_server_add_parsed(context->memc, $3.c_str, $3.length, $3.port, 0)) != MEMCACHED_SUCCESS)
             {
-              std::stringstream ss;
-              ss << "--SERVERS=" << $3;
-              memcached_set_error_string(parser->memc, rc, ss.str().c_str(), ss.str().length());
               YYERROR;
             }
           }
diff --git a/libmemcached/options/scanner.l b/libmemcached/options/scanner.l
index dc6427b2..2a45455c 100644
--- a/libmemcached/options/scanner.l
+++ b/libmemcached/options/scanner.l
@@ -160,7 +160,6 @@ static void get_lex_chars(char* buffer, int& result, int max_size, Context *cont
 "--PREFIX_KEY"	       		        { yyextra->begin= yytext; return PREFIX_KEY; }
 
 "--"[[:alnum:]]*   {
-      std::cerr << "Started at " << yytext << std::endl;
       yyextra->begin= yytext;
       return UNKNOWN_OPTION;
     }
@@ -191,13 +190,13 @@ JENKINS			{ return JENKINS; }
       return HOSTNAME;
     }
 
-[[:digit:]]{1,3}"."[[:digit:]]{1,3}"."[[:digit:]]{1,3}"."[[:digit:]]{1,3}: { 
+(([[:digit:]]{1,3}"."){3}([[:digit:]]{1,3})): {
       yylval->string.c_str = yytext;
       yylval->string.length = yyleng;
       return IPADDRESS_WITH_PORT;
     }
 
-[[:digit:]]{1,3}"."[[:digit:]]{1,3}"."[[:digit:]]{1,3}"."[[:digit:]]{1,3}  { 
+(([[:digit:]]{1,3}"."){3}([[:digit:]]{1,3})) {
       yylval->string.c_str = yytext;
       yylval->string.length = yyleng;
       return IPADDRESS;
@@ -209,7 +208,7 @@ JENKINS			{ return JENKINS; }
       return STRING;
     }
 
-\".*\" { 
+(\".*\") {
       yylval->string.c_str = yytext;
       yylval->string.length = yyleng;
       return QUOTED_STRING;
diff --git a/tests/parser.cc b/tests/parser.cc
index 231a59b6..4e0e23ff 100644
--- a/tests/parser.cc
+++ b/tests/parser.cc
@@ -363,7 +363,7 @@ test_return_t memcached_create_with_options_with_filename(memcached_st *junk)
 
   memcached_st *memc_ptr;
   memc_ptr= memcached_create_with_options(STRING_WITH_LEN("--CONFIGURE-FILE=\"support/example.cnf\""));
-  test_true(memc_ptr);
+  test_true_got(memc_ptr, memcached_last_error_message(memc_ptr));
   memcached_free(memc_ptr);
 
   return TEST_SUCCESS;
@@ -373,15 +373,16 @@ test_return_t libmemcached_check_configuration_with_filename_test(memcached_st *
 {
   (void)junk;
   memcached_return_t rc;
+  char buffer[BUFSIZ];
 
-  rc= libmemcached_check_configuration(STRING_WITH_LEN("--CONFIGURE-FILE=\"support/example.cnf\""), NULL, 0);
-  test_true(rc == MEMCACHED_SUCCESS);
+  rc= libmemcached_check_configuration(STRING_WITH_LEN("--CONFIGURE-FILE=\"support/example.cnf\""), buffer, sizeof(buffer));
+  test_true_got(rc == MEMCACHED_SUCCESS, buffer);
 
-  rc= libmemcached_check_configuration(STRING_WITH_LEN("--CONFIGURE-FILE=support/example.cnf"), NULL, 0);
-  test_false(rc == MEMCACHED_SUCCESS);
+  rc= libmemcached_check_configuration(STRING_WITH_LEN("--CONFIGURE-FILE=support/example.cnf"), buffer, sizeof(buffer));
+  test_false_with(rc == MEMCACHED_SUCCESS, buffer);
 
-  rc= libmemcached_check_configuration(STRING_WITH_LEN("--CONFIGURE-FILE=\"bad-path/example.cnf\""), NULL, 0);
-  test_true_got(rc == MEMCACHED_ERRNO, memcached_strerror(NULL, rc));
+  rc= libmemcached_check_configuration(STRING_WITH_LEN("--CONFIGURE-FILE=\"bad-path/example.cnf\""), buffer, sizeof(buffer));
+  test_true_got(rc == MEMCACHED_ERRNO, buffer);
 
   return TEST_SUCCESS;
 }
@@ -391,12 +392,13 @@ test_return_t libmemcached_check_configuration_test(memcached_st *junk)
   (void)junk;
 
   memcached_return_t rc;
+  char buffer[BUFSIZ];
 
-  rc= libmemcached_check_configuration(STRING_WITH_LEN("--server=localhost"), NULL, 0);
-  test_true(rc == MEMCACHED_SUCCESS);
+  rc= libmemcached_check_configuration(STRING_WITH_LEN("--server=localhost"), buffer, sizeof(buffer));
+  test_true_got(rc == MEMCACHED_SUCCESS, buffer);
 
-  rc= libmemcached_check_configuration(STRING_WITH_LEN("--dude=localhost"), NULL, 0);
-  test_false(rc == MEMCACHED_SUCCESS);
+  rc= libmemcached_check_configuration(STRING_WITH_LEN("--dude=localhost"), buffer, sizeof(buffer));
+  test_false_with(rc == MEMCACHED_SUCCESS, buffer);
   test_true(rc == MEMCACHED_PARSE_ERROR);
 
   return TEST_SUCCESS;
@@ -408,16 +410,16 @@ test_return_t memcached_create_with_options_test(memcached_st *junk)
 
   memcached_st *memc_ptr;
   memc_ptr= memcached_create_with_options(STRING_WITH_LEN("--server=localhost"));
-  test_true(memc_ptr);
+  test_true_got(memc_ptr, memcached_last_error_message(memc_ptr));
   memcached_free(memc_ptr);
 
   memc_ptr= memcached_create_with_options(STRING_WITH_LEN("--dude=localhost"));
-  test_false(memc_ptr);
+  test_false_with(memc_ptr, memcached_last_error_message(memc_ptr));
 
   return TEST_SUCCESS;
 }
 
-#define RANDOM_STRINGS 10
+#define RANDOM_STRINGS 50
 test_return_t random_statement_build_test(memcached_st *junk)
 {
   (void)junk;
@@ -459,12 +461,14 @@ test_return_t random_statement_build_test(memcached_st *junk)
     random_options.resize(random_options.size() -1);
 
     memcached_return_t rc;
-    rc= libmemcached_check_configuration(random_options.c_str(), random_options.size(), NULL, 0);
-    if (rc != MEMCACHED_SUCCESS)
+    memcached_st *memc_ptr= memcached_create(NULL);
+    rc= memcached_parse_configuration(memc_ptr, random_options.c_str(), random_options.size());
+    if (rc == MEMCACHED_PARSE_ERROR)
     {
-      std::cerr << "Failed to parse: (" << random_options << ")" << std::endl;
-      std::cerr << "\t " << memcached_strerror(NULL, rc) << std::endl;
+      std::cerr << std::endl << "Failed to parse(" << memcached_strerror(NULL, rc) << "): " << random_options << std::endl;
+      memcached_error_print(memc_ptr);
     }
+    memcached_free(memc_ptr);
   }
 
   return TEST_SUCCESS;