From 3da9aa6b6792407d57816ec4a4cdfd0b4b3434b8 Mon Sep 17 00:00:00 2001 From: Neels Hofmeyr Date: Tue, 26 Sep 2017 14:21:44 +0200 Subject: ctrl: tighten CTRL input parsing Validate that incoming CTRL commands... - have decimal IDs, - return error on trailing characters, - have invalid characters in variable identifiers, - send detailed error messages as reply to the requestor. Adjust ctrl_test.{c,ok}, which best show the change in behavior. Message handling causes log messages on stderr; previously, stderr was empty. Add '[ignore]' in testsuite.at so that the nonempty stderr doesn't cause test failures. Change-Id: I96a9b6b6a3a5e0b80513aa9eaa727ae8c9c7d7a1 --- tests/ctrl/ctrl_test.c | 110 ++++++++++++++++++++---------------------------- tests/ctrl/ctrl_test.ok | 76 ++++++++++++--------------------- 2 files changed, 73 insertions(+), 113 deletions(-) (limited to 'tests/ctrl') diff --git a/tests/ctrl/ctrl_test.c b/tests/ctrl/ctrl_test.c index 39ec61a8..a38591f3 100644 --- a/tests/ctrl/ctrl_test.c +++ b/tests/ctrl/ctrl_test.c @@ -79,14 +79,19 @@ static void assert_test(struct ctrl_handle *ctrl, struct ctrl_connection *ccon, cmd = ctrl_cmd_parse2(ctx, msg); OSMO_ASSERT(cmd); - OSMO_ASSERT(t->expect_parsed.type == cmd->type); + if (t->expect_parsed.type != cmd->type) { + printf("type mismatch: got %s\n", get_value_string(ctrl_type_vals, cmd->type)); + OSMO_ASSERT(t->expect_parsed.type == cmd->type); + } #define ASSERT_SAME_STR(field) \ assert_same_str(#field, t->expect_parsed.field, cmd->field) ASSERT_SAME_STR(id); - ASSERT_SAME_STR(variable); - ASSERT_SAME_STR(value); + if (t->expect_parsed.type != CTRL_TYPE_ERROR) { + ASSERT_SAME_STR(variable); + ASSERT_SAME_STR(value); + } ASSERT_SAME_STR(reply); talloc_free(cmd); @@ -140,75 +145,66 @@ static const struct one_test test_messages_list[] = { { .type = CTRL_TYPE_GET, .id = "1", - .variable = "variable\n", /* current bug */ + .variable = "variable", }, "ERROR 1 Command not found", }, { "GET 1 var\ni\nable", { - .type = CTRL_TYPE_GET, + .type = CTRL_TYPE_ERROR, .id = "1", - .variable = "var\ni\nable", /* current bug */ + .reply = "GET with trailing characters", }, - "ERROR 1 Command not found", - + "ERROR 1 GET with trailing characters", }, { "GET 1 var\ti\table", { - .type = CTRL_TYPE_GET, + .type = CTRL_TYPE_ERROR, .id = "1", - .variable = "var\ti\table", /* current bug */ + .reply = "GET variable contains invalid characters", }, - "ERROR 1 Command not found", + "ERROR 1 GET variable contains invalid characters", }, { "GET 1 var\ri\rable", { - .type = CTRL_TYPE_GET, + .type = CTRL_TYPE_ERROR, .id = "1", - .variable = "var\ri\rable", /* current bug */ + .reply = "GET variable contains invalid characters", }, - "ERROR 1 Command not found", + "ERROR 1 GET variable contains invalid characters", }, { "GET 1 variable value", { - .type = CTRL_TYPE_GET, + .type = CTRL_TYPE_ERROR, .id = "1", - .variable = "variable", - .value = NULL, + .reply = "GET with trailing characters", }, - "ERROR 1 Command not found", - + "ERROR 1 GET with trailing characters", }, { "GET 1 variable value\n", { - .type = CTRL_TYPE_GET, + .type = CTRL_TYPE_ERROR, .id = "1", - .variable = "variable", - .value = NULL, + .reply = "GET with trailing characters", }, - "ERROR 1 Command not found", - + "ERROR 1 GET with trailing characters", }, { "GET 1 variable multiple value tokens", { - .type = CTRL_TYPE_GET, + .type = CTRL_TYPE_ERROR, .id = "1", - .variable = "variable", - .value = NULL, + .reply = "GET with trailing characters", }, - "ERROR 1 Command not found", - + "ERROR 1 GET with trailing characters", }, { "GET 1 variable multiple value tokens\n", { - .type = CTRL_TYPE_GET, + .type = CTRL_TYPE_ERROR, .id = "1", - .variable = "variable", - .value = NULL, + .reply = "GET with trailing characters", }, - "ERROR 1 Command not found", - + "ERROR 1 GET with trailing characters", }, { "SET 1 variable value", { @@ -218,7 +214,6 @@ static const struct one_test test_messages_list[] = { .value = "value", }, "ERROR 1 Command not found", - }, { "SET 1 variable value\n", { @@ -228,27 +223,22 @@ static const struct one_test test_messages_list[] = { .value = "value", }, "ERROR 1 Command not found", - }, { "SET weird_id variable value", { - .type = CTRL_TYPE_SET, - .id = "weird_id", - .variable = "variable", - .value = "value", + .type = CTRL_TYPE_ERROR, + .id = "err", + .reply = "Invalid message ID number", }, - "ERROR weird_id Command not found", - + "ERROR err Invalid message ID number", }, { "SET weird_id variable value\n", { - .type = CTRL_TYPE_SET, - .id = "weird_id", - .variable = "variable", - .value = "value", + .type = CTRL_TYPE_ERROR, + .id = "err", + .reply = "Invalid message ID number", }, - "ERROR weird_id Command not found", - + "ERROR err Invalid message ID number", }, { "SET 1 variable multiple value tokens", { @@ -278,7 +268,6 @@ static const struct one_test test_messages_list[] = { .value = "value_with_trailing_spaces ", }, "ERROR 1 Command not found", - }, { "SET 1 variable value_with_trailing_spaces \n", { @@ -288,27 +277,22 @@ static const struct one_test test_messages_list[] = { .value = "value_with_trailing_spaces ", }, "ERROR 1 Command not found", - }, { "SET \n special_char_id value", { - .type = CTRL_TYPE_SET, - .id = "\n", - .variable = "special_char_id", - .value = "value", + .type = CTRL_TYPE_ERROR, + .id = "err", + .reply = "Invalid message ID number", }, - "ERROR \n Command not found", - + "ERROR err Invalid message ID number", }, { "SET \t special_char_id value", { - .type = CTRL_TYPE_SET, - .id = "\t", - .variable = "special_char_id", - .value = "value", + .type = CTRL_TYPE_ERROR, + .id = "err", + .reply = "Invalid message ID number", }, - "ERROR \t Command not found", - + "ERROR err Invalid message ID number", }, { "GET_REPLY 1 variable OK", { @@ -317,7 +301,6 @@ static const struct one_test test_messages_list[] = { .variable = "variable", .reply = "OK", }, - .reply_str = NULL, }, { "SET_REPLY 1 variable OK", { @@ -326,7 +309,6 @@ static const struct one_test test_messages_list[] = { .variable = "variable", .reply = "OK", }, - .reply_str = NULL, }, }; diff --git a/tests/ctrl/ctrl_test.ok b/tests/ctrl/ctrl_test.ok index 4a3a1696..087ebbc2 100644 --- a/tests/ctrl/ctrl_test.ok +++ b/tests/ctrl/ctrl_test.ok @@ -19,7 +19,7 @@ ok test: 'GET 1 variable\n' parsing: id = '1' -variable = 'variable\n' +variable = 'variable' value = '(null)' reply = '(null)' handling: @@ -28,65 +28,51 @@ ok test: 'GET 1 var\ni\nable' parsing: id = '1' -variable = 'var\ni\nable' -value = '(null)' -reply = '(null)' +reply = 'GET with trailing characters' handling: -replied: 'ERROR 1 Command not found' +replied: 'ERROR 1 GET with trailing characters' ok test: 'GET 1 var\ti\table' parsing: id = '1' -variable = 'var\ti\table' -value = '(null)' -reply = '(null)' +reply = 'GET variable contains invalid characters' handling: -replied: 'ERROR 1 Command not found' +replied: 'ERROR 1 GET variable contains invalid characters' ok test: 'GET 1 var\ri\rable' parsing: id = '1' -variable = 'var\ri\rable' -value = '(null)' -reply = '(null)' +reply = 'GET variable contains invalid characters' handling: -replied: 'ERROR 1 Command not found' +replied: 'ERROR 1 GET variable contains invalid characters' ok test: 'GET 1 variable value' parsing: id = '1' -variable = 'variable' -value = '(null)' -reply = '(null)' +reply = 'GET with trailing characters' handling: -replied: 'ERROR 1 Command not found' +replied: 'ERROR 1 GET with trailing characters' ok test: 'GET 1 variable value\n' parsing: id = '1' -variable = 'variable' -value = '(null)' -reply = '(null)' +reply = 'GET with trailing characters' handling: -replied: 'ERROR 1 Command not found' +replied: 'ERROR 1 GET with trailing characters' ok test: 'GET 1 variable multiple value tokens' parsing: id = '1' -variable = 'variable' -value = '(null)' -reply = '(null)' +reply = 'GET with trailing characters' handling: -replied: 'ERROR 1 Command not found' +replied: 'ERROR 1 GET with trailing characters' ok test: 'GET 1 variable multiple value tokens\n' parsing: id = '1' -variable = 'variable' -value = '(null)' -reply = '(null)' +reply = 'GET with trailing characters' handling: -replied: 'ERROR 1 Command not found' +replied: 'ERROR 1 GET with trailing characters' ok test: 'SET 1 variable value' parsing: @@ -108,21 +94,17 @@ replied: 'ERROR 1 Command not found' ok test: 'SET weird_id variable value' parsing: -id = 'weird_id' -variable = 'variable' -value = 'value' -reply = '(null)' +id = 'err' +reply = 'Invalid message ID number' handling: -replied: 'ERROR weird_id Command not found' +replied: 'ERROR err Invalid message ID number' ok test: 'SET weird_id variable value\n' parsing: -id = 'weird_id' -variable = 'variable' -value = 'value' -reply = '(null)' +id = 'err' +reply = 'Invalid message ID number' handling: -replied: 'ERROR weird_id Command not found' +replied: 'ERROR err Invalid message ID number' ok test: 'SET 1 variable multiple value tokens' parsing: @@ -162,21 +144,17 @@ replied: 'ERROR 1 Command not found' ok test: 'SET \n special_char_id value' parsing: -id = '\n' -variable = 'special_char_id' -value = 'value' -reply = '(null)' +id = 'err' +reply = 'Invalid message ID number' handling: -replied: 'ERROR \n Command not found' +replied: 'ERROR err Invalid message ID number' ok test: 'SET \t special_char_id value' parsing: -id = '\t' -variable = 'special_char_id' -value = 'value' -reply = '(null)' +id = 'err' +reply = 'Invalid message ID number' handling: -replied: 'ERROR \t Command not found' +replied: 'ERROR err Invalid message ID number' ok test: 'GET_REPLY 1 variable OK' parsing: -- cgit v1.2.3