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 2018/12/06 22:23:03 UTC
[1/2] qpid-proton git commit: PROTON-1978: [c] Make disposition
performative handling more efficient - Minimise the effort to update
deliveries affected by the disposition performative: If there are fewer
deliveries outstanding than delivery ids in t
Repository: qpid-proton
Updated Branches:
refs/heads/master b44e3fffb -> 5ba471d97
PROTON-1978: [c] Make disposition performative handling more efficient
- Minimise the effort to update deliveries affected by the disposition
performative:
If there are fewer deliveries outstanding than delivery ids in the
performative then loop through them all to update them.
Otherwise, loop through the specified delivery ids.
- We have to go through this effort as we don't keep the outstanding
deliveries sorted by id. Instead they are in a hash table and in a
separate linked list.
Problem found by oss-fuzz: https://oss-fuzz.com/testcase?key=5118747114209280
Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/6a5140b4
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/6a5140b4
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/6a5140b4
Branch: refs/heads/master
Commit: 6a5140b4efc2b6c8f92ad2ca8f7ea551b37bcd1c
Parents: b44e3ff
Author: Andrew Stitcher <as...@apache.org>
Authored: Wed Dec 5 13:12:50 2018 -0500
Committer: Andrew Stitcher <as...@apache.org>
Committed: Thu Dec 6 17:21:15 2018 -0500
----------------------------------------------------------------------
c/src/core/transport.c | 133 ++++++++++++-------
.../crash/5118747114209280 | Bin 0 -> 107 bytes
2 files changed, 86 insertions(+), 47 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/6a5140b4/c/src/core/transport.c
----------------------------------------------------------------------
diff --git a/c/src/core/transport.c b/c/src/core/transport.c
index 5dce530..545897a 100644
--- a/c/src/core/transport.c
+++ b/c/src/core/transport.c
@@ -1631,6 +1631,69 @@ static inline bool sequence_lte(pn_sequence_t a, pn_sequence_t b) {
return b-a <= INT32_MAX;
}
+static int pni_do_delivery_disposition(pn_transport_t * transport, pn_delivery_t *delivery, bool settled, bool remote_data, bool type_init, uint64_t type) {
+ pn_disposition_t *remote = &delivery->remote;
+
+ if (type_init) remote->type = type;
+
+ if (remote_data) {
+ switch (type) {
+ case PN_RECEIVED:
+ pn_data_rewind(transport->disp_data);
+ pn_data_next(transport->disp_data);
+ pn_data_enter(transport->disp_data);
+
+ if (pn_data_next(transport->disp_data)) {
+ remote->section_number = pn_data_get_uint(transport->disp_data);
+ }
+ if (pn_data_next(transport->disp_data)) {
+ remote->section_offset = pn_data_get_ulong(transport->disp_data);
+ }
+ break;
+
+ case PN_ACCEPTED:
+ break;
+
+ case PN_REJECTED: {
+ int err = pn_scan_error(transport->disp_data, &remote->condition, SCAN_ERROR_DISP);
+
+ if (err) return err;
+ break;
+ }
+ case PN_RELEASED:
+ break;
+
+ case PN_MODIFIED:
+ pn_data_rewind(transport->disp_data);
+ pn_data_next(transport->disp_data);
+ pn_data_enter(transport->disp_data);
+
+ if (pn_data_next(transport->disp_data)) {
+ remote->failed = pn_data_get_bool(transport->disp_data);
+ }
+ if (pn_data_next(transport->disp_data)) {
+ remote->undeliverable = pn_data_get_bool(transport->disp_data);
+ }
+ pn_data_narrow(transport->disp_data);
+ pn_data_clear(remote->data);
+ pn_data_appendn(remote->annotations, transport->disp_data, 1);
+ pn_data_widen(transport->disp_data);
+ break;
+
+ default:
+ pn_data_copy(remote->data, transport->disp_data);
+ break;
+ }
+ }
+
+ remote->settled = settled;
+ delivery->updated = true;
+ pn_work_update(transport->connection, delivery);
+
+ pn_collector_put(transport->connection->collector, PN_OBJECT, delivery, PN_DELIVERY);
+ return 0;
+}
+
int pn_do_disposition(pn_transport_t *transport, uint8_t frame_type, uint16_t channel, pn_data_t *args, const pn_bytes_t *payload)
{
bool role;
@@ -1649,6 +1712,10 @@ int pn_do_disposition(pn_transport_t *transport, uint8_t frame_type, uint16_t ch
return pn_do_error(transport, "amqp:not-allowed", "no such channel: %u", channel);
}
+ if (!sequence_lte(first, last)) {
+ return pn_do_error(transport, "amqp:not allowed", "illegal delivery range: %x-%x", first, last);
+ }
+
pn_delivery_map_t *deliveries;
if (role) {
deliveries = &ssn->state.outgoing;
@@ -1664,54 +1731,26 @@ int pn_do_disposition(pn_transport_t *transport, uint8_t frame_type, uint16_t ch
// TODO: We should really also clamp the first value here, but we're not keeping track of the earliest
// unsettled delivery sequence no
last = sequence_lte(last, deliveries->next) ? last : deliveries->next;
- first = sequence_lte(first, last) ? first : last;
- for (pn_sequence_t id = first; sequence_lte(id, last); ++id) {
- pn_delivery_t *delivery = pni_delivery_map_get(deliveries, id);
- if (delivery) {
- pn_disposition_t *remote = &delivery->remote;
- if (type_init) remote->type = type;
- if (remote_data) {
- switch (type) {
- case PN_RECEIVED:
- pn_data_rewind(transport->disp_data);
- pn_data_next(transport->disp_data);
- pn_data_enter(transport->disp_data);
- if (pn_data_next(transport->disp_data))
- remote->section_number = pn_data_get_uint(transport->disp_data);
- if (pn_data_next(transport->disp_data))
- remote->section_offset = pn_data_get_ulong(transport->disp_data);
- break;
- case PN_ACCEPTED:
- break;
- case PN_REJECTED:
- err = pn_scan_error(transport->disp_data, &remote->condition, SCAN_ERROR_DISP);
- if (err) return err;
- break;
- case PN_RELEASED:
- break;
- case PN_MODIFIED:
- pn_data_rewind(transport->disp_data);
- pn_data_next(transport->disp_data);
- pn_data_enter(transport->disp_data);
- if (pn_data_next(transport->disp_data))
- remote->failed = pn_data_get_bool(transport->disp_data);
- if (pn_data_next(transport->disp_data))
- remote->undeliverable = pn_data_get_bool(transport->disp_data);
- pn_data_narrow(transport->disp_data);
- pn_data_clear(remote->data);
- pn_data_appendn(remote->annotations, transport->disp_data, 1);
- pn_data_widen(transport->disp_data);
- break;
- default:
- pn_data_copy(remote->data, transport->disp_data);
- break;
- }
- }
- remote->settled = settled;
- delivery->updated = true;
- pn_work_update(transport->connection, delivery);
- pn_collector_put(transport->connection->collector, PN_OBJECT, delivery, PN_DELIVERY);
+ // If there are fewer deliveries in the session than the range then look at every delivery in the session
+ // otherwise look at every delivery_id in the disposition performative
+ pn_hash_t *dh = deliveries->deliveries;
+ if (last-first+1 >= pn_hash_size(dh)) {
+ for (pn_handle_t entry = pn_hash_head(dh); entry!=0 ; entry = pn_hash_next(dh, entry)) {
+ pn_sequence_t key = pn_hash_key(dh, entry);
+ if (sequence_lte(first, key) && sequence_lte(key, last)) {
+ pn_delivery_t *delivery = (pn_delivery_t*) pn_hash_value(dh, entry);
+ err = pni_do_delivery_disposition(transport, delivery, settled, remote_data, type_init, type);
+ if (err) return err;
+ }
+ }
+ } else {
+ for (pn_sequence_t id = first; sequence_lte(id, last); ++id) {
+ pn_delivery_t *delivery = pni_delivery_map_get(deliveries, id);
+ if (delivery) {
+ err = pni_do_delivery_disposition(transport, delivery, settled, remote_data, type_init, type);
+ if (err) return err;
+ }
}
}
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/6a5140b4/c/tests/fuzz/fuzz-connection-driver/crash/5118747114209280
----------------------------------------------------------------------
diff --git a/c/tests/fuzz/fuzz-connection-driver/crash/5118747114209280 b/c/tests/fuzz/fuzz-connection-driver/crash/5118747114209280
new file mode 100644
index 0000000..61c4e12
Binary files /dev/null and b/c/tests/fuzz/fuzz-connection-driver/crash/5118747114209280 differ
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org
[2/2] qpid-proton git commit: PROTON-1979: [c] Forbid AMQP values
that could lead to a nested descriptor type - Any described type descriptors
that could lead to a nested described type in the descriptor type itself are
forbidden as these can lead to i
Posted by as...@apache.org.
PROTON-1979: [c] Forbid AMQP values that could lead to a nested descriptor type
- Any described type descriptors that could lead to a nested described type in the
descriptor type itself are forbidden as these can lead to indefinite stack use.
- In any event only symbol and ulong are currently valid types for descriptors,
all other types are reserved although syntactically valid (according to amqp 1.0).
Problem found by oss-fuzz: https://oss-fuzz.com/testcase?key=5920119225057280
Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/5ba471d9
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/5ba471d9
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/5ba471d9
Branch: refs/heads/master
Commit: 5ba471d97f9e04c8e68f2270681038b3c1eac0ed
Parents: 6a5140b
Author: Andrew Stitcher <as...@apache.org>
Authored: Thu Dec 6 14:45:47 2018 -0500
Committer: Andrew Stitcher <as...@apache.org>
Committed: Thu Dec 6 17:22:19 2018 -0500
----------------------------------------------------------------------
c/src/core/decoder.c | 7 ++++++-
.../fuzz-message-decode/crash/5920119225057280 | Bin 0 -> 96383 bytes
2 files changed, 6 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/5ba471d9/c/src/core/decoder.c
----------------------------------------------------------------------
diff --git a/c/src/core/decoder.c b/c/src/core/decoder.c
index fd7b69a..a2e99b2 100644
--- a/c/src/core/decoder.c
+++ b/c/src/core/decoder.c
@@ -485,6 +485,11 @@ static int pni_decoder_decode_type(pn_decoder_t *decoder, pn_data_t *data, uint8
size_t pn_data_siblings(pn_data_t *data);
+static inline bool pni_allowed_descriptor_code(uint8_t code)
+{
+ return code != PNE_DESCRIPTOR && code != PNE_ARRAY8 && code != PNE_ARRAY32;
+}
+
int pni_decoder_single_described(pn_decoder_t *decoder, pn_data_t *data)
{
if (!pn_decoder_remaining(decoder)) {
@@ -493,7 +498,7 @@ int pni_decoder_single_described(pn_decoder_t *decoder, pn_data_t *data)
uint8_t code = *decoder->position++;;
- if (code == PNE_DESCRIPTOR) {
+ if (!pni_allowed_descriptor_code(code)) {
return PN_ARG_ERR;
}
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/5ba471d9/c/tests/fuzz/fuzz-message-decode/crash/5920119225057280
----------------------------------------------------------------------
diff --git a/c/tests/fuzz/fuzz-message-decode/crash/5920119225057280 b/c/tests/fuzz/fuzz-message-decode/crash/5920119225057280
new file mode 100644
index 0000000..3a4657d
Binary files /dev/null and b/c/tests/fuzz/fuzz-message-decode/crash/5920119225057280 differ
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org