From bc22359be57c13bf4bb408fd98d91745f99d5125 Mon Sep 17 00:00:00 2001 From: Arran Cudbard-Bell Date: Wed, 27 Nov 2024 17:36:29 -0600 Subject: [PATCH] Have fr_sbuff_extend_lowat check the eof state of the sbuff. Fixes #5462 Don't extend the sbuff in the fr_sbuff_terminal_search function --- src/lib/util/sbuff.c | 46 +++++++++++++++------ src/lib/util/sbuff.h | 85 ++++++++++++++++++++++++-------------- src/lib/util/sbuff_tests.c | 2 +- 3 files changed, 89 insertions(+), 44 deletions(-) diff --git a/src/lib/util/sbuff.c b/src/lib/util/sbuff.c index fef844ed1e020..1c4f07f6a78e9 100644 --- a/src/lib/util/sbuff.c +++ b/src/lib/util/sbuff.c @@ -262,7 +262,7 @@ size_t fr_sbuff_shift(fr_sbuff_t *sbuff, size_t shift) /** Refresh the buffer with more data from the file * */ -size_t fr_sbuff_extend_file(fr_sbuff_t *sbuff, size_t extension) +size_t fr_sbuff_extend_file(fr_sbuff_extend_status_t *status, fr_sbuff_t *sbuff, size_t extension) { fr_sbuff_t *sbuff_i; size_t read, available, total_read, shift; @@ -323,6 +323,7 @@ size_t fr_sbuff_extend_file(fr_sbuff_t *sbuff, size_t extension) if (read < available) { if (!feof(fctx->file)) { /* It's a real error */ fr_strerror_printf("Error extending buffer: %s", fr_syserror(ferror(fctx->file))); + *status |= FR_SBUFF_FLAG_EXTEND_ERROR; return 0; } @@ -332,8 +333,18 @@ size_t fr_sbuff_extend_file(fr_sbuff_t *sbuff, size_t extension) return read; } +/** Accessor function for the EOF state of the file extendor + * + */ +bool fr_sbuff_eof_file(fr_sbuff_t *sbuff) +{ + fr_sbuff_uctx_file_t *fctx = sbuff->uctx; + return fctx->eof; +} + /** Reallocate the current buffer * + * @param[in] status Extend status. * @param[in] sbuff to be extended. * @param[in] extension How many additional bytes should be allocated * in the buffer. @@ -341,7 +352,7 @@ size_t fr_sbuff_extend_file(fr_sbuff_t *sbuff, size_t extension) * - 0 the extension operation failed. * - >0 the number of bytes the buffer was extended by. */ -size_t fr_sbuff_extend_talloc(fr_sbuff_t *sbuff, size_t extension) +size_t fr_sbuff_extend_talloc(fr_sbuff_extend_status_t *status, fr_sbuff_t *sbuff, size_t extension) { fr_sbuff_uctx_talloc_t *tctx = sbuff->uctx; size_t clen, nlen, elen = extension; @@ -386,6 +397,7 @@ size_t fr_sbuff_extend_talloc(fr_sbuff_t *sbuff, size_t extension) new_buff = talloc_realloc(tctx->ctx, sbuff->buff, char, nlen); if (unlikely(!new_buff)) { fr_strerror_printf("Failed extending buffer by %zu bytes to %zu bytes", elen, nlen); + *status |= FR_SBUFF_FLAG_EXTEND_ERROR; return 0; } @@ -536,8 +548,6 @@ static inline bool fr_sbuff_terminal_search(fr_sbuff_t *in, char const *p, ssize_t mid; size_t remaining; - bool reset_p = (p == in->p); - fr_sbuff_extend_status_t status = FR_SBUFF_EXTENDABLE; if (!term) return false; /* If there's no terminals, we don't need to search */ @@ -549,13 +559,22 @@ static inline bool fr_sbuff_terminal_search(fr_sbuff_t *in, char const *p, /* * Special case for EOFlike states */ - remaining = fr_sbuff_extend_lowat(&status, in, needle_len); - if (remaining == 0) { - if (status & FR_SBUFF_EXTEND_ERROR) return false; - return (idx['\0'] != 0); + remaining = fr_sbuff_remaining(in); + if ((remaining == 0) && !fr_sbuff_is_extendable(in)) { + if (idx['\0'] != 0) return true; + return false; } - if (reset_p) p = in->p; + if (remaining < needle_len) { + fr_assert_msg(!fr_sbuff_is_extendable(in), + "Caller failed to extend buffer by %zu bytes before calling fr_sbuff_terminal_search", + needle_len); + /* + * We can't search for the needle if we don't have + * enough data to match it. + */ + return false; + } mid = term_idx - 1; /* Inform the mid point from the index */ @@ -925,8 +944,7 @@ size_t fr_sbuff_out_unescape_until(fr_sbuff_t *out, fr_sbuff_t *in, size_t len, uint8_t idx[UINT8_MAX + 1]; /* Fast path index */ size_t needle_len = 1; - - fr_sbuff_extend_status_t status = FR_SBUFF_EXTENDABLE; /* Tracks if we can extend */ + fr_sbuff_extend_status_t status = 0; /* * If we don't need to do unescaping @@ -2140,10 +2158,10 @@ bool fr_sbuff_is_terminal(fr_sbuff_t *in, fr_sbuff_term_t const *tt) * No terminal, check for EOF. */ if (!tt) { - fr_sbuff_extend_status_t status = FR_SBUFF_EXTENDABLE; + fr_sbuff_extend_status_t status = 0; if ((fr_sbuff_extend_lowat(&status, in, 1) == 0) && - (status & FR_SBUFF_EXTEND_ERROR) == 0) { + (status & FR_SBUFF_FLAG_EXTEND_ERROR) == 0) { return true; } @@ -2156,6 +2174,8 @@ bool fr_sbuff_is_terminal(fr_sbuff_t *in, fr_sbuff_term_t const *tt) */ fr_sbuff_terminal_idx_init(&needle_len, idx, tt); + fr_sbuff_extend_lowat(NULL, in, needle_len); + return fr_sbuff_terminal_search(in, in->p, idx, tt, needle_len); } diff --git a/src/lib/util/sbuff.h b/src/lib/util/sbuff.h index ea590d27e2ca5..3b8307fa2e57a 100644 --- a/src/lib/util/sbuff.h +++ b/src/lib/util/sbuff.h @@ -51,13 +51,36 @@ extern "C" { typedef ssize_t fr_slen_t; typedef struct fr_sbuff_s fr_sbuff_t; typedef struct fr_sbuff_ptr_s fr_sbuff_marker_t; -typedef size_t(*fr_sbuff_extend_t)(fr_sbuff_t *sbuff, size_t req_extension); #include #include #include #include +/** Whether the buffer is currently extendable and whether it was extended + * + */ +DIAG_OFF(attributes) +typedef enum CC_HINT(flag_enum) { + FR_SBUFF_FLAG_EXTENDED = 0x01, //!< The last call to extend function actually extended the buffer. + FR_SBUFF_FLAG_EXTEND_ERROR = 0x02 //!< The last call to an extend function resulted in an error. + ///< Error should be provided using fr_strerror_const/fr_strerror_printf + ///< by the extension function. +} fr_sbuff_extend_status_t; +DIAG_OFF(attributes) + +/** Extension callback + * + * Retrieves additional data from a source and adds it to a buffer. + */ +typedef size_t(*fr_sbuff_extend_t)(fr_sbuff_extend_status_t *status, fr_sbuff_t *sbuff, size_t req_extension); + +/** For a given extension function, returns whether it is at EOF + * + */ +typedef bool(*fr_sbuff_eof_t)(fr_sbuff_t *sbuff); + + struct fr_sbuff_ptr_s { union { char const *p_i; //!< Immutable position pointer. @@ -98,6 +121,8 @@ struct fr_sbuff_s { fr_sbuff_extend_t extend; //!< Function to re-populate or extend ///< the buffer. + fr_sbuff_eof_t eof; //!< Function to determine if the buffer is at EOF. + void *uctx; //!< Extend uctx data. fr_sbuff_t *parent; //!< sbuff this sbuff was copied from. @@ -246,23 +271,14 @@ static inline void fr_sbuff_parse_error_to_strerror(fr_sbuff_parse_error_t err) fr_strerror_const(fr_table_str_by_value(sbuff_parse_error_table, err, "")); } -#define FR_SBUFF_FLAG_EXTENDABLE 0x01 -#define FR_SBUFF_FLAG_EXTENDED 0x02 -#define FR_SBUFF_FLAG_EXTEND_ERROR 0x04 - -/** Whether the buffer is currently extendable and whether it was extended - * +/** Return whether the sbuff is extendable */ -typedef enum { - FR_SBUFF_NOT_EXTENDABLE = 0x00, - FR_SBUFF_EXTENDABLE = FR_SBUFF_FLAG_EXTENDABLE, - FR_SBUFF_EXTENDABLE_EXTENDED = FR_SBUFF_FLAG_EXTENDABLE | FR_SBUFF_FLAG_EXTENDED, - FR_SBUFF_EXTENDED = FR_SBUFF_FLAG_EXTENDED, - FR_SBUFF_EXTEND_ERROR = FR_SBUFF_FLAG_EXTEND_ERROR -} fr_sbuff_extend_status_t; +static inline bool fr_sbuff_is_extendable(fr_sbuff_t *sbuff) +{ + return sbuff->extend && (!sbuff->eof || (sbuff->eof(sbuff) == false)); +} -#define fr_sbuff_is_extendable(_status) ((_status) & FR_SBUFF_FLAG_EXTENDABLE) -#define fr_sbuff_was_extended(_status) ((_status) & FR_SBUFF_FLAG_EXTENDED) +#define fr_sbuff_was_extended(_status) (_status & FR_SBUFF_FLAG_EXTENDED) extern bool const sbuff_char_class_uint[UINT8_MAX + 1]; extern bool const sbuff_char_class_int[UINT8_MAX + 1]; @@ -401,7 +417,7 @@ do { \ * * @private */ -#define _FR_SBUFF(_sbuff_or_marker, _start, _current, _end, _extend, _adv_parent) \ +#define _FR_SBUFF(_sbuff_or_marker, _start, _current, _end, _extend, _eof, _adv_parent) \ ((fr_sbuff_t){ \ .buff = fr_sbuff_buff(_sbuff_or_marker), \ .start = (_start), \ @@ -411,6 +427,7 @@ do { \ .adv_parent = (_adv_parent), \ .shifted = 0, \ .extend = (_extend), \ + .eof = (_eof), \ .uctx = fr_sbuff_ptr(_sbuff_or_marker)->uctx, \ .parent = fr_sbuff_ptr(_sbuff_or_marker) \ }) @@ -428,6 +445,7 @@ do { \ fr_sbuff_current(_sbuff_or_marker), \ fr_sbuff_end(_sbuff_or_marker), \ fr_sbuff_ptr(_sbuff_or_marker)->extend, \ + fr_sbuff_ptr(_sbuff_or_marker)->eof, \ 0x00) /** Create a new sbuff pointing to the same underlying buffer @@ -461,6 +479,7 @@ do { \ fr_sbuff_start(_sbuff_or_marker), \ fr_sbuff_current(_sbuff_or_marker), \ NULL, \ + NULL, \ 0x00) /** Create a new sbuff pointing to the same underlying buffer @@ -475,6 +494,7 @@ do { \ fr_sbuff_current(_sbuff_or_marker), \ fr_sbuff_end(_sbuff_or_marker), \ fr_sbuff_ptr(_sbuff_or_marker)->extend, \ + fr_sbuff_ptr(_sbuff_or_marker)->eof, \ 0x01) /** Create a new sbuff pointing to the same underlying buffer @@ -489,6 +509,7 @@ do { \ fr_sbuff_current(_sbuff_or_marker), \ fr_sbuff_end(_sbuff_or_marker), \ fr_sbuff_ptr(_sbuff_or_marker)->extend, \ + fr_sbuff_ptr(_sbuff_or_marker)->eof, \ 0x01) /** Creates a compound literal to pass into functions which accept a sbuff @@ -576,9 +597,11 @@ void fr_sbuff_update(fr_sbuff_t *sbuff, char *new_buff, size_t new_len); size_t fr_sbuff_shift(fr_sbuff_t *sbuff, size_t shift); -size_t fr_sbuff_extend_file(fr_sbuff_t *sbuff, size_t extension); +size_t fr_sbuff_extend_file(fr_sbuff_extend_status_t *status, fr_sbuff_t *sbuff, size_t extension); + +bool fr_sbuff_eof_file(fr_sbuff_t *sbuff); -size_t fr_sbuff_extend_talloc(fr_sbuff_t *sbuff, size_t extension); +size_t fr_sbuff_extend_talloc(fr_sbuff_extend_status_t *status, fr_sbuff_t *sbuff, size_t extension); int fr_sbuff_trim_talloc(fr_sbuff_t *sbuff, size_t len); @@ -681,6 +704,7 @@ static inline fr_sbuff_t *fr_sbuff_init_file(fr_sbuff_t *sbuff, fr_sbuff_uctx_fi .p = buff, .end = buff, //!< Starts with 0 bytes available .extend = fr_sbuff_extend_file, + .eof = fr_sbuff_eof_file, .uctx = fctx }; @@ -989,26 +1013,27 @@ static inline fr_slen_t _fr_sbuff_error(fr_sbuff_t *sbuff, char const *err) #define FR_SBUFF_CHECK_REMAINING_RETURN(_sbuff, _len) \ if ((_len) > fr_sbuff_remaining(_sbuff)) return -((_len) - fr_sbuff_remaining(_sbuff)) -static inline size_t _fr_sbuff_extend_lowat(fr_sbuff_extend_status_t *status, fr_sbuff_t *in, - size_t remaining, size_t lowat) +static inline size_t _fr_sbuff_extend_lowat(fr_sbuff_extend_status_t *status, fr_sbuff_t *in, size_t remaining, size_t lowat) { size_t extended; + fr_sbuff_extend_status_t our_status = 0; - if (status && !(*status & FR_SBUFF_EXTENDABLE)) { - not_extendable: - if (status) *status = FR_SBUFF_NOT_EXTENDABLE; + if (!fr_sbuff_is_extendable(in)) { + no_extend: + if (status) *status = our_status; return remaining; } - if (remaining >= lowat) { - if (status) *status = FR_SBUFF_EXTENDABLE; - return remaining; - } + /* Still have data remaining, no need to try and extend */ + if (remaining >= lowat) goto no_extend; - if (!in->extend || !(extended = in->extend(in, lowat - remaining))) goto not_extendable; + if (!in->extend || ((extended = in->extend(&our_status, in, lowat - remaining)) == 0)) { + goto no_extend; + } - if (status) *status = FR_SBUFF_EXTENDABLE_EXTENDED; + our_status |= FR_SBUFF_FLAG_EXTENDED; + if (status) *status = our_status; return remaining + extended; } diff --git a/src/lib/util/sbuff_tests.c b/src/lib/util/sbuff_tests.c index 0d698bec27059..1da44abe2d5b2 100644 --- a/src/lib/util/sbuff_tests.c +++ b/src/lib/util/sbuff_tests.c @@ -1068,7 +1068,7 @@ static void test_file_extend(void) TEST_CHECK_LEN(fr_sbuff_adv_past_whitespace(&our_sbuff, SIZE_MAX, NULL), sizeof(fbuff) - PATTERN_LEN); TEST_CASE("Verify extend on unused child buffer"); child_sbuff = FR_SBUFF(&our_sbuff); - slen = fr_sbuff_extend_file(&child_sbuff, 0); + slen = fr_sbuff_extend_file(NULL, &child_sbuff, 0); TEST_CHECK_SLEN(slen, sizeof(fbuff) % PATTERN_LEN); TEST_CASE("Verify that we passed all and only whitespace"); (void) fr_sbuff_out_abstrncpy(NULL, &post_ws, &our_sbuff, 24);