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

diff --git a/src/lib/util/sbuff.c b/src/lib/util/sbuff.c
index fef844ed1e020ed1174dd620109032d183b2aa83..1c4f07f6a78e932cff13e21f9fb8351f1836698a 100644 (file)
--- 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 ea590d27e2ca5244b5e278f311ffb216c3dce6d2..3b8307fa2e57a108bf228b9a68b54dd69cfeb0b7 100644 (file)
--- 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 <freeradius-devel/util/atexit.h>
 #include <freeradius-devel/util/strerror.h>
 #include <freeradius-devel/util/table.h>
 #include <freeradius-devel/util/talloc.h>
 
+/** 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, "<INVALID>"));
 }
 
-#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 0d698bec2705988988e91547af6f2d8a715ff36b..1da44abe2d5b20848771850eea5ccd279bfcf420 100644 (file)
--- 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);