You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by as...@apache.org on 2026/09/14 22:21:48 UTC
(qpid-proton) branch main updated (caf0f45c5 -> 0ec4d3970)
This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a change to branch main
in repository https://gitbox.apache.org/repos/asf/qpid-proton.git
from caf0f45c5 PROTON-2960: Don't pass uncast 0 as null pointer into pn_data_fill
new 364e39511 PROTON-2954: Simplify pn_data_t
new c1c48bcb3 PROTON-2966: Improve the error reporting from pn_decoder_decode
new aaf4c2ce7 PROTON-2967: Decode AMQP values iteratively rather than recursively
new 040c0993f PROTON-2968: pn_decoder_decode rejects too large compound counts
new 6e237cc6c PROTON-2969: Stop dumping a value once the output buffer is full
new 0ec4d3970 PROTON-2970: Validate the SASL state before handling a frame
The 6 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
c/src/core/codec.c | 56 +++----
c/src/core/data.h | 110 ++++++------
c/src/core/decoder.c | 417 ++++++++++++++++++++++++++++------------------
c/src/core/decoder.h | 1 -
c/src/core/encoder.c | 24 +--
c/src/core/fixed_string.h | 10 +-
c/src/core/value_dump.c | 25 ++-
c/src/sasl/sasl.c | 8 +
c/tests/data_test.cpp | 243 +++++++++++++++++++++++++++
9 files changed, 617 insertions(+), 277 deletions(-)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org
(qpid-proton) 02/06: PROTON-2966: Improve the error reporting from pn_decoder_decode
Posted by as...@apache.org.
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 c1c48bcb3c559644969ba48c4cec130894224165
Author: Andrew Stitcher <as...@apache.org>
AuthorDate: Fri Aug 28 00:40:37 2026 -0400
PROTON-2966: Improve the error reporting from pn_decoder_decode
---
c/src/core/decoder.c | 38 ++++++++++++++++++++------------------
c/src/core/decoder.h | 1 -
2 files changed, 20 insertions(+), 19 deletions(-)
diff --git a/c/src/core/decoder.c b/c/src/core/decoder.c
index 64d471fe8..5d03212f2 100644
--- a/c/src/core/decoder.c
+++ b/c/src/core/decoder.c
@@ -27,23 +27,15 @@
#include <string.h>
-static inline pn_error_t *pni_decoder_error(pn_decoder_t *decoder)
-{
- if (!decoder->error) decoder->error = pn_error();
- return decoder->error;
-}
-
void pn_decoder_initialize(pn_decoder_t *decoder)
{
decoder->input = NULL;
decoder->size = 0;
decoder->position = NULL;
- decoder->error = NULL;
}
void pn_decoder_finalize(pn_decoder_t *decoder)
{
- pn_error_free(decoder->error);
}
static inline uint8_t pn_decoder_readf8(pn_decoder_t *decoder)
@@ -315,7 +307,7 @@ static int pni_decoder_decode_value(pn_decoder_t *decoder, pn_data_t *data, uint
size = pn_decoder_readf32(decoder);
break;
default:
- return PN_ARG_ERR;
+ return pn_error_format(pn_data_error(data), PN_ARG_ERR, "unrecognized variable-width typecode: %u", code);
}
if (pn_decoder_remaining(decoder) < size) return PN_UNDERFLOW;
@@ -335,7 +327,7 @@ static int pni_decoder_decode_value(pn_decoder_t *decoder, pn_data_t *data, uint
err = pn_data_put_symbol(data, bytes);
break;
default:
- return PN_ARG_ERR;
+ return pn_error_format(pn_data_error(data), PN_ARG_ERR, "unrecognized variable-width typecode: %u", code);
}
}
@@ -362,7 +354,11 @@ static int pni_decoder_decode_value(pn_decoder_t *decoder, pn_data_t *data, uint
if (pn_decoder_remaining(decoder) < min_expected_size+1) return PN_UNDERFLOW;
size = pn_decoder_readf8(decoder);
// size must be at least big enough for count or count+constructor
- if (size < min_expected_size) return PN_ARG_ERR;
+ if (size < min_expected_size) {
+ return pn_error_format(pn_data_error(data), PN_ARG_ERR,
+ "%s size %zu too small to hold its own header",
+ pn_type_name(pn_code2type(code)), size);
+ }
if (pn_decoder_remaining(decoder) < size) return PN_UNDERFLOW;
count = pn_decoder_readf8(decoder);
break;
@@ -375,12 +371,16 @@ static int pni_decoder_decode_value(pn_decoder_t *decoder, pn_data_t *data, uint
if (pn_decoder_remaining(decoder) < min_expected_size+4) return PN_UNDERFLOW;
size = pn_decoder_readf32(decoder);
// size must be at least big enough for count or count+constructor
- if (size < min_expected_size) return PN_ARG_ERR;
+ if (size < min_expected_size) {
+ return pn_error_format(pn_data_error(data), PN_ARG_ERR,
+ "%s size %zu too small to hold its own header",
+ pn_type_name(pn_code2type(code)), size);
+ }
if (pn_decoder_remaining(decoder) < size) return PN_UNDERFLOW;
count = pn_decoder_readf32(decoder);
break;
default:
- return PN_ARG_ERR;
+ return pn_error_format(pn_data_error(data), PN_ARG_ERR, "internal error");
}
switch (code)
@@ -398,7 +398,9 @@ static int pni_decoder_decode_value(pn_decoder_t *decoder, pn_data_t *data, uint
int e = pni_decoder_decode_type(decoder, data, &acode);
if (e) return e;
pn_type_t type = pn_code2type(acode);
- if ((int)type < 0) return (int)type;
+ if ((int)type < 0) {
+ return pn_error_format(pn_data_error(data), (int) type, "unrecognized array element typecode: %u", acode);
+ }
for (size_t i = 0; i < count; i++)
{
e = pni_decoder_decode_value(decoder, data, acode);
@@ -420,7 +422,7 @@ static int pni_decoder_decode_value(pn_decoder_t *decoder, pn_data_t *data, uint
if (err) return err;
break;
default:
- return PN_ARG_ERR;
+ return pn_error_format(pn_data_error(data), PN_ARG_ERR, "internal error");
}
pn_data_enter(data);
for (size_t i = 0; i < count; i++)
@@ -433,7 +435,7 @@ static int pni_decoder_decode_value(pn_decoder_t *decoder, pn_data_t *data, uint
return 0;
}
default:
- return pn_error_format(pni_decoder_error(decoder), PN_ARG_ERR, "unrecognized typecode: %u", code);
+ return pn_error_format(pn_data_error(data), PN_ARG_ERR, "unrecognized typecode: %u", code);
}
return err;
@@ -497,7 +499,7 @@ int pni_decoder_single_described(pn_decoder_t *decoder, pn_data_t *data)
uint8_t code = *decoder->position++;;
if (!pni_allowed_descriptor_code(code)) {
- return PN_ARG_ERR;
+ return pn_error_format(pn_data_error(data), PN_ARG_ERR, "invalid descriptor value typecode: %u", code);
}
int err = pni_decoder_decode_value(decoder, data, code);
@@ -530,7 +532,7 @@ ssize_t pn_decoder_decode(pn_decoder_t *decoder, const char *src, size_t size, p
int err = pni_decoder_single(decoder, dst);
- if (err == PN_UNDERFLOW)
+ if (err == PN_UNDERFLOW)
return pn_error_format(pn_data_error(dst), PN_UNDERFLOW, "not enough data to decode");
if (err) return err;
diff --git a/c/src/core/decoder.h b/c/src/core/decoder.h
index 70bc4c8a8..bd870959d 100644
--- a/c/src/core/decoder.h
+++ b/c/src/core/decoder.h
@@ -26,7 +26,6 @@ typedef struct pn_decoder_t {
const char *input;
size_t size;
const char *position;
- pn_error_t *error;
} pn_decoder_t;
void pn_decoder_initialize(pn_decoder_t *decoder);
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org
(qpid-proton) 05/06: PROTON-2969: Stop dumping a value once the output buffer is full
Posted by as...@apache.org.
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 <as...@apache.org>
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 <no...@anthropic.com>
---
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: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org
(qpid-proton) 01/06: PROTON-2954: Simplify pn_data_t
Posted by as...@apache.org.
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 364e3951121e2181e42324748a863c3fc22e4713
Author: Andrew Stitcher <as...@apache.org>
AuthorDate: Sun Aug 30 01:19:34 2026 +0000
PROTON-2954: Simplify pn_data_t
- The different compound types now all have the same node data
- Call out the scratch space in compound nodes explicitly
---
c/src/core/codec.c | 30 ++++++++++----------
c/src/core/data.h | 78 ++++++++++++----------------------------------------
c/src/core/encoder.c | 24 ++++++++--------
3 files changed, 44 insertions(+), 88 deletions(-)
diff --git a/c/src/core/codec.c b/c/src/core/codec.c
index 75cc49e04..2b1f861a1 100644
--- a/c/src/core/codec.c
+++ b/c/src/core/codec.c
@@ -98,7 +98,7 @@ static const pn_fields_t *pni_node_fields(pn_data_t *data, pni_node_t *node)
if (!node) return NULL;
if (node->type != PN_DESCRIBED) return NULL;
- pni_node_t *descriptor = pn_data_node(data, node->u.as_described.down);
+ pni_node_t *descriptor = pn_data_node(data, node->u.as_compound.down);
if (!descriptor || descriptor->type != PN_ULONG) {
return NULL;
@@ -822,8 +822,8 @@ int pn_data_vfill(pn_data_t *data, const char *fmt, va_list ap)
pni_nid_t down = current->u.as_deferred.down;
uint16_t count = current->u.as_deferred.children_count;
current->type = PN_DESCRIBED;
- current->u.as_described.down = down;
- current->u.as_described.children_count = count;
+ current->u.as_compound.down = down;
+ current->u.as_compound.children_count = count;
continue;
} else if (parent->u.as_deferred.type == PN_NULL && parent->u.as_deferred.children_count == 1) {
pn_data_exit(data);
@@ -1648,8 +1648,8 @@ int pn_data_put_list(pn_data_t *data)
pni_node_t *node = pni_data_add(data);
if (node == NULL) return PN_OUT_OF_MEMORY;
node->type = PN_LIST;
- node->u.as_list.down = 0;
- node->u.as_list.children_count = 0;
+ node->u.as_compound.down = 0;
+ node->u.as_compound.children_count = 0;
return 0;
}
@@ -1658,8 +1658,8 @@ int pn_data_put_map(pn_data_t *data)
pni_node_t *node = pni_data_add(data);
if (node == NULL) return PN_OUT_OF_MEMORY;
node->type = PN_MAP;
- node->u.as_map.down = 0;
- node->u.as_map.children_count = 0;
+ node->u.as_compound.down = 0;
+ node->u.as_compound.children_count = 0;
return 0;
}
@@ -1669,8 +1669,8 @@ int pn_data_put_array(pn_data_t *data, bool described, pn_type_t type)
if (node == NULL) return PN_OUT_OF_MEMORY;
node->type = described ? PN_ARRAY_DESCRIBED : PN_ARRAY;
node->array_type = (uint8_t)type;
- node->u.as_array.down = 0;
- node->u.as_array.children_count = 0;
+ node->u.as_compound.down = 0;
+ node->u.as_compound.children_count = 0;
return 0;
}
@@ -1687,8 +1687,8 @@ int pn_data_put_described(pn_data_t *data)
pni_node_t *node = pni_data_add(data);
if (node == NULL) return PN_OUT_OF_MEMORY;
node->type = PN_DESCRIBED;
- node->u.as_described.down = 0;
- node->u.as_described.children_count = 0;
+ node->u.as_compound.down = 0;
+ node->u.as_compound.children_count = 0;
return 0;
}
@@ -1903,7 +1903,7 @@ size_t pn_data_get_list(pn_data_t *data)
{
pni_node_t *node = pni_data_current(data);
if (node && node->type == PN_LIST) {
- return node->u.as_list.children_count;
+ return node->u.as_compound.children_count;
} else {
return 0;
}
@@ -1913,7 +1913,7 @@ size_t pn_data_get_map(pn_data_t *data)
{
pni_node_t *node = pni_data_current(data);
if (node && node->type == PN_MAP) {
- return node->u.as_map.children_count;
+ return node->u.as_compound.children_count;
} else {
return 0;
}
@@ -1925,10 +1925,10 @@ size_t pn_data_get_array(pn_data_t *data)
if (!node) return 0;
if (node->type == PN_ARRAY_DESCRIBED) {
- return node->u.as_array.children_count - 1; // Descriptor doesn't count
+ return node->u.as_compound.children_count - 1; // Descriptor doesn't count
}
if (node->type == PN_ARRAY) {
- return node->u.as_array.children_count;
+ return node->u.as_compound.children_count;
}
return 0;
}
diff --git a/c/src/core/data.h b/c/src/core/data.h
index 4dee470e9..d7338010b 100644
--- a/c/src/core/data.h
+++ b/c/src/core/data.h
@@ -41,17 +41,12 @@ typedef uint16_t pni_nid_t;
* buffer (data->buf); as_bytes.offset and as_bytes.size locate the bytes.
* DECIMAL128 and UUID always have as_bytes.size == 16.
*
- * PN_ARRAY nodes use as_array: element type, whether the array has a
- * descriptor child (described), and encoder scratch (start, small).
+ * Compound nodes share a single layout for down/children and scratch state:
+ * PN_ARRAY, PN_ARRAY_DESCRIBED, PN_LIST, PN_MAP and PN_DESCRIBED all use
+ * as_compound.
*
- * PN_LIST nodes use as_list: whether the list is the body of a described
- * value (controls trailing-null elision during encoding), and encoder
- * scratch (start, small).
- *
- * PN_MAP nodes use as_map: encoder scratch (start, small).
- *
- * All other types (NULL, DESCRIBED) carry no payload; only the type tag
- * on pni_node_t is meaningful.
+ * All other types carry no payload; only the type tag on pni_node_t is
+ * meaningful.
*/
typedef union {
bool as_bool;
@@ -79,29 +74,14 @@ typedef union {
uint8_t type; /* deferred type */
} as_deferred;
- // Compound types include navigation
+ // Compound types share the same navigation and scratch layout.
struct {
- pni_nid_t down; // offset 0: 2 bytes
- pni_nid_t children_count; // offset 2: 2 bytes
- uint32_t start; // offset 4: 4 bytes
- } as_array; // 8 bytes
-
- struct {
- pni_nid_t down; // offset 0: 2 bytes
- pni_nid_t children_count; // offset 2: 2 bytes
- uint32_t start; // offset 4: 4 bytes
- } as_list; // 8 bytes
-
- struct {
- pni_nid_t down; // offset 0: 2 bytes
- pni_nid_t children_count; // offset 2: 2 bytes
- uint32_t start; // offset 4: 4 bytes
- } as_map; // 8 bytes
-
- struct {
- pni_nid_t down; // offset 0: 2 bytes
- pni_nid_t children_count; // offset 2: 2 bytes
- } as_described; // 4 bytes (union is 8)
+ pni_nid_t down; // offset 0: 2 bytes
+ pni_nid_t children_count; // offset 2: 2 bytes
+ union {
+ uint32_t as_u32;
+ } scratch; // offset 4: 4 bytes
+ } as_compound; // 8 bytes
} pni_node_payload_t;
/*
@@ -173,13 +153,10 @@ static inline pni_nid_t pni_node_get_down(pni_node_t *node)
switch (node->type) {
case PN_ARRAY:
case PN_ARRAY_DESCRIBED:
- return node->u.as_array.down;
case PN_LIST:
- return node->u.as_list.down;
case PN_MAP:
- return node->u.as_map.down;
case PN_DESCRIBED:
- return node->u.as_described.down;
+ return node->u.as_compound.down;
case PN_DEFER:
return node->u.as_deferred.down;
default:
@@ -193,16 +170,10 @@ static inline void pni_node_set_down(pni_node_t *node, pni_nid_t down)
switch (node->type) {
case PN_ARRAY:
case PN_ARRAY_DESCRIBED:
- node->u.as_array.down = down;
- break;
case PN_LIST:
- node->u.as_list.down = down;
- break;
case PN_MAP:
- node->u.as_map.down = down;
- break;
case PN_DESCRIBED:
- node->u.as_described.down = down;
+ node->u.as_compound.down = down;
break;
case PN_DEFER:
node->u.as_deferred.down = down;
@@ -218,13 +189,10 @@ static inline pni_nid_t pni_node_get_children(pni_node_t *node)
switch (node->type) {
case PN_ARRAY:
case PN_ARRAY_DESCRIBED:
- return node->u.as_array.children_count;
case PN_LIST:
- return node->u.as_list.children_count;
case PN_MAP:
- return node->u.as_map.children_count;
case PN_DESCRIBED:
- return node->u.as_described.children_count;
+ return node->u.as_compound.children_count;
case PN_DEFER:
return node->u.as_deferred.children_count;
default:
@@ -238,16 +206,10 @@ static inline void pni_node_set_children(pni_node_t *node, pni_nid_t count)
switch (node->type) {
case PN_ARRAY:
case PN_ARRAY_DESCRIBED:
- node->u.as_array.children_count = count;
- break;
case PN_LIST:
- node->u.as_list.children_count = count;
- break;
case PN_MAP:
- node->u.as_map.children_count = count;
- break;
case PN_DESCRIBED:
- node->u.as_described.children_count = count;
+ node->u.as_compound.children_count = count;
break;
case PN_DEFER:
node->u.as_deferred.children_count = count;
@@ -263,16 +225,10 @@ static inline void pni_node_inc_children(pni_node_t *node)
switch (node->type) {
case PN_ARRAY:
case PN_ARRAY_DESCRIBED:
- node->u.as_array.children_count++;
- break;
case PN_LIST:
- node->u.as_list.children_count++;
- break;
case PN_MAP:
- node->u.as_map.children_count++;
- break;
case PN_DESCRIBED:
- node->u.as_described.children_count++;
+ node->u.as_compound.children_count++;
break;
case PN_DEFER:
node->u.as_deferred.children_count++;
diff --git a/c/src/core/encoder.c b/c/src/core/encoder.c
index 502a3a86f..f3c9f7225 100644
--- a/c/src/core/encoder.c
+++ b/c/src/core/encoder.c
@@ -328,25 +328,25 @@ static int pni_encoder_enter(void *ctx, pn_data_t *data, pni_node_t *node)
case PNE_SYM8: { pn_bytes_t b = {node->u.as_bytes.size, buf.start + node->u.as_bytes.offset}; pn_encoder_writev8(encoder, &b); return 0; }
case PNE_SYM32: { pn_bytes_t b = {node->u.as_bytes.size, buf.start + node->u.as_bytes.offset}; pn_encoder_writev32(encoder, &b); return 0; }
case PNE_ARRAY32:
- node->u.as_array.start = (uint32_t) encoder->position;
+ node->u.as_compound.scratch.as_u32 = (uint32_t) encoder->position;
// we'll backfill the size on exit
encoder->position += 4;
bool described = (node->type == PN_ARRAY_DESCRIBED);
- pn_encoder_writef32(encoder, described ? node->u.as_array.children_count - 1 : node->u.as_array.children_count);
+ pn_encoder_writef32(encoder, described ? node->u.as_compound.children_count - 1 : node->u.as_compound.children_count);
if (described)
pn_encoder_writef8(encoder, 0);
return 0;
case PNE_LIST32:
- node->u.as_list.start = (uint32_t) encoder->position;
+ node->u.as_compound.scratch.as_u32 = (uint32_t) encoder->position;
// we'll backfill the size later
encoder->position += 4;
- pn_encoder_writef32(encoder, node->u.as_list.children_count);
+ pn_encoder_writef32(encoder, node->u.as_compound.children_count);
return 0;
case PNE_MAP32:
- node->u.as_map.start = (uint32_t) encoder->position;
+ node->u.as_compound.scratch.as_u32 = (uint32_t) encoder->position;
// we'll backfill the size later
encoder->position += 4;
- pn_encoder_writef32(encoder, node->u.as_map.children_count);
+ pn_encoder_writef32(encoder, node->u.as_compound.children_count);
return 0;
default:
return pn_error_format(pn_data_error(data), PN_ERR, "unrecognized encoding: %u", code);
@@ -362,8 +362,8 @@ static int pni_encoder_exit(void *ctx, pn_data_t *data, pni_node_t *node)
// Special case 0 length list, but not as element in an array
pni_node_t *parent = pn_data_node(data, node->parent);
- if (node->type==PN_LIST && node->u.as_list.children_count-encoder->null_count==0 && !pn_is_in_array(data, parent, node)) {
- encoder->position = node->u.as_list.start - 1; // position of list opcode
+ if (node->type==PN_LIST && node->u.as_compound.children_count-encoder->null_count==0 && !pn_is_in_array(data, parent, node)) {
+ encoder->position = node->u.as_compound.scratch.as_u32 - 1; // position of list opcode
pn_encoder_writef8(encoder, PNE_LIST0);
encoder->null_count = 0;
return 0;
@@ -372,16 +372,16 @@ static int pni_encoder_exit(void *ctx, pn_data_t *data, pni_node_t *node)
switch (node->type) {
case PN_ARRAY:
case PN_ARRAY_DESCRIBED:
- if ((node->type == PN_ARRAY_DESCRIBED && node->u.as_array.children_count == 1) ||
- (node->type == PN_ARRAY && node->u.as_array.children_count == 0)) {
+ if ((node->type == PN_ARRAY_DESCRIBED && node->u.as_compound.children_count == 1) ||
+ (node->type == PN_ARRAY && node->u.as_compound.children_count == 0)) {
pn_encoder_writef8(encoder, pn_type2code(encoder, node->array_type));
}
PN_FALLTHROUGH;
case PN_LIST:
case PN_MAP: {
pos = encoder->position;
- /* start is at the same offset in as_array/as_list/as_map — use as_list uniformly */
- uint32_t start = node->u.as_list.start;
+ /* all compound node payloads share the same navigation and scratch layout */
+ uint32_t start = node->u.as_compound.scratch.as_u32;
/* content_size: bytes written after the 8-byte *32 header (size+count fields) */
size_t content_size = pos - start - 8;
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org
(qpid-proton) 06/06: PROTON-2970: Validate the SASL state before handling a frame
Posted by as...@apache.org.
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 0ec4d39707285d12608e6597671747c541ba245d
Author: Andrew Stitcher <as...@apache.org>
AuthorDate: Mon Aug 24 21:43:15 2026 -0400
PROTON-2970: Validate the SASL state before handling a frame
Each SASL body handler checked that the frame was one its role could receive - but not that it was one it was expecting at this point in the protocol exchange.
Check the state of the exchange as well. A server takes an init only once it
has posted its mechanisms, and a response only once it has posted a
challenge. A client takes a mechanisms frame only before it has posted
anything, and a challenge or an outcome only once it has posted an init or a
response. Anything else is PN_ERR, as a frame arriving at the wrong role
already was.
The state tested is desired_state rather than last_state. A pipelining peer
can legitimately send its next frame before we have actually written our own,
so last_state can still be lagging behind the frame we have already committed
to sending.
Assisted-By: Claude Opus 5 <no...@anthropic.com>
---
c/src/sasl/sasl.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/c/src/sasl/sasl.c b/c/src/sasl/sasl.c
index 3e639de58..d757c8adc 100644
--- a/c/src/sasl/sasl.c
+++ b/c/src/sasl/sasl.c
@@ -904,6 +904,10 @@ int pn_do_init(pn_transport_t *transport, uint8_t frame_type, uint16_t channel,
// We should only receive this if we are a sasl server
if (sasl->client) return PN_ERR;
+ // Check the protocol state using desired_state not last_state: a pipelining peer can
+ // legitimately send its next frame before we've actually written out our own, so
+ // last_state can still be lagging behind the frame we've already committed to sending.
+ if (sasl->desired_state != SASL_POSTED_MECHANISMS) return PN_ERR;
pn_bytes_t mech;
pn_bytes_t recv;
@@ -936,6 +940,7 @@ int pn_do_mechanisms(pn_transport_t *transport, uint8_t frame_type, uint16_t cha
// We should only receive this if we are a sasl client
if (!sasl->client) return PN_ERR;
+ if (sasl->desired_state != SASL_NONE) return PN_ERR;
pn_string_t *mechs = pn_string("");
@@ -1004,6 +1009,7 @@ int pn_do_challenge(pn_transport_t *transport, uint8_t frame_type, uint16_t chan
// We should only receive this if we are a sasl client
if (!sasl->client) return PN_ERR;
+ if (sasl->desired_state != SASL_POSTED_INIT && sasl->desired_state != SASL_POSTED_RESPONSE) return PN_ERR;
pn_bytes_t recv;
@@ -1025,6 +1031,7 @@ int pn_do_response(pn_transport_t *transport, uint8_t frame_type, uint16_t chann
// We should only receive this if we are a sasl server
if (sasl->client) return PN_ERR;
+ if (sasl->desired_state != SASL_POSTED_CHALLENGE) return PN_ERR;
pn_bytes_t recv;
@@ -1046,6 +1053,7 @@ int pn_do_outcome(pn_transport_t *transport, uint8_t frame_type, uint16_t channe
// We should only receive this if we are a sasl client
if (!sasl->client) return PN_ERR;
+ if (sasl->desired_state != SASL_POSTED_INIT && sasl->desired_state != SASL_POSTED_RESPONSE) return PN_ERR;
uint8_t outcome;
pn_bytes_t recv;
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org
(qpid-proton) 03/06: PROTON-2967: Decode AMQP values iteratively rather than recursively
Posted by as...@apache.org.
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 aaf4c2ce7669c19453bc09c8f49147b73ed37f5e
Author: Andrew Stitcher <as...@apache.org>
AuthorDate: Mon Aug 24 21:43:23 2026 -0400
PROTON-2967: Decode AMQP values iteratively rather than recursively
pn_decoder_decode() recursed once per level of nesting, so the C stack it
consumed grew with the nesting depth of the value being decoded.
Decode iteratively instead. The state a recursive decoder keeps in its stack
frames - how many children of the enclosing container are still to come, and,
for an array, the constructor its elements share - is now kept in the
container's own node, in the scratch space the encoder also uses. The tree
being built is therefore also the decoder's stack, so the only bound on
nesting is the node array: PNI_NID_MAX, and any limit set with
pn_data_set_decode_limits().
Every open node - list, map, array and described alike - carries a count of
the children still to be decoded, which lets a single loop close each node as
its last child arrives.
The AMQP standard is contradictory about whether described types
are allowed to be directly nested:
In the descriptive text for described format code it says "prmitive
format code" which would disallow a described type. The formal syntax
allows it, merely specifying "constructor". The section on transactions
requires the message body to the transaction coordinator to be an amqp
value (a described type) with a direct value of another described type
for declaring and discharging transactions.
So to sanity check incoming AMQP values we only allow a single directly
nested described type. This is all that is required per the standard,
and it's hard to see a valid use for more deeply nested described types.
Assisted-By: Claude Opus 5 <no...@anthropic.com>
---
c/src/core/codec.c | 14 +-
c/src/core/data.h | 27 +++-
c/src/core/decoder.c | 403 +++++++++++++++++++++++++++++---------------------
c/tests/data_test.cpp | 187 +++++++++++++++++++++++
4 files changed, 453 insertions(+), 178 deletions(-)
diff --git a/c/src/core/codec.c b/c/src/core/codec.c
index 2b1f861a1..e90449b9b 100644
--- a/c/src/core/codec.c
+++ b/c/src/core/codec.c
@@ -1457,16 +1457,6 @@ pn_type_t pn_data_type(pn_data_t *data)
}
}
-pn_type_t pni_data_parent_type(pn_data_t *data)
-{
- pni_node_t *node = pn_data_node(data, data->parent);
- if (node) {
- return node->type;
- } else {
- return PN_INVALID;
- }
-}
-
size_t pn_data_siblings(pn_data_t *data)
{
pni_node_t *node = pn_data_node(data, data->parent);
@@ -1674,9 +1664,9 @@ int pn_data_put_array(pn_data_t *data, bool described, pn_type_t type)
return 0;
}
-void pni_data_set_array_type(pn_data_t *data, pn_type_t type)
+void pni_data_set_parent_array_type(pn_data_t *data, pn_type_t type)
{
- pni_node_t *array = pni_data_current(data);
+ pni_node_t *array = pn_data_node(data, data->parent);
if (array) {
array->array_type = (uint8_t)type;
}
diff --git a/c/src/core/data.h b/c/src/core/data.h
index d7338010b..0189be0fe 100644
--- a/c/src/core/data.h
+++ b/c/src/core/data.h
@@ -34,6 +34,18 @@ typedef uint16_t pni_nid_t;
#define PN_ARRAY_DESCRIBED 26 // Internal type: described array
#define PN_DEFER 27 // Internal type: node used only in pn_data_fill/vfill
+/*
+ * Bookkeeping the decoder keeps in a compound node while it is being decoded.
+ * It is dead once the node's last child has been decoded, so it can live in
+ * the node's scratch space. remaining is a pni_nid_t because a node can never
+ * have more children than the node array can hold.
+ */
+typedef struct {
+ pni_nid_t remaining; /* children still to be decoded */
+ uint8_t typecode; /* constructor shared by all elements (arrays only) */
+ uint8_t unused;
+} pni_decoder_state_t;
+
/*
* Value payload for a pni_node_t.
*
@@ -79,7 +91,8 @@ typedef union {
pni_nid_t down; // offset 0: 2 bytes
pni_nid_t children_count; // offset 2: 2 bytes
union {
- uint32_t as_u32;
+ uint32_t as_u32; /* encoder: where the node's header was written */
+ pni_decoder_state_t as_decoder_state; /* decoder: what is left to decode */
} scratch; // offset 4: 4 bytes
} as_compound; // 8 bytes
} pni_node_payload_t;
@@ -147,6 +160,14 @@ static inline pni_node_t * pn_data_node(pn_data_t *data, pni_nid_t nd)
return nd ? (data->nodes + nd - 1) : NULL;
}
+/* The type of the node we are currently inside, PN_INVALID at the top level.
+ * This can be an internal type, e.g. PN_ARRAY_DESCRIBED. */
+static inline pn_type_t pni_data_parent_type(pn_data_t *data)
+{
+ pni_node_t *node = pn_data_node(data, data->parent);
+ return node ? (pn_type_t) node->type : PN_INVALID;
+}
+
static inline pni_nid_t pni_node_get_down(pni_node_t *node)
{
if (!node) return 0;
@@ -238,6 +259,10 @@ static inline void pni_node_inc_children(pni_node_t *node)
}
}
+/* Set the element type of the array we are currently inside (data->parent).
+ * The decoder must create an array node before it has read the constructor
+ * that gives the element type, so it fills the type in afterwards. */
+void pni_data_set_parent_array_type(pn_data_t *data, pn_type_t type);
int pni_data_traverse(pn_data_t *data,
int (*enter)(void *ctx, pn_data_t *data, pni_node_t *node),
int (*exit)(void *ctx, pn_data_t *data, pni_node_t *node),
diff --git a/c/src/core/decoder.c b/c/src/core/decoder.c
index 5d03212f2..a525ecb1c 100644
--- a/c/src/core/decoder.c
+++ b/c/src/core/decoder.c
@@ -27,6 +27,21 @@
#include <string.h>
+/*
+ * Decoding is iterative rather than recursive.
+ *
+ * Nesting depth is bounded only by the input, so a decoder that recursed once
+ * per level would consume C stack in proportion to it. The state that a
+ * recursive decoder would keep in its stack frames - how many children of the
+ * enclosing container are still to come, and, for an array, the constructor
+ * its elements share - is instead kept in the container's own node, in the
+ * scratch space the encoder also uses (pni_decoder_state()).
+ *
+ * The tree being built is therefore also the decoder's stack: the only bound
+ * on nesting is the node array, which is bounded by PNI_NID_MAX and by any
+ * limit set with pn_data_set_decode_limits().
+ */
+
void pn_decoder_initialize(pn_decoder_t *decoder)
{
decoder->input = NULL;
@@ -87,14 +102,6 @@ static inline size_t pn_decoder_remaining(pn_decoder_t *decoder)
return decoder->input + decoder->size - decoder->position;
}
-typedef union {
- uint32_t i;
- uint32_t a[2];
- uint64_t l;
- float f;
- double d;
-} conv_t;
-
static inline pn_type_t pn_code2type(uint8_t code)
{
switch (code)
@@ -169,19 +176,80 @@ static inline pn_type_t pn_code2type(uint8_t code)
}
}
-static int pni_decoder_decode_type(pn_decoder_t *decoder, pn_data_t *data, uint8_t *code);
-static int pni_decoder_single_described(pn_decoder_t *decoder, pn_data_t *data);
-static int pni_decoder_single(pn_decoder_t *decoder, pn_data_t *data);
-void pni_data_set_array_type(pn_data_t *data, pn_type_t type);
+// Typecodes that introduce children which have to be decoded in turn.
+// PNE_LIST0 is not one of them: it is an empty list, so it decodes in a single
+// step like a scalar.
+static inline bool pni_decoder_is_container_code(uint8_t code)
+{
+ switch (code)
+ {
+ case PNE_ARRAY8:
+ case PNE_ARRAY32:
+ case PNE_LIST8:
+ case PNE_LIST32:
+ case PNE_MAP8:
+ case PNE_MAP32:
+ return true;
+ default:
+ return false;
+ }
+}
+
+// Everything else (bar the descriptor prefix) decodes to a single childless node.
+static inline bool pni_decoder_is_scalar_code(uint8_t code)
+{
+ return code != PNE_DESCRIPTOR && !pni_decoder_is_container_code(code);
+}
+
+/*
+ * The decoder's stack.
+ *
+ * "Open" nodes are the container and described nodes we have entered and not
+ * yet left; depth counts them and is a local of pni_decoder_decode_value().
+ * The innermost open node - the one we are putting children into - is
+ * data->parent, and it carries the state saying what is left to decode.
+ *
+ * NB the returned pointer is invalidated by any pn_data_put_*(), which may
+ * reallocate the node array; always re-fetch it after putting a node.
+ */
+static inline pni_decoder_state_t *pni_decoder_state(pn_data_t *data)
+{
+ return &pn_data_node(data, data->parent)->u.as_compound.scratch.as_decoder_state;
+}
+
+// One fewer child for the open node to wait for. Called before putting that
+// child, so a node's count reaches 0 exactly as its last child is added.
+static inline void pni_decoder_dec_remaining_children(pn_data_t *data, unsigned depth)
+{
+ if (depth > 0) pni_decoder_state(data)->remaining--;
+}
+
+// Array elements are encoded without a constructor of their own, so they are
+// decoded differently from every other value.
+static inline bool pni_decoder_in_array(pn_data_t *data, unsigned depth)
+{
+ if (depth == 0) return false;
+ pn_type_t type = pni_data_parent_type(data);
+ return type == PN_ARRAY || type == PN_ARRAY_DESCRIBED;
+}
+
+typedef union {
+ uint32_t i;
+ uint32_t a[2];
+ uint64_t l;
+ float f;
+ double d;
+} conv_t;
-static int pni_decoder_decode_value(pn_decoder_t *decoder, pn_data_t *data, uint8_t code)
+// Decode a value that has no children to decode: any scalar, or an empty list.
+// The constructor has already been read; code is it.
+static int pni_decoder_decode_scalar(pn_decoder_t *decoder, pn_data_t *data, uint8_t code)
{
int err;
conv_t conv;
pn_decimal128_t dec128;
pn_uuid_t uuid;
size_t size;
- size_t count;
switch (code)
{
@@ -336,104 +404,6 @@ static int pni_decoder_decode_value(pn_decoder_t *decoder, pn_data_t *data, uint
case PNE_LIST0:
err = pn_data_put_list(data);
break;
- case PNE_ARRAY8:
- case PNE_ARRAY32:
- case PNE_LIST8:
- case PNE_LIST32:
- case PNE_MAP8:
- case PNE_MAP32: {
- size_t min_expected_size = 0;
- switch (code)
- {
- case PNE_ARRAY8:
- min_expected_size += 1; // Array has a constructor of at least 1 byte
- PN_FALLTHROUGH;
- case PNE_LIST8:
- case PNE_MAP8:
- min_expected_size += 1; // All these types have a count
- if (pn_decoder_remaining(decoder) < min_expected_size+1) return PN_UNDERFLOW;
- size = pn_decoder_readf8(decoder);
- // size must be at least big enough for count or count+constructor
- if (size < min_expected_size) {
- return pn_error_format(pn_data_error(data), PN_ARG_ERR,
- "%s size %zu too small to hold its own header",
- pn_type_name(pn_code2type(code)), size);
- }
- if (pn_decoder_remaining(decoder) < size) return PN_UNDERFLOW;
- count = pn_decoder_readf8(decoder);
- break;
- case PNE_ARRAY32:
- min_expected_size += 1; // Array has a constructor of at least 1 byte
- PN_FALLTHROUGH;
- case PNE_LIST32:
- case PNE_MAP32:
- min_expected_size += 4; // All these types have a count
- if (pn_decoder_remaining(decoder) < min_expected_size+4) return PN_UNDERFLOW;
- size = pn_decoder_readf32(decoder);
- // size must be at least big enough for count or count+constructor
- if (size < min_expected_size) {
- return pn_error_format(pn_data_error(data), PN_ARG_ERR,
- "%s size %zu too small to hold its own header",
- pn_type_name(pn_code2type(code)), size);
- }
- if (pn_decoder_remaining(decoder) < size) return PN_UNDERFLOW;
- count = pn_decoder_readf32(decoder);
- break;
- default:
- return pn_error_format(pn_data_error(data), PN_ARG_ERR, "internal error");
- }
-
- switch (code)
- {
- case PNE_ARRAY8:
- case PNE_ARRAY32:
- {
- uint8_t next = *decoder->position;
- bool described = (next == PNE_DESCRIPTOR);
- err = pn_data_put_array(data, described, (pn_type_t) 0);
- if (err) return err;
-
- pn_data_enter(data);
- uint8_t acode;
- int e = pni_decoder_decode_type(decoder, data, &acode);
- if (e) return e;
- pn_type_t type = pn_code2type(acode);
- if ((int)type < 0) {
- return pn_error_format(pn_data_error(data), (int) type, "unrecognized array element typecode: %u", acode);
- }
- for (size_t i = 0; i < count; i++)
- {
- e = pni_decoder_decode_value(decoder, data, acode);
- if (e) return e;
- }
- pn_data_exit(data);
-
- pni_data_set_array_type(data, type);
- }
- return 0;
- case PNE_LIST8:
- case PNE_LIST32:
- err = pn_data_put_list(data);
- if (err) return err;
- break;
- case PNE_MAP8:
- case PNE_MAP32:
- err = pn_data_put_map(data);
- if (err) return err;
- break;
- default:
- return pn_error_format(pn_data_error(data), PN_ARG_ERR, "internal error");
- }
- pn_data_enter(data);
- for (size_t i = 0; i < count; i++)
- {
- int e = pni_decoder_single(decoder, data);
- if (e) return e;
- }
- pn_data_exit(data);
-
- return 0;
- }
default:
return pn_error_format(pn_data_error(data), PN_ARG_ERR, "unrecognized typecode: %u", code);
}
@@ -441,87 +411,190 @@ static int pni_decoder_decode_value(pn_decoder_t *decoder, pn_data_t *data, uint
return err;
}
-pn_type_t pni_data_parent_type(pn_data_t *data);
-
-static int pni_decoder_decode_type(pn_decoder_t *decoder, pn_data_t *data, uint8_t *code)
+// Decode the value of a descriptor. Descriptors are restricted to scalars: a
+// compound descriptor buys nothing and is a nesting path we would rather not
+// have to bound.
+static int pni_decoder_decode_descriptor(pn_decoder_t *decoder, pn_data_t *data)
{
- int err;
+ if (!pn_decoder_remaining(decoder)) return PN_UNDERFLOW;
+
+ uint8_t code = *decoder->position++;
- if (!pn_decoder_remaining(decoder)) {
- return PN_UNDERFLOW;
+ if (!pni_decoder_is_scalar_code(code)) {
+ return pn_error_format(pn_data_error(data), PN_ARG_ERR, "invalid descriptor value typecode: %u", code);
}
- uint8_t next = *decoder->position++;
+ return pni_decoder_decode_scalar(decoder, data, code);
+}
- if (next != PNE_DESCRIPTOR) {
- *code = next;
- return 0;
- }
+// How many descriptors may prefix a single value: @d1:@d2:value is accepted,
+// another level of chaining is not.
+#define PNI_DECODER_MAX_DESCRIPTORS 2
- pn_type_t parent_type = pni_data_parent_type(data);
- if (parent_type != PN_ARRAY && parent_type != PN_ARRAY_DESCRIBED) {
- err = pn_data_put_described(data);
- if (err) return err;
+/*
+ * Read the constructor of the next value: the descriptors prefixing it, if
+ * any, and then its format code, which is returned in *code.
+ *
+ * Each descriptor puts a PN_DESCRIBED node and enters it. Such a node holds
+ * exactly two children: the descriptor value, decoded here, and the value it
+ * describes. The node is left open for that value, which the caller decodes
+ * and which closes the node.
+ */
+static int pni_decoder_decode_constructor(pn_decoder_t *decoder, pn_data_t *data,
+ unsigned *depth, uint8_t *code)
+{
+ unsigned descriptors = 0;
+
+ while (true) {
+ if (!pn_decoder_remaining(decoder)) return PN_UNDERFLOW;
+
+ uint8_t next = *decoder->position++;
+ if (next != PNE_DESCRIPTOR) {
+ *code = next;
+ return 0;
+ }
+
+ if (++descriptors > PNI_DECODER_MAX_DESCRIPTORS) {
+ return pn_error_format(pn_data_error(data), PN_ARG_ERR, "nested described type depth exceeded");
+ }
- // pni_decoder_single has the corresponding exit
+ pni_decoder_dec_remaining_children(data, *depth);
+ int err = pn_data_put_described(data);
+ if (err) return err;
pn_data_enter(data);
+ (*depth)++;
+
+ // Of the node's two children the descriptor is decoded right here, leaving
+ // just the value it describes for the caller.
+ pni_decoder_state(data)->remaining = 1;
+ err = pni_decoder_decode_descriptor(decoder, data);
+ if (err) return err;
}
+}
- err = pni_decoder_single_described(decoder, data);
- if (err) return err;
+/*
+ * Put an array node, enter it and read the constructor its elements share.
+ *
+ * An array may be prefixed by a descriptor, which describes the array as a
+ * whole rather than any one element. It is held as the array node's first
+ * child, so it is added before the element count is recorded and does not
+ * count towards it.
+ */
+static int pni_decoder_open_array(pn_decoder_t *decoder, pn_data_t *data, unsigned *depth, pni_nid_t count)
+{
+ // The header check in pni_decoder_open_container leaves at least the one
+ // constructor byte an array must have, so this peek is in bounds.
+ bool described = (*decoder->position == PNE_DESCRIPTOR);
- err = pni_decoder_decode_type(decoder, data, code);
+ int err = pn_data_put_array(data, described, (pn_type_t) 0);
if (err) return err;
+ pn_data_enter(data);
+ (*depth)++;
- return 0;
-}
+ if (described) {
+ decoder->position++;
+ err = pni_decoder_decode_descriptor(decoder, data);
+ if (err) return err;
+ }
-size_t pn_data_siblings(pn_data_t *data);
+ if (!pn_decoder_remaining(decoder)) return PN_UNDERFLOW;
+ uint8_t element_code = *decoder->position++;
-// We disallow using any compound type as a described descriptor to avoid recursion
-// in decoding. Although these seem syntactically valid they don't seem to be of any
-// conceivable use!
-static inline bool pni_allowed_descriptor_code(uint8_t code)
-{
- return
- code != PNE_DESCRIPTOR &&
- code != PNE_ARRAY8 && code != PNE_ARRAY32 &&
- code != PNE_LIST8 && code != PNE_LIST32 &&
- code != PNE_MAP8 && code != PNE_MAP32;
+ if (element_code == PNE_DESCRIPTOR) {
+ return pn_error_format(pn_data_error(data), PN_ARG_ERR,
+ "chained descriptor not supported for a described array");
+ }
+
+ pn_type_t element_type = pn_code2type(element_code);
+ if ((int) element_type < 0) {
+ return pn_error_format(pn_data_error(data), (int) element_type,
+ "unrecognized array element typecode: %u", element_code);
+ }
+ // The array node had to be created before its element type could be known.
+ pni_data_set_parent_array_type(data, element_type);
+
+ pni_decoder_state_t *state = pni_decoder_state(data);
+ state->remaining = count;
+ state->typecode = element_code;
+ return 0;
}
-int pni_decoder_single_described(pn_decoder_t *decoder, pn_data_t *data)
+/*
+ * Read a container header - the byte size and the child count - then put the
+ * container node and enter it. Its children are decoded by the main loop; the
+ * state left in the node says how many of them are still to come.
+ */
+static int pni_decoder_open_container(pn_decoder_t *decoder, pn_data_t *data, unsigned *depth, uint8_t code)
{
- if (!pn_decoder_remaining(decoder)) {
- return PN_UNDERFLOW;
+ const pn_type_t type = pn_code2type(code); // PN_LIST, PN_MAP or PN_ARRAY
+ size_t width; // bytes in each of the size and count fields
+
+ switch (code)
+ {
+ case PNE_LIST8: case PNE_MAP8: case PNE_ARRAY8: width = 1; break;
+ case PNE_LIST32: case PNE_MAP32: case PNE_ARRAY32: width = 4; break;
+ default:
+ return pn_error_format(pn_data_error(data), PN_ARG_ERR, "internal error");
}
- uint8_t code = *decoder->position++;;
+ // What the size field has to cover: the count, and for an array at least one
+ // byte of the constructor its elements share.
+ const size_t min_size = width + (type == PN_ARRAY ? 1 : 0);
- if (!pni_allowed_descriptor_code(code)) {
- return pn_error_format(pn_data_error(data), PN_ARG_ERR, "invalid descriptor value typecode: %u", code);
+ if (pn_decoder_remaining(decoder) < width + min_size) return PN_UNDERFLOW;
+ size_t size = (width == 1) ? pn_decoder_readf8(decoder) : pn_decoder_readf32(decoder);
+ if (size < min_size) {
+ return pn_error_format(pn_data_error(data), PN_ARG_ERR,
+ "%s size %zu too small to hold its own header",
+ pn_type_name(type), size);
}
+ if (pn_decoder_remaining(decoder) < size) return PN_UNDERFLOW;
+ size_t count = (width == 1) ? pn_decoder_readf8(decoder) : pn_decoder_readf32(decoder);
- int err = pni_decoder_decode_value(decoder, data, code);
- if (err) return err;
+ if (type == PN_ARRAY) return pni_decoder_open_array(decoder, data, depth, (pni_nid_t) count);
- if (pni_data_parent_type(data) == PN_DESCRIBED && pn_data_siblings(data) > 1) {
- pn_data_exit(data);
- }
+ int err = (type == PN_LIST) ? pn_data_put_list(data) : pn_data_put_map(data);
+ if (err) return err;
+ pn_data_enter(data);
+ (*depth)++;
+ pni_decoder_state(data)->remaining = (pni_nid_t) count;
return 0;
}
-int pni_decoder_single(pn_decoder_t *decoder, pn_data_t *data)
+// Decode one complete value - with all of its descendants - into data.
+static int pni_decoder_decode_value(pn_decoder_t *decoder, pn_data_t *data)
{
- uint8_t code;
- int err = pni_decoder_decode_type(decoder, data, &code);
- if (err) return err;
- err = pni_decoder_decode_value(decoder, data, code);
- if (err) return err;
- if (pni_data_parent_type(data) == PN_DESCRIBED && pn_data_siblings(data) > 1) {
- pn_data_exit(data);
+ unsigned depth = 0; // open nodes: how deep into the value we currently are
+
+ while (true)
+ {
+ uint8_t code = 0;
+ int err;
+
+ if (pni_decoder_in_array(data, depth)) {
+ code = pni_decoder_state(data)->typecode; // every element shares it
+ } else {
+ err = pni_decoder_decode_constructor(decoder, data, &depth, &code);
+ if (err) return err;
+ }
+
+ // Whatever we decode next is one of the children the open node is waiting for.
+ pni_decoder_dec_remaining_children(data, depth);
+
+ err = pni_decoder_is_container_code(code)
+ ? pni_decoder_open_container(decoder, data, &depth, code)
+ : pni_decoder_decode_scalar(decoder, data, code);
+ if (err) return err;
+
+ // Leave every node that now has all of its children - a container opened
+ // empty is complete as soon as it is opened. Back at depth 0 the one value
+ // we were asked for is complete.
+ while (depth > 0 && pni_decoder_state(data)->remaining == 0) {
+ pn_data_exit(data);
+ depth--;
+ }
+ if (depth == 0) return 0;
}
- return 0;
}
ssize_t pn_decoder_decode(pn_decoder_t *decoder, const char *src, size_t size, pn_data_t *dst)
@@ -530,7 +603,7 @@ ssize_t pn_decoder_decode(pn_decoder_t *decoder, const char *src, size_t size, p
decoder->size = size;
decoder->position = src;
- int err = pni_decoder_single(decoder, dst);
+ int err = pni_decoder_decode_value(decoder, dst);
if (err == PN_UNDERFLOW)
return pn_error_format(pn_data_error(dst), PN_UNDERFLOW, "not enough data to decode");
diff --git a/c/tests/data_test.cpp b/c/tests/data_test.cpp
index b7a2c7675..4a6e04d20 100644
--- a/c/tests/data_test.cpp
+++ b/c/tests/data_test.cpp
@@ -22,14 +22,31 @@
#include "./pn_test.hpp"
#include "core/data.h"
+#include "core/value_dump.h"
#include <proton/codec.h>
#include <proton/error.h>
#include <cstdarg>
+#include <cstdint>
+#include <cstring>
+#include <string>
+#include <vector>
using namespace pn_test;
+// Compare the semantic content of two encoded AMQP byte buffers using pn_value_dump(),
+// the same textifying path used for frame tracing. This tolerates the encoder legitimately
+// choosing a different (but equivalent) encoding width on re-encode, e.g. LIST0 instead of
+// an empty LIST8.
+static void check_roundtrip(pn_bytes_t initial, pn_bytes_t final_bytes) {
+ char initial_buf[256];
+ char final_buf[256];
+ pn_value_dump(initial, initial_buf, sizeof(initial_buf));
+ pn_value_dump(final_bytes, final_buf, sizeof(final_buf));
+ CHECK(std::string(initial_buf) == std::string(final_buf));
+}
+
// Check that pn_data_set_decode_limits() enforces a node-count cap.
TEST_CASE("data_decode_node_limit") {
auto_free<pn_data_t, pn_data_free> data(pn_data(0));
@@ -227,6 +244,176 @@ TEST_CASE("data_described_list") {
CHECK("@open(16) [channel-max=965], \"extra\"" == inspect(data));
}
+TEST_CASE("data_decode_single_nested_described_type") {
+ auto_free<pn_data_t, pn_data_free> data(pn_data(0));
+
+ // described(amqp-value) whose value is itself a described list
+ // 0x00 0x53 0x77 0x00 0x53 0x31 0x45
+ const uint8_t encoded[] = {
+ 0x00, 0x53, 0x77,
+ 0x00, 0x53, 0x31,
+ 0x45
+ };
+
+ ssize_t dec = pn_data_decode(data, (const char *) encoded, sizeof(encoded));
+ CHECK(dec == (ssize_t) sizeof(encoded));
+ CHECK(pn_data_errno(data) == 0);
+
+ char roundtrip[32];
+ int enc = pn_data_encode(data, roundtrip, sizeof(roundtrip));
+ REQUIRE(enc > 0);
+ check_roundtrip(pn_bytes(sizeof(encoded), (const char *) encoded), pn_bytes((size_t) enc, roundtrip));
+}
+
+TEST_CASE("data_decode_rejects_deep_described_chain") {
+ auto_free<pn_data_t, pn_data_free> data(pn_data(0));
+
+ // three chained described constructors: only one nested described value is allowed.
+ const uint8_t encoded[] = {
+ 0x00, 0x53, 0x77,
+ 0x00, 0x53, 0x31,
+ 0x00, 0x53, 0x32,
+ 0x40
+ };
+
+ ssize_t dec = pn_data_decode(data, (const char *) encoded, sizeof(encoded));
+ CHECK(dec == PN_ARG_ERR);
+}
+
+TEST_CASE("data_decode_described_empty_list") {
+ auto_free<pn_data_t, pn_data_free> data(pn_data(0));
+
+ // @ulong(0x77):list() where the empty list is encoded as LIST8 (not the
+ // compact LIST0 form), to exercise the container-open/immediately-empty path.
+ // 0x00 0x53 0x77 0xc0 0x01 0x00
+ const uint8_t encoded[] = {
+ 0x00, 0x53, 0x77, 0xc0, 0x01, 0x00
+ };
+
+ ssize_t dec = pn_data_decode(data, (const char *) encoded, sizeof(encoded));
+ CHECK(dec == (ssize_t) sizeof(encoded));
+ CHECK(pn_data_errno(data) == 0);
+
+ char roundtrip[32];
+ int enc = pn_data_encode(data, roundtrip, sizeof(roundtrip));
+ REQUIRE(enc > 0);
+ check_roundtrip(pn_bytes(sizeof(encoded), (const char *) encoded), pn_bytes((size_t) enc, roundtrip));
+}
+
+TEST_CASE("data_decode_described_empty_list_in_list") {
+ auto_free<pn_data_t, pn_data_free> data(pn_data(0));
+
+ // [ @ulong(0x77):list(), 5 ]
+ const uint8_t encoded[] = {
+ 0xc0, 0x09, 0x02,
+ 0x00, 0x53, 0x77, 0xc0, 0x01, 0x00,
+ 0x52, 0x05
+ };
+
+ ssize_t dec = pn_data_decode(data, (const char *) encoded, sizeof(encoded));
+ CHECK(dec == (ssize_t) sizeof(encoded));
+ CHECK(pn_data_errno(data) == 0);
+ CHECK("[@amqp-value(119) [], 5]" == inspect(data));
+
+ char roundtrip[32];
+ int enc = pn_data_encode(data, roundtrip, sizeof(roundtrip));
+ REQUIRE(enc > 0);
+ check_roundtrip(pn_bytes(sizeof(encoded), (const char *) encoded), pn_bytes((size_t) enc, roundtrip));
+}
+
+TEST_CASE("data_decode_described_empty_map") {
+ auto_free<pn_data_t, pn_data_free> data(pn_data(0));
+
+ // @ulong(0x77):map() where the empty map is encoded as MAP8.
+ // 0x00 0x53 0x77 0xc1 0x01 0x00
+ const uint8_t encoded[] = {
+ 0x00, 0x53, 0x77, 0xc1, 0x01, 0x00
+ };
+
+ ssize_t dec = pn_data_decode(data, (const char *) encoded, sizeof(encoded));
+ CHECK(dec == (ssize_t) sizeof(encoded));
+ CHECK(pn_data_errno(data) == 0);
+
+ char roundtrip[32];
+ int enc = pn_data_encode(data, roundtrip, sizeof(roundtrip));
+ REQUIRE(enc > 0);
+ check_roundtrip(pn_bytes(sizeof(encoded), (const char *) encoded), pn_bytes((size_t) enc, roundtrip));
+}
+
+// Hand-build `depth` nested AMQP list32 frames as raw wire bytes: [0xd0][size:4 BE][count:4 BE],
+// innermost count 0, each outer one wrapping the next as its single element. This lets us drive
+// the decoder to a nesting depth no pn_data_t-based construction can conveniently reach, without
+// relying on any recursive helper on the encode side.
+static std::vector<uint8_t> build_nested_list32(uint32_t depth) {
+ const size_t per = 9; // 1 tag + 4 size + 4 count
+ std::vector<uint8_t> buf(depth * per);
+ uint8_t *p = buf.data();
+ for (uint32_t i = 0; i < depth; i++) {
+ uint32_t count = (i == depth - 1) ? 0 : 1;
+ uint32_t size = 4 + (uint32_t) ((depth - 1 - i) * per); // covers count(4) + nested content
+ *p++ = 0xd0; // PNE_LIST32
+ p[0] = (uint8_t) (size >> 24); p[1] = (uint8_t) (size >> 16); p[2] = (uint8_t) (size >> 8); p[3] = (uint8_t) size;
+ p += 4;
+ p[0] = (uint8_t) (count >> 24); p[1] = (uint8_t) (count >> 16); p[2] = (uint8_t) (count >> 8); p[3] = (uint8_t) count;
+ p += 4;
+ }
+ return buf;
+}
+
+TEST_CASE("data_decode_deep_nesting") {
+ auto_free<pn_data_t, pn_data_free> data(pn_data(0));
+
+ // Depth well beyond what any fixed recursion-depth heuristic would allow, but
+ // comfortably under the absolute PNI_NID_MAX node-id ceiling (2^16 - 1), so the
+ // only limit in play is the (disabled) node-count limit below. The iterative
+ // decoder keeps the nesting in its own heap-backed node storage, so a value
+ // this deeply nested decodes normally.
+ const uint32_t depth = 20000;
+ std::vector<uint8_t> buf = build_nested_list32(depth);
+
+ pn_data_set_decode_limits(data, 0, 0); // unlimited: isolate depth handling from node budget
+ ssize_t dec = pn_data_decode(data, (const char *) buf.data(), buf.size());
+ CHECK(dec == (ssize_t) buf.size());
+ CHECK(pn_data_errno(data) == 0);
+}
+
+TEST_CASE("data_decode_rejects_deeply_nested_values_on_node_limit") {
+ auto_free<pn_data_t, pn_data_free> data(pn_data(0));
+
+ // Same deep-nesting shape as above, but now with a tiny node budget: decoding
+ // must fail cleanly with PN_OUT_OF_MEMORY rather than fail some other way or
+ // succeed, proving that node count is what bounds nesting.
+ const uint32_t depth = 20000;
+ std::vector<uint8_t> buf = build_nested_list32(depth);
+
+ pn_data_set_decode_limits(data, 10, 0);
+ ssize_t dec = pn_data_decode(data, (const char *) buf.data(), buf.size());
+ CHECK(dec == PN_OUT_OF_MEMORY);
+ CHECK(pn_data_errno(data) == PN_OUT_OF_MEMORY);
+}
+
+TEST_CASE("data_decode_into_entered_container") {
+ auto_free<pn_data_t, pn_data_free> data(pn_data(0));
+
+ // Decoding appends to wherever the pn_data_t is positioned, including inside
+ // a container the caller has entered: exactly one value is decoded and the
+ // caller's position is left as it was.
+ REQUIRE(pn_data_put_list(data) == 0);
+ REQUIRE(pn_data_enter(data));
+
+ const uint8_t encoded[] = {
+ 0xc0, 0x04, 0x02, 0x52, 0x05, 0x40 // [5, null]
+ };
+
+ ssize_t dec = pn_data_decode(data, (const char *) encoded, sizeof(encoded));
+ CHECK(dec == (ssize_t) sizeof(encoded));
+ CHECK(pn_data_errno(data) == 0);
+
+ REQUIRE(pn_data_exit(data));
+ pn_data_rewind(data);
+ CHECK("[[5, null]]" == inspect(data));
+}
+
TEST_CASE("data_map") {
auto_free<pn_data_t, pn_data_free> data(pn_data(1));
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org
(qpid-proton) 04/06: PROTON-2968: pn_decoder_decode rejects too large compound counts
Posted by as...@apache.org.
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 040c0993f3b85205607740f2c847ea7ef754693d
Author: Andrew Stitcher <as...@apache.org>
AuthorDate: Fri Sep 11 16:08:57 2026 -0400
PROTON-2968: pn_decoder_decode rejects too large compound counts
Elements of a zero width type (null, true, ...) take up no input bytes at all, so the child count together with the input size in an array of these does not bound the size of the decoded array.
A compound with more children than the allowed number of nodes in a pn_data_t couldn't fit in the node array, so reject the count outright with PN_OUT_OF_MEMORY, which is what a decode that ran out of nodes would report anyway.
Assisted-By: Claude Opus 5 <no...@anthropic.com>
---
c/src/core/codec.c | 7 +++----
c/src/core/data.h | 7 +++++++
c/src/core/decoder.c | 10 ++++++++++
c/tests/data_test.cpp | 36 ++++++++++++++++++++++++++++++++++++
4 files changed, 56 insertions(+), 4 deletions(-)
diff --git a/c/src/core/codec.c b/c/src/core/codec.c
index e90449b9b..63d20d1fb 100644
--- a/c/src/core/codec.c
+++ b/c/src/core/codec.c
@@ -486,8 +486,7 @@ void pn_data_clear(pn_data_t *data)
static int pni_data_grow(pn_data_t *data)
{
- /* Resolve the effective ceiling: the user-set limit, or the hard uint16 max. */
- pni_nid_t effective_max = data->max_nid ? data->max_nid : PNI_NID_MAX;
+ pni_nid_t effective_max = pni_data_max_nid(data);
/* The limit is on logical nodes in use (size), not pre-allocated capacity.
* pni_data_new() is the only caller and it only calls us when size >= capacity,
@@ -1292,8 +1291,8 @@ static size_t pni_data_id(pn_data_t *data, pni_node_t *node)
static pni_node_t *pni_data_new(pn_data_t *data)
{
- /* Enforce max_nid limit on logical node count, regardless of pre-allocated capacity. */
- if (data->max_nid > 0 && data->size >= data->max_nid) {
+ /* Enforce the node limit on logical node count, regardless of pre-allocated capacity. */
+ if (data->size >= pni_data_max_nid(data)) {
pn_error_set(pni_data_error(data), PN_OUT_OF_MEMORY, "pn_data node limit exceeded");
return NULL;
}
diff --git a/c/src/core/data.h b/c/src/core/data.h
index 0189be0fe..37b70f306 100644
--- a/c/src/core/data.h
+++ b/c/src/core/data.h
@@ -155,6 +155,13 @@ struct pn_data_t {
#define PNI_DATA_DEFAULT_MAX_NODES 1024
#define PNI_DATA_BODY_MAX_NODES 0
+/* The most nodes this pn_data_t is allowed to hold: the limit set with
+ * pn_data_set_decode_limits(), or the hard pni_nid_t ceiling if none is set. */
+static inline pni_nid_t pni_data_max_nid(pn_data_t *data)
+{
+ return data->max_nid ? data->max_nid : PNI_NID_MAX;
+}
+
static inline pni_node_t * pn_data_node(pn_data_t *data, pni_nid_t nd)
{
return nd ? (data->nodes + nd - 1) : NULL;
diff --git a/c/src/core/decoder.c b/c/src/core/decoder.c
index a525ecb1c..59681dabf 100644
--- a/c/src/core/decoder.c
+++ b/c/src/core/decoder.c
@@ -551,6 +551,16 @@ static int pni_decoder_open_container(pn_decoder_t *decoder, pn_data_t *data, un
if (pn_decoder_remaining(decoder) < size) return PN_UNDERFLOW;
size_t count = (width == 1) ? pn_decoder_readf8(decoder) : pn_decoder_readf32(decoder);
+ // Array elements of a zero width type (null, true, ...) take no input bytes
+ // at all, so a count is not bounded by the size the way a list's is. Reject
+ // any count that could never fit in this pn_data_t - whose node budget may be
+ // well below the hard ceiling - rather than truncating it.
+ if (count > pni_data_max_nid(data)) {
+ return pn_error_format(pn_data_error(data), PN_OUT_OF_MEMORY,
+ "%s count %zu exceeds the pn_data node limit",
+ pn_type_name(type), count);
+ }
+
if (type == PN_ARRAY) return pni_decoder_open_array(decoder, data, depth, (pni_nid_t) count);
int err = (type == PN_LIST) ? pn_data_put_list(data) : pn_data_put_map(data);
diff --git a/c/tests/data_test.cpp b/c/tests/data_test.cpp
index 4a6e04d20..4beccef74 100644
--- a/c/tests/data_test.cpp
+++ b/c/tests/data_test.cpp
@@ -392,6 +392,42 @@ TEST_CASE("data_decode_rejects_deeply_nested_values_on_node_limit") {
CHECK(pn_data_errno(data) == PN_OUT_OF_MEMORY);
}
+TEST_CASE("data_decode_rejects_element_count_beyond_node_limit") {
+ auto_free<pn_data_t, pn_data_free> data(pn_data(0));
+
+ // array32 of null: null elements are zero width, so the declared count is not
+ // bounded by the declared size and can name more elements than the node array
+ // could ever hold. That must be rejected.
+ // 0xe0 size=0x00000005 count=0x00010000 0x40(null)
+ const uint8_t encoded[] = {
+ 0xf0, 0x00, 0x00, 0x00, 0x05, 0x00, 0x01, 0x00, 0x00, 0x40
+ };
+
+ pn_data_set_decode_limits(data, 0, 0); // unlimited: only the hard node-id ceiling applies
+ ssize_t dec = pn_data_decode(data, (const char *) encoded, sizeof(encoded));
+ CHECK(dec == PN_OUT_OF_MEMORY);
+ CHECK(pn_data_errno(data) == PN_OUT_OF_MEMORY);
+}
+
+TEST_CASE("data_decode_rejects_element_count_beyond_configured_node_limit") {
+ auto_free<pn_data_t, pn_data_free> data(pn_data(0));
+
+ // Same shape, but with a count (100) that is under the hard node-id ceiling
+ // and only over the limit this pn_data_t was configured with. The count is
+ // measured against that limit, so the array is rejected from its header
+ // rather than after the budget has been spent decoding its elements.
+ // 0xf0 size=0x00000005 count=0x00000064 0x40(null)
+ const uint8_t encoded[] = {
+ 0xf0, 0x00, 0x00, 0x00, 0x05, 0x00, 0x00, 0x00, 0x64, 0x40
+ };
+
+ pn_data_set_decode_limits(data, 10, 0);
+ ssize_t dec = pn_data_decode(data, (const char *) encoded, sizeof(encoded));
+ CHECK(dec == PN_OUT_OF_MEMORY);
+ CHECK(pn_data_errno(data) == PN_OUT_OF_MEMORY);
+ CHECK(pn_data_size(data) == 0); // rejected from the header, before any node was put
+}
+
TEST_CASE("data_decode_into_entered_container") {
auto_free<pn_data_t, pn_data_free> data(pn_data(0));
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org