Return a decode result from Buffer::copy

src/rmq/rmqamqpt/rmqamqpt_basicack.cpp

@@ -25,6 +25,9 @@
 
 namespace BloombergLP {
 namespace rmqamqpt {
+namespace {
+const uint8_t BITMASK_BIT_MULTIPLE = (1 << 0);
+} // namespace
 
 BasicAck::BasicAck(bsl::uint64_t dt, bool multiple)
 : d_deliveryTag(dt)
@@ -44,19 +47,13 @@ bool BasicAck::decode(BasicAck* ack,
 {
     rmqamqpt::Buffer buffer(data, dataLength);
 
-    if (buffer.available() < sizeof(bdlb::BigEndianUint64)) {
-
-        return false;
-    }
-
-    ack->d_deliveryTag = buffer.copy<bdlb::BigEndianUint64>();
-
-    if (buffer.available() < sizeof(bool)) {
+    uint8_t bitmask = 0;
+    if (!buffer.copy<bdlb::BigEndianUint64>(&ack->d_deliveryTag) ||
+        !buffer.copy<uint8_t>(&bitmask)) {
         return false;
     }
 
-    ack->d_multiple = buffer.copy<bool>();
-
+    ack->d_multiple = bitmask & BITMASK_BIT_MULTIPLE;
     return true;
 }
 

src/rmq/rmqamqpt/rmqamqpt_basiccancel.cpp

@@ -22,6 +22,9 @@
 
 namespace BloombergLP {
 namespace rmqamqpt {
+namespace {
+const uint8_t BITMASK_BIT_NOWAIT = (1 << 0);
+} // namespace
 
 BasicCancel::BasicCancel(const bsl::string& consumerTag, bool noWait)
 : d_consumerTag(consumerTag)
@@ -42,11 +45,12 @@ bool BasicCancel::decode(BasicCancel* cancel,
     if (!Types::decodeShortString(&cancel->d_consumerTag, &buffer)) {
         return false;
     }
-    if (buffer.available() < sizeof(bool)) {
+    uint8_t bitmask = 0;
+    if (!buffer.copy<uint8_t>(&bitmask)) {
         return false;
     }
-    cancel->d_noWait = buffer.copy<bool>();
 
+    cancel->d_noWait = bitmask & BITMASK_BIT_NOWAIT;
     return true;
 }
 

src/rmq/rmqamqpt/rmqamqpt_basicconsume.cpp

@@ -67,23 +67,22 @@ bool BasicConsume::decode(BasicConsume* consume,
 {
     rmqamqpt::Buffer buffer(data, dataLength);
 
-    // uint16_t reserved1;
-    if (sizeof(uint16_t /* reserved1 */) > buffer.available()) {
+    uint16_t reserved1 = 0;
+    if (!buffer.copy<bdlb::BigEndianUint16>(&reserved1)) {
         return false;
     }
-    /*reserved1 = */ buffer.copy<bdlb::BigEndianUint16>();
     if (!Types::decodeShortString(&consume->d_queue, &buffer)) {
         return false;
     }
     if (!Types::decodeShortString(&consume->d_consumerTag, &buffer)) {
         return false;
     }
-    if (sizeof(uint8_t) > buffer.available()) {
+
+    uint8_t theByte = 0;
+    if (!buffer.copy<uint8_t>(&theByte)) {
         return false;
     }
 
-    const uint8_t theByte = buffer.copy<uint8_t>();
-
     consume->d_noLocal   = theByte & MASK_BIT_0;
     consume->d_noAck     = theByte & MASK_BIT_1;
     consume->d_exclusive = theByte & MASK_BIT_2;

src/rmq/rmqamqpt/rmqamqpt_basicdeliver.cpp

@@ -26,7 +26,8 @@ namespace rmqamqpt {
 
 namespace {
 BALL_LOG_SET_NAMESPACE_CATEGORY("RMQAMQPT.BASICDELIVER")
-}
+const uint8_t BITMASK_BIT_REDELIVERED = (1 << 0);
+} // namespace
 
 BasicDeliver::BasicDeliver(const bsl::string& consumerTag,
                            uint64_t deliveryTag,
@@ -60,15 +61,16 @@ bool BasicDeliver::decode(BasicDeliver* deliver,
         return false;
     }
 
-    if (sizeof(deliver->d_deliveryTag) + sizeof(deliver->d_redelivered) >
-        buffer.available()) {
+    uint8_t bitmask = 0;
+    if (!buffer.copy<bdlb::BigEndianUint64>(&deliver->d_deliveryTag) ||
+        !buffer.copy<uint8_t>(&bitmask)) {
         BALL_LOG_ERROR
             << "Not enough data to decode BasicDeliver frame: available "
             << buffer.available();
         return false;
     }
-    deliver->d_deliveryTag = buffer.copy<bdlb::BigEndianUint64>();
-    deliver->d_redelivered = buffer.copy<bool>();
+
+    deliver->d_redelivered = bitmask & BITMASK_BIT_REDELIVERED;
 
     if (!Types::decodeShortString(&deliver->d_exchange, &buffer)) {
         return false;

src/rmq/rmqamqpt/rmqamqpt_basicnack.cpp

@@ -48,17 +48,12 @@ bool BasicNack::decode(BasicNack* nack,
 {
     rmqamqpt::Buffer buffer(data, dataLength);
 
-    if (buffer.available() < sizeof(bdlb::BigEndianUint64)) {
-        return false;
-    }
-
-    nack->d_deliveryTag = buffer.copy<bdlb::BigEndianUint64>();
-
-    if (buffer.available() < sizeof(uint8_t)) {
+    uint8_t bitmask = 0;
+    if (!buffer.copy<bdlb::BigEndianUint64>(&nack->d_deliveryTag) ||
+        !buffer.copy<uint8_t>(&bitmask)) {
         return false;
     }
 
-    uint8_t bitmask  = buffer.copy<uint8_t>();
     nack->d_multiple = bitmask & BITMASK_BIT_0;
     nack->d_requeue  = bitmask & BITMASK_BIT_1;
 

src/rmq/rmqamqpt/rmqamqpt_basicproperties.cpp

@@ -252,11 +252,12 @@ bool BasicProperties::decode(BasicProperties* props,
 {
     rmqamqpt::Buffer buffer(data, dataLength);
 
-    if (sizeof(uint16_t) > buffer.available()) {
+    uint16_t flags = 0;
+    if (!buffer.copy<bdlb::BigEndianUint16>(&flags)) {
         return false;
     }
-    bool success         = true;
-    const uint16_t flags = buffer.copy<bdlb::BigEndianUint16>();
+
+    bool success = true;
 
     rmqt::Properties properties;
 
@@ -288,10 +289,20 @@ bool BasicProperties::decode(BasicProperties* props,
         }
     }
     if (flags & maskForProperty(DELIVERY_MODE)) {
-        properties.deliveryMode = buffer.copy<uint8_t>();
+        properties.deliveryMode = uint8_t();
+        if (!buffer.copy<uint8_t>(&*properties.deliveryMode)) {
+            BALL_LOG_ERROR << "Decoding fail for basic property: "
+                           << PROPERTY_NAMES[DELIVERY_MODE];
+            success = false;
+        }
     }
     if (flags & maskForProperty(PRIORITY)) {
-        properties.priority = buffer.copy<uint8_t>();
+        properties.priority = uint8_t();
+        if (!buffer.copy<uint8_t>(&*properties.priority)) {
+            BALL_LOG_ERROR << "Decoding fail for basic property: "
+                           << PROPERTY_NAMES[PRIORITY];
+            success = false;
+        }
     }
     if (flags & maskForProperty(CORRELATION_ID)) {
         properties.correlationId = "";
@@ -331,7 +342,12 @@ bool BasicProperties::decode(BasicProperties* props,
     }
     if (flags & maskForProperty(TIMESTAMP)) {
         properties.timestamp = bdlt::Datetime();
-        rmqamqpt::Types::decodeTimestamp(&*properties.timestamp, &buffer);
+        if (!rmqamqpt::Types::decodeTimestamp(&*properties.timestamp,
+                                              &buffer)) {
+            BALL_LOG_ERROR << "Decoding fail for basic property: "
+                           << PROPERTY_NAMES[TIMESTAMP];
+            success = false;
+        }
     }
     if (flags & maskForProperty(TYPE)) {
         properties.type = "";

src/rmq/rmqamqpt/rmqamqpt_basicpublish.cpp

@@ -53,22 +53,23 @@ bool BasicPublish::decode(BasicPublish* publish,
                           bsl::size_t dataLength)
 {
     rmqamqpt::Buffer buffer(data, dataLength);
-    if (sizeof(int16_t) > buffer.available()) {
+
+    int16_t reserved1 = 0;
+    if (!buffer.copy<bdlb::BigEndianInt16>(&reserved1)) {
         return false;
     }
-    buffer.copy<bdlb::BigEndianInt16>();
     if (!Types::decodeShortString(&publish->d_exchangeName, &buffer)) {
         return false;
     }
     if (!Types::decodeShortString(&publish->d_routingKey, &buffer)) {
         return false;
     }
-    if (sizeof(uint8_t) > buffer.available()) {
+
+    uint8_t theByte = 0;
+    if (!buffer.copy<uint8_t>(&theByte)) {
         return false;
     }
 
-    const uint8_t theByte = buffer.copy<uint8_t>();
-
     publish->d_mandatory = theByte & MASK_BIT_0;
     publish->d_immediate = theByte & MASK_BIT_1;
 

src/rmq/rmqamqpt/rmqamqpt_basicqos.cpp

@@ -26,7 +26,8 @@ namespace rmqamqpt {
 
 namespace {
 BALL_LOG_SET_NAMESPACE_CATEGORY("RMQAMQPT.BASICQOS")
-}
+const uint8_t BITMASK_BIT_GLOBAL = (1 << 0);
+} // namespace
 
 BasicQoS::BasicQoS(bsl::uint16_t prefetchCount,
                    bsl::uint32_t prefetchSize,
@@ -50,18 +51,16 @@ bool BasicQoS::decode(BasicQoS* qos,
 {
     rmqamqpt::Buffer buffer(data, dataLength);
 
-    if (sizeof(qos->d_prefetchSize) + sizeof(qos->d_prefetchCount) +
-            sizeof(qos->d_global) >
-        buffer.available()) {
-        BALL_LOG_ERROR
-            << "Not enough data to decode ConnectionTune frame: available "
-            << buffer.available();
+    uint8_t bitmask = 0;
+    if (!buffer.copy<bdlb::BigEndianUint32>(&qos->d_prefetchSize) ||
+        !buffer.copy<bdlb::BigEndianUint16>(&qos->d_prefetchCount) ||
+        !buffer.copy<uint8_t>(&bitmask)) {
+        BALL_LOG_ERROR << "Not enough data to decode BasicQoS frame: available "
+                       << buffer.available();
         return false;
     }
 
-    qos->d_prefetchSize  = buffer.copy<bdlb::BigEndianUint32>();
-    qos->d_prefetchCount = buffer.copy<bdlb::BigEndianUint16>();
-    qos->d_global        = buffer.copy<bool>();
+    qos->d_global = bitmask & BITMASK_BIT_GLOBAL;
     return true;
 }
 

src/rmq/rmqamqpt/rmqamqpt_basicreturn.cpp

@@ -52,11 +52,10 @@ bool BasicReturn::decode(BasicReturn* basicReturn,
 {
     rmqamqpt::Buffer buffer(data, dataLength);
 
-    if (buffer.available() < sizeof(basicReturn->d_replyCode)) {
+    uint16_t replyCode = 0;
+    if (!buffer.copy<bdlb::BigEndianUint16>(&replyCode)) {
         return false;
     }
-
-    const uint16_t replyCode = buffer.copy<bdlb::BigEndianUint16>();
     basicReturn->d_replyCode =
         static_cast<rmqamqpt::Constants::AMQPReplyCode>(replyCode);
 

src/rmq/rmqamqpt/rmqamqpt_buffer.cpp

@@ -17,6 +17,8 @@
 
 #include <bsls_assert.h>
 
+#include <bsl_algorithm.h>
+
 namespace BloombergLP {
 namespace rmqamqpt {
 
@@ -30,8 +32,9 @@ Buffer::Buffer(const_pointer start, size_type length)
 
 Buffer Buffer::consume(size_type size)
 {
-    Buffer b(ptr(), size);
-    skip(size);
+    const size_type consumed = bsl::min(size, available());
+    Buffer b(ptr(), consumed);
+    skip(consumed);
     return b;
 }
 

src/rmq/rmqamqpt/rmqamqpt_buffer.h

@@ -19,7 +19,7 @@
 #include <bslmf_assert.h>
 #include <bslmf_isbitwisemoveable.h>
 #include <bslmf_istriviallycopyable.h>
-#include <bsls_assert.h>
+#include <bsls_compilerfeatures.h>
 
 #include <bsl_cstddef.h>
 #include <bsl_cstdint.h>
@@ -48,9 +48,19 @@ class Buffer {
     ~Buffer()             = default;
 #endif
 
-    template <typename TYPE>
-    TYPE copy();
+    /// Read a `TYPE` out of the buffer, assign it to `value` and advance
+    /// past it.
+    ///
+    /// Return false, leaving `value` and the read position untouched, when
+    /// fewer than `sizeof(TYPE)` bytes remain. `TYPE` is the type as it
+    /// appears on the wire and must be given explicitly; `VALUE` is
+    /// deduced, so a big-endian field can be read straight into the
+    /// host-order member it belongs to.
+    template <typename TYPE, typename VALUE>
+    bool copy(VALUE* value);
 
+    /// Return a `Buffer` over the next `size` bytes and advance past them,
+    /// truncating `size` to `available()`.
     Buffer consume(size_type size);
     void skip(size_type size);
     const_pointer ptr() const { return d_data + d_offset; }
@@ -64,16 +74,25 @@ class Buffer {
     offset_type d_offset;
 };
 
-template <typename TYPE>
-TYPE Buffer::copy()
+template <typename TYPE, typename VALUE>
+bool Buffer::copy(VALUE* value)
 {
     BSLMF_ASSERT(bsl::is_trivially_copyable<TYPE>::value);
     BSLMF_ASSERT(bslmf::IsBitwiseMoveable<TYPE>::value);
-    BSLS_ASSERT(available() >= sizeof(TYPE));
+    BSLMF_ASSERT(sizeof(VALUE) == sizeof(TYPE));
+    // AMQP `bit` fields share an octet: read a uint8_t and mask it instead
+    BSLMF_ASSERT((!bsl::is_same<TYPE, bool>::value));
+
+    if (available() < sizeof(TYPE)) {
+        return false;
+    }
+
     TYPE val;
     bsl::memcpy(&val, ptr(), sizeof(TYPE));
     skip(sizeof(TYPE));
-    return val;
+
+    *value = val;
+    return true;
 }
 
 } // namespace rmqamqpt

src/rmq/rmqamqpt/rmqamqpt_channelclose.cpp

@@ -52,26 +52,26 @@ bool ChannelClose::decode(ChannelClose* close,
                           bsl::size_t dataLength)
 {
     rmqamqpt::Buffer buffer(data, dataLength);
-    if (buffer.available() < sizeof(bdlb::BigEndianUint16)) {
+
+    uint16_t replyCode = 0;
+    if (!buffer.copy<bdlb::BigEndianUint16>(&replyCode)) {
         return false;
     }
-
-    const uint16_t replyCode = buffer.copy<bdlb::BigEndianUint16>();
     close->d_replyCode =
         static_cast<rmqamqpt::Constants::AMQPReplyCode>(replyCode);
 
     if (!Types::decodeShortString(&close->d_replyText, &buffer)) {
         return false;
     }
 
-    if (buffer.available() < 2 * sizeof(bdlb::BigEndianUint16)) {
+    uint16_t classId  = 0;
+    uint16_t methodId = 0;
+    if (!buffer.copy<bdlb::BigEndianUint16>(&classId) ||
+        !buffer.copy<bdlb::BigEndianUint16>(&methodId)) {
         return false;
     }
 
-    const uint16_t classId = buffer.copy<bdlb::BigEndianUint16>();
     close->d_classId = static_cast<rmqamqpt::Constants::AMQPClassId>(classId);
-
-    const uint16_t methodId = buffer.copy<bdlb::BigEndianUint16>();
     close->d_methodId =
         static_cast<rmqamqpt::Constants::AMQPMethodId>(methodId);