qpid-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From astitc...@apache.org
Subject [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
Date Thu, 06 Dec 2018 22:23:03 GMT
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 <astitcher@apache.org>
Authored: Wed Dec 5 13:12:50 2018 -0500
Committer: Andrew Stitcher <astitcher@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


Mime
View raw message