From: Nick Porter Date: Fri, 6 Jan 2023 16:53:26 +0000 (+0000) Subject: Handle corner case in using fr_sbuff_is...() functions on markers (#4839) X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f875127439f8f2a11a286b3e09b6b490d05ac044;p=thirdparty%2Ffreeradius-server.git Handle corner case in using fr_sbuff_is...() functions on markers (#4839) * Corner case fix for calling an fr_sbuff_is... function on a marker Previously if fr_sbuff_is_...() was called on a marker and the buffer had advanced past the end, then the function always returned false, regardless of the character the marker was pointing at. * Test to check fr_sbuff_is..() handles markers regardless of buffer position * Correct parsing .0 into fr_time_delta_t --- diff --git a/src/lib/util/sbuff.h b/src/lib/util/sbuff.h index 41c6e96c815..881c577122c 100644 --- a/src/lib/util/sbuff.h +++ b/src/lib/util/sbuff.h @@ -1729,49 +1729,58 @@ static inline bool _fr_sbuff_is_char(fr_sbuff_t *sbuff, char *p, char c) if (!fr_sbuff_extend(sbuff)) return false; return *p == c; } -#define fr_sbuff_is_char(_sbuff_or_marker, _c) _fr_sbuff_is_char(fr_sbuff_ptr(_sbuff_or_marker), fr_sbuff_current(_sbuff_or_marker), _c) - -static inline bool _fr_sbuff_is_digit(fr_sbuff_t *sbuff, char *p) +static inline bool _fr_marker_is_char(fr_sbuff_marker_t *marker, char *p, char c) { - if (!fr_sbuff_extend(sbuff)) return false; - return isdigit(*p); + if (!fr_sbuff_extend(marker)) return false; + return *p == c; } -#define fr_sbuff_is_digit(_sbuff_or_marker) _fr_sbuff_is_digit(fr_sbuff_ptr(_sbuff_or_marker), fr_sbuff_current(_sbuff_or_marker)) +#define fr_sbuff_is_char(_sbuff_or_marker, _c) \ + _Generic((_sbuff_or_marker), \ + fr_sbuff_t * : _fr_sbuff_is_char((fr_sbuff_t *)(_sbuff_or_marker), fr_sbuff_current(_sbuff_or_marker), _c), \ + fr_sbuff_marker_t * : _fr_marker_is_char((fr_sbuff_marker_t *)(_sbuff_or_marker), fr_sbuff_current(_sbuff_or_marker), _c) \ + ) -static inline bool _fr_sbuff_is_upper(fr_sbuff_t *sbuff, char const *p) -{ - if (!fr_sbuff_extend(sbuff)) return false; - return isupper(*p); -} -#define fr_sbuff_is_upper(_sbuff_or_marker) _fr_sbuff_is_upper(fr_sbuff_ptr(_sbuff_or_marker), fr_sbuff_current(_sbuff_or_marker)) +#define SBUFF_IS_FUNC(_name, _test) \ + static inline bool _fr_sbuff_is_ ## _name(fr_sbuff_t *sbuff, char *p) \ + { \ + if (!fr_sbuff_extend(sbuff)) return false; \ + return _test; \ + }\ + static inline bool _fr_marker_is_ ## _name(fr_sbuff_marker_t *marker, char *p) \ + { \ + if (!fr_sbuff_extend(marker)) return false; \ + return _test; \ + } -static inline bool _fr_sbuff_is_lower(fr_sbuff_t *sbuff, char const *p) -{ - if (!fr_sbuff_extend(sbuff)) return false; - return islower(*p); -} -#define fr_sbuff_is_lower(_sbuff_or_marker) _fr_sbuff_is_lower(fr_sbuff_ptr(_sbuff_or_marker), fr_sbuff_current(_sbuff_or_marker)) +#define SBUFF_IS_GENERIC(_sbuff_or_marker, _name) \ + _Generic((_sbuff_or_marker), \ + fr_sbuff_t * : _fr_sbuff_is_ ## _name((fr_sbuff_t *)(_sbuff_or_marker), fr_sbuff_current(_sbuff_or_marker)), \ + fr_sbuff_marker_t * : _fr_marker_is_ ## _name((fr_sbuff_marker_t *)(_sbuff_or_marker), fr_sbuff_current(_sbuff_or_marker)) \ + ) -static inline bool _fr_sbuff_is_alpha(fr_sbuff_t *sbuff, char const *p) -{ - if (!fr_sbuff_extend(sbuff)) return false; - return isalpha(*p); -} -#define fr_sbuff_is_alpha(_sbuff_or_marker) _fr_sbuff_is_alpha(fr_sbuff_ptr(_sbuff_or_marker), fr_sbuff_current(_sbuff_or_marker)) +SBUFF_IS_FUNC(digit, isdigit(*p)) +#define fr_sbuff_is_digit(_sbuff_or_marker) \ + SBUFF_IS_GENERIC(_sbuff_or_marker, digit) -static inline bool _fr_sbuff_is_space(fr_sbuff_t *sbuff, char const *p) -{ - if (!fr_sbuff_extend(sbuff)) return false; - return isspace(*p); -} -#define fr_sbuff_is_space(_sbuff_or_marker) _fr_sbuff_is_space(fr_sbuff_ptr(_sbuff_or_marker), fr_sbuff_current(_sbuff_or_marker)) +SBUFF_IS_FUNC(upper, isupper(*p)) +#define fr_sbuff_is_upper(_sbuff_or_marker) \ + SBUFF_IS_GENERIC(_sbuff_or_marker, upper) -static inline bool _fr_sbuff_is_hex(fr_sbuff_t *sbuff, char const *p) -{ - if (!fr_sbuff_extend(sbuff)) return false; - return isxdigit(*p); -} -#define fr_sbuff_is_hex(_sbuff_or_marker) _fr_sbuff_is_hex(fr_sbuff_ptr(_sbuff_or_marker), fr_sbuff_current(_sbuff_or_marker)) +SBUFF_IS_FUNC(lower, islower(*p)) +#define fr_sbuff_is_lower(_sbuff_or_marker) \ + SBUFF_IS_GENERIC(_sbuff_or_marker, lower) + +SBUFF_IS_FUNC(alpha, isalpha(*p)) +#define fr_sbuff_is_alpha(_sbuff_or_marker) \ + SBUFF_IS_GENERIC(_sbuff_or_marker, alpha) + +SBUFF_IS_FUNC(space, isspace(*p)) +#define fr_sbuff_is_space(_sbuff_or_marker) \ + SBUFF_IS_GENERIC(_sbuff_or_marker, space) + +SBUFF_IS_FUNC(hex, isxdigit(*p)) +#define fr_sbuff_is_hex(_sbuff_or_marker) \ + SBUFF_IS_GENERIC(_sbuff_or_marker, hex) /** @} */ diff --git a/src/lib/util/sbuff_tests.c b/src/lib/util/sbuff_tests.c index be77e4af2d9..18878fa1545 100644 --- a/src/lib/util/sbuff_tests.c +++ b/src/lib/util/sbuff_tests.c @@ -72,6 +72,35 @@ static void test_parse_init(void) TEST_CHECK(sbuff.end == in + strlen(in)); } +static void test_is_char(void) +{ + char const in[] = "i am a test string"; + fr_sbuff_t sbuff; + fr_sbuff_marker_t marker; + + fr_sbuff_init_in(&sbuff, in, sizeof(in) - 1); + TEST_CHECK(fr_sbuff_is_char(&sbuff, 'i')); + TEST_CHECK(!fr_sbuff_is_char(&sbuff, 'z')); + + fr_sbuff_advance(&sbuff, 2); + TEST_CHECK(!fr_sbuff_is_char(&sbuff, 'i')); + TEST_CHECK(fr_sbuff_is_char(&sbuff, 'a')); + + fr_sbuff_advance(&sbuff, 15); + TEST_CHECK(fr_sbuff_is_char(&sbuff, 'g')); + fr_sbuff_marker(&marker, &sbuff); + TEST_CHECK(fr_sbuff_is_char(&marker, 'g')); + + /* + * Ensure that after advancing the buffer past + * the end, the marker can still be correctly + * tested + */ + fr_sbuff_advance(&sbuff, 1); + TEST_CHECK(!fr_sbuff_is_char(&sbuff, 'g')); + TEST_CHECK(fr_sbuff_is_char(&marker, 'g')); +} + static void test_bstrncpy_exact(void) { char const in[] = "i am a test string"; @@ -1541,6 +1570,7 @@ TEST_LIST = { * Basic tests */ { "fr_sbuff_init", test_parse_init }, + { "fr_sbuff_is_char", test_is_char }, { "fr_sbuff_out_bstrncpy_exact", test_bstrncpy_exact }, { "fr_sbuff_out_bstrncpy", test_bstrncpy }, { "fr_sbuff_out_bstrncpy_allowed", test_bstrncpy_allowed }, diff --git a/src/lib/util/time.c b/src/lib/util/time.c index aa55024556e..c27843939eb 100644 --- a/src/lib/util/time.c +++ b/src/lib/util/time.c @@ -296,7 +296,7 @@ fr_slen_t fr_time_delta_from_substr(fr_time_delta_t *out, fr_sbuff_t *in, fr_tim if (fr_sbuff_next_if_char(&our_in, '.')) { fr_sbuff_marker_t m_f; size_t f_len; - uint64_t f; /* Fractional units */ + uint64_t f = 0; /* Fractional units */ /* * Normalise as a positive integer @@ -321,7 +321,7 @@ fr_slen_t fr_time_delta_from_substr(fr_time_delta_t *out, fr_sbuff_t *in, fr_tim * up the fractional parsing when the * fraction is all zeros... */ - if ((sberr != FR_SBUFF_PARSE_ERROR_NOT_FOUND) || fr_sbuff_is_char(&m_f, '0')) goto num_error; + if ((sberr != FR_SBUFF_PARSE_ERROR_NOT_FOUND) || !fr_sbuff_is_char(&m_f, '0')) goto num_error; } f_len = fr_sbuff_behind(&m_f);