From ecf02b501d387ee5761eb31db6f760de60ac09a1 Mon Sep 17 00:00:00 2001 From: Brian Aker Date: Wed, 23 Mar 2011 10:02:56 -0700 Subject: [PATCH 1/1] Fixes issue where if illegal options caused parsing to fail an incorrect message would still be returned. --- libmemcached/options.cc | 14 ++++---- libmemcached/options/context.h | 8 +++-- libmemcached/options/parser.yy | 61 +++++++++------------------------- libmemcached/options/scanner.l | 7 ++-- tests/parser.cc | 40 ++++++++++++---------- 5 files changed, 53 insertions(+), 77 deletions(-) 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; -- 2.30.2