This is an automated email from the ASF dual-hosted git repository. asf-gitbox-commits pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/qpid-proton.git
commit 6e237cc6ce73dace6e7334062040e90eccbcb83a Author: Andrew Stitcher <[email protected]> AuthorDate: Fri Aug 28 13:50:02 2026 -0400 PROTON-2969: Stop dumping a value once the output buffer is full pn_value_dump_nondescribed_value() kept walking a value's children after the output buffer had filled up, with nothing left to write. Return as soon as the buffer is full. Also: - Skip a chain of descriptors iteratively rather than recursively, so dumping a value uses a bounded amount of C stack whatever it contains. Clean ups to pn_fixed_string_t: - Assert that we only make pn_fixed_string_t with at least 1 byte of storage otherwise there is nowehere for the terminating '\0'. - Make that all users of pn_fixed_string (which is internal oinly) pass in at least one byte. Assisted-By: Claude Opus 5 <[email protected]> --- c/src/core/codec.c | 5 +++++ c/src/core/fixed_string.h | 10 +++++++++- c/src/core/value_dump.c | 25 ++++++++++++++++++++----- c/tests/data_test.cpp | 20 ++++++++++++++++++++ 4 files changed, 54 insertions(+), 6 deletions(-) diff --git a/c/src/core/codec.c b/c/src/core/codec.c index 63d20d1fb..ccecced01 100644 --- a/c/src/core/codec.c +++ b/c/src/core/codec.c @@ -1277,6 +1277,11 @@ int pn_data_print(pn_data_t *data) int pn_data_format(pn_data_t *data, char *bytes, size_t *size) { + // With no free space there is nowhere to put even the terminating null, so + // there is no formatting to be done. Overflow is not reported here, as it + // isn't for a buffer that merely turns out to be too small. + if (*size == 0) return 0; + pn_fixed_string_t str = pn_fixed_string(bytes, *size); pn_data_inspect(data, &str); pn_fixed_string_terminate(&str); diff --git a/c/src/core/fixed_string.h b/c/src/core/fixed_string.h index 27d23c7e8..1ebd51ece 100644 --- a/c/src/core/fixed_string.h +++ b/c/src/core/fixed_string.h @@ -26,6 +26,7 @@ #include <proton/annotations.h> +#include <assert.h> #include <stdarg.h> #include <stdio.h> #include <string.h> @@ -43,6 +44,9 @@ typedef struct pn_string_const_t { } pn_string_const_t; static inline pn_fixed_string_t pn_fixed_string(char *bytes, uint32_t size) { + // A fixed string has nowhere to put even the terminating null unless it has + // at least one byte, so a zero sized buffer is never meaningful. + assert(size > 0); return (pn_fixed_string_t) {.bytes=bytes, .size=size, .position=0}; } @@ -92,8 +96,12 @@ static inline void pn_fixed_string_quote(pn_fixed_string_t *str, const char *dat } } +static inline bool pn_fixed_string_is_full(pn_fixed_string_t *str) { + return str->position==str->size; +} + static inline void pn_fixed_string_terminate(pn_fixed_string_t *str) { - if (str->position==str->size) str->position--; + if (pn_fixed_string_is_full(str)) str->position--; str->bytes[str->position] = 0; } diff --git a/c/src/core/value_dump.c b/c/src/core/value_dump.c index 33f5df94d..828ce4e39 100644 --- a/c/src/core/value_dump.c +++ b/c/src/core/value_dump.c @@ -41,12 +41,17 @@ static inline pn_bytes_t pn_bytes_advance(pn_bytes_t bytes, size_t size) { // This is only used in places where a described value is not wanted/allowed // So we interpret type to mean the 'base' type of the described value in this case static inline void pni_frame_get_type_value2(pni_consumer_t* consumer, uint8_t* type, pn_bytes_t* value) { + // Skip over a (possibly empty) chain of descriptors. Each descriptor is followed by + // its own code, which is discarded here since only the base type/value is wanted. + uint32_t descriptor_count = 0; if (!pni_consumer_readf8(consumer, type)) goto error; - if (*type==PNE_DESCRIPTOR) { - // Skip over descriptor potentially recursively - uint8_t dtype; - pn_bytes_t dvalue; - pni_frame_get_type_value2(consumer, &dtype, &dvalue); + while (*type==PNE_DESCRIPTOR) { + descriptor_count++; + if (!pni_consumer_readf8(consumer, type)) goto error; + } + for (uint32_t i = 0; i < descriptor_count; i++) { + pn_bytes_t discarded; + if (!pni_consumer_read_value_not_described(consumer, *type, &discarded)) goto error; if (!pni_consumer_readf8(consumer, type)) goto error; } if (!pni_consumer_read_value_not_described(consumer, *type, value)) goto error; @@ -502,6 +507,12 @@ void pn_value_dump_array(uint32_t count, pn_bytes_t value, pn_fixed_string_t *ou } void pn_value_dump_nondescribed_value(uint8_t type, pn_bytes_t value, pn_fixed_string_t *output){ + // Once the output buffer is full there's nothing left to write, so stop here + // rather than walking the rest of the value. + if (pn_fixed_string_is_full(output)) { + return; + } + if (!type_iscompund(type)) { // The one exception to 'scalar' is LIST0 which is encoded as a special type pn_value_dump_scalar(type, value, output); @@ -618,6 +629,10 @@ size_t pni_value_dump(pn_bytes_t frame, pn_fixed_string_t *output) size_t pn_value_dump(pn_bytes_t frame, char *bytes, uint32_t size) { + // With no output buffer there is nowhere to put even the terminating null, + // so nothing is dumped and nothing is consumed. + if (size == 0) return 0; + pn_fixed_string_t output = pn_fixed_string(bytes, size); size_t fsize = pni_value_dump(frame, &output); pn_fixed_string_terminate(&output); diff --git a/c/tests/data_test.cpp b/c/tests/data_test.cpp index 4beccef74..46d651703 100644 --- a/c/tests/data_test.cpp +++ b/c/tests/data_test.cpp @@ -450,6 +450,26 @@ TEST_CASE("data_decode_into_entered_container") { CHECK("[[5, null]]" == inspect(data)); } +TEST_CASE("data_format_no_free_space") { + auto_free<pn_data_t, pn_data_free> data(pn_data(0)); + REQUIRE(pn_data_put_int(data, 5) == 0); + + // No free space at all: there is nowhere to put even the terminating null, + // so there is nothing to format and the buffer must be left alone. As with a + // buffer that is merely too small, this is not reported as an overflow. + char buf[1] = {'x'}; + size_t size = 0; + CHECK(pn_data_format(data, buf, &size) == 0); + CHECK(buf[0] == 'x'); + CHECK(size == 0); + + // One byte is just enough to hold the terminating null of a truncated result. + size = sizeof(buf); + CHECK(pn_data_format(data, buf, &size) == 0); + CHECK(buf[0] == '\0'); + CHECK(size == 0); +} + TEST_CASE("data_map") { auto_free<pn_data_t, pn_data_free> data(pn_data(1)); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
