From c3b904a87da41eeda957a558d9825fcd9da8705f Mon Sep 17 00:00:00 2001 From: Pau Espin Pedrol Date: Tue, 28 Jul 2020 15:37:02 +0200 Subject: [PATCH] vty: Allow using hex representations in cmd numeric ranges Ranges can now be specified in hexadecimal notation. In this case, only hexadecimal values are accepted (prefixed with "0x"). In order to allow using a hexadecimal value as an input argument, the command must specify the range in hexadecimal form. This way all existing commands (decimal) won't get an hexadecimal value unless they are further extended in the future, avoiding hard to notice breakage due to use of stroul() without using base=0 or even worse, using atoi() directly (which only understands decimal and provides no error checking mechanism). A command argument can be expanded to accept both decimal and hex in a range by means of specifying both, example: "mycmd (<0-255>|<0x0-0xff>)". Related: OS#5631 Change-Id: Ia2b7fbbf5502c28374c21dbff548232680da27d4 --- src/vty/command.c | 61 ++++++++++++++++++++++++++++++++++---- tests/vty/vty_test.c | 67 ++++++++++++++++++++++++++++++++++++++++++ tests/vty/vty_test.err | 10 +++++++ tests/vty/vty_test.ok | 48 ++++++++++++++++++++++++++++++ 4 files changed, 180 insertions(+), 6 deletions(-) diff --git a/src/vty/command.c b/src/vty/command.c index d7b8026ff..1e76474f1 100644 --- a/src/vty/command.c +++ b/src/vty/command.c @@ -1480,19 +1480,52 @@ static enum match_type cmd_ipv6_prefix_match(const char *str) #error "LONG_MAX not defined!" #endif +/* This function is aimed at quickly guessing & filtering the numeric base a + * string can contain, by no means validates the entire value. + * Returns 16 if string follows pattern "({+,-}0x[digits])" + * Returns 10 if string follows pattern "({+,-}[digits])" + * Returns a negative value if something other is detected (error) +*/ +static int check_base(const char *str) +{ + const char *ptr = str; + /* Skip any space */ + while (isspace(*ptr)) ptr++; + + /* Skip optional sign: */ + if (*ptr == '+' || *ptr == '-') + ptr++; + if (*ptr == '0') { + ptr++; + if (*ptr == '\0' || isdigit(*ptr)) + return 10; + if (!(*ptr == 'x' || *ptr == 'X')) + return -1; + ptr++; + if (isxdigit(*ptr)) + return 16; + return -1; + } + return 10; +} + int vty_cmd_range_match(const char *range, const char *str) { char *p; char buf[DECIMAL_STRLEN_MAX_UNSIGNED + 1]; char *endptr = NULL; + int min_base, max_base, val_base; if (str == NULL) return 1; + if ((val_base = check_base(str)) < 0) + return 0; + if (range[1] == '-') { signed long min = 0, max = 0, val; - val = strtol(str, &endptr, 10); + val = strtol(str, &endptr, val_base); if (*endptr != '\0') return 0; @@ -1504,7 +1537,9 @@ int vty_cmd_range_match(const char *range, const char *str) return 0; strncpy(buf, range, p - range); buf[p - range] = '\0'; - min = -strtol(buf, &endptr, 10); + if ((min_base = check_base(buf)) < 0) + return 0; + min = -strtol(buf, &endptr, min_base); if (*endptr != '\0') return 0; @@ -1516,7 +1551,9 @@ int vty_cmd_range_match(const char *range, const char *str) return 0; strncpy(buf, range, p - range); buf[p - range] = '\0'; - max = strtol(buf, &endptr, 10); + if ((max_base = check_base(buf)) < 0) + return 0; + max = strtol(buf, &endptr, max_base); if (*endptr != '\0') return 0; @@ -1528,7 +1565,7 @@ int vty_cmd_range_match(const char *range, const char *str) if (str[0] == '-') return 0; - val = strtoul(str, &endptr, 10); + val = strtoul(str, &endptr, val_base); if (*endptr != '\0') return 0; @@ -1540,7 +1577,9 @@ int vty_cmd_range_match(const char *range, const char *str) return 0; strncpy(buf, range, p - range); buf[p - range] = '\0'; - min = strtoul(buf, &endptr, 10); + if ((min_base = check_base(buf)) < 0) + return 0; + min = strtoul(buf, &endptr, min_base); if (*endptr != '\0') return 0; @@ -1552,7 +1591,9 @@ int vty_cmd_range_match(const char *range, const char *str) return 0; strncpy(buf, range, p - range); buf[p - range] = '\0'; - max = strtoul(buf, &endptr, 10); + if ((max_base = check_base(buf)) < 0) + return 0; + max = strtoul(buf, &endptr, max_base); if (*endptr != '\0') return 0; @@ -1560,6 +1601,14 @@ int vty_cmd_range_match(const char *range, const char *str) return 0; } + /* Don't allow ranges specified by min and max with different bases */ + if (min_base != max_base) + return 0; + /* arg value passed must match the base of the range */ + if (min_base != val_base) + return 0; + + /* Everything's fine */ return 1; } diff --git a/tests/vty/vty_test.c b/tests/vty/vty_test.c index c84e4c9c1..3076a703d 100644 --- a/tests/vty/vty_test.c +++ b/tests/vty/vty_test.c @@ -448,6 +448,34 @@ DEFUN(cfg_numeric_range, cfg_numeric_range_cmd, return CMD_SUCCESS; } +DEFUN(cfg_range_base10, cfg_range_base10_cmd, + "range-base10 <0-999999>", + "testing decimal range\n" + "the decimal range\n") +{ + printf("Called: 'return-success'\n"); + return CMD_SUCCESS; +} + +DEFUN(cfg_range_base16, cfg_range_base16_cmd, + "range-base16 <0x0-0x8888>", + "testing hexadecimal range\n" + "the hexadecimal range\n") +{ + printf("Called: 'return-success'\n"); + return CMD_SUCCESS; +} + +DEFUN(cfg_range_baseboth, cfg_range_baseboth_cmd, + "range-baseboth (<0-999999>|<0x0-0x8888>)", + "testing both ranges\n" + "the decimal range\n" + "the hexadecimal range\n") +{ + printf("Called: 'return-success'\n"); + return CMD_SUCCESS; +} + void test_vty_add_cmds() { install_element(CONFIG_NODE, &cfg_ret_warning_cmd); @@ -473,6 +501,10 @@ void test_vty_add_cmds() install_element_ve(&cfg_ambiguous_str_2_cmd); install_element_ve(&cfg_numeric_range_cmd); + + install_element_ve(&cfg_range_base10_cmd); + install_element_ve(&cfg_range_base16_cmd); + install_element_ve(&cfg_range_baseboth_cmd); } void test_is_cmd_ambiguous() @@ -509,6 +541,40 @@ void test_numeric_range() destroy_test_vty(&test, vty); } +void test_ranges() +{ + struct vty *vty; + struct vty_test test; + + printf("Going to test test_ranges()\n"); + vty = create_test_vty(&test); + + printf("test range-base10\n"); + OSMO_ASSERT(do_vty_command(vty, "range-base10 0") == CMD_SUCCESS); + OSMO_ASSERT(do_vty_command(vty, "range-base10 40000") == CMD_SUCCESS); + OSMO_ASSERT(do_vty_command(vty, "range-base10 -400000") == CMD_ERR_NO_MATCH); + OSMO_ASSERT(do_vty_command(vty, "range-base10 0x0") == CMD_ERR_NO_MATCH); + OSMO_ASSERT(do_vty_command(vty, "range-base10 0x343") == CMD_ERR_NO_MATCH); + OSMO_ASSERT(do_vty_command(vty, "range-base10 -0x343") == CMD_ERR_NO_MATCH); + + printf("test range-base16\n"); + OSMO_ASSERT(do_vty_command(vty, "range-base16 0") == CMD_ERR_NO_MATCH); + OSMO_ASSERT(do_vty_command(vty, "range-base16 40000") == CMD_ERR_NO_MATCH); + OSMO_ASSERT(do_vty_command(vty, "range-base16 -400000") == CMD_ERR_NO_MATCH); + OSMO_ASSERT(do_vty_command(vty, "range-base16 0x0") == CMD_SUCCESS); + OSMO_ASSERT(do_vty_command(vty, "range-base16 0x343") == CMD_SUCCESS); + OSMO_ASSERT(do_vty_command(vty, "range-base16 -0x343") == CMD_ERR_NO_MATCH); + + printf("test range-baseboth\n"); + OSMO_ASSERT(do_vty_command(vty, "range-baseboth 0") == CMD_SUCCESS); + OSMO_ASSERT(do_vty_command(vty, "range-baseboth 40000") == CMD_SUCCESS); + OSMO_ASSERT(do_vty_command(vty, "range-baseboth -400000") == CMD_ERR_NO_MATCH); + OSMO_ASSERT(do_vty_command(vty, "range-baseboth 0x0") == CMD_SUCCESS); + OSMO_ASSERT(do_vty_command(vty, "range-baseboth 0x343") == CMD_SUCCESS); + OSMO_ASSERT(do_vty_command(vty, "range-baseboth -0x343") == CMD_ERR_NO_MATCH); + + destroy_test_vty(&test, vty); +} /* Application specific attributes */ enum vty_test_attr { VTY_TEST_ATTR_FOO = 0, @@ -591,6 +657,7 @@ int main(int argc, char **argv) test_is_cmd_ambiguous(); test_numeric_range(); + test_ranges(); /* Leak check */ OSMO_ASSERT(talloc_total_blocks(stats_ctx) == 1); diff --git a/tests/vty/vty_test.err b/tests/vty/vty_test.err index 1cb4190c9..b021425dd 100644 --- a/tests/vty/vty_test.err +++ b/tests/vty/vty_test.err @@ -65,3 +65,13 @@ Got VTY event: 2 Got VTY event: 2 Got VTY event: 1 Got VTY event: 3 +Got VTY event: 2 +Got VTY event: 2 +Got VTY event: 2 +Got VTY event: 2 +Got VTY event: 2 +Got VTY event: 2 +Got VTY event: 2 +Got VTY event: 2 +Got VTY event: 1 +Got VTY event: 3 diff --git a/tests/vty/vty_test.ok b/tests/vty/vty_test.ok index 5f509f654..e97fbfc4e 100644 --- a/tests/vty/vty_test.ok +++ b/tests/vty/vty_test.ok @@ -320,4 +320,52 @@ Called: 'return-success' Returned: 0, Current node: 1 '%s> ' Going to execute 'numeric-range -400000' Returned: 2, Current node: 1 '%s> ' +Going to test test_ranges() +test range-base10 +Going to execute 'range-base10 0' +Called: 'return-success' +Returned: 0, Current node: 1 '%s> ' +Going to execute 'range-base10 40000' +Called: 'return-success' +Returned: 0, Current node: 1 '%s> ' +Going to execute 'range-base10 -400000' +Returned: 2, Current node: 1 '%s> ' +Going to execute 'range-base10 0x0' +Returned: 2, Current node: 1 '%s> ' +Going to execute 'range-base10 0x343' +Returned: 2, Current node: 1 '%s> ' +Going to execute 'range-base10 -0x343' +Returned: 2, Current node: 1 '%s> ' +test range-base16 +Going to execute 'range-base16 0' +Returned: 2, Current node: 1 '%s> ' +Going to execute 'range-base16 40000' +Returned: 2, Current node: 1 '%s> ' +Going to execute 'range-base16 -400000' +Returned: 2, Current node: 1 '%s> ' +Going to execute 'range-base16 0x0' +Called: 'return-success' +Returned: 0, Current node: 1 '%s> ' +Going to execute 'range-base16 0x343' +Called: 'return-success' +Returned: 0, Current node: 1 '%s> ' +Going to execute 'range-base16 -0x343' +Returned: 2, Current node: 1 '%s> ' +test range-baseboth +Going to execute 'range-baseboth 0' +Called: 'return-success' +Returned: 0, Current node: 1 '%s> ' +Going to execute 'range-baseboth 40000' +Called: 'return-success' +Returned: 0, Current node: 1 '%s> ' +Going to execute 'range-baseboth -400000' +Returned: 2, Current node: 1 '%s> ' +Going to execute 'range-baseboth 0x0' +Called: 'return-success' +Returned: 0, Current node: 1 '%s> ' +Going to execute 'range-baseboth 0x343' +Called: 'return-success' +Returned: 0, Current node: 1 '%s> ' +Going to execute 'range-baseboth -0x343' +Returned: 2, Current node: 1 '%s> ' All tests passed