From d0b5189d1e3fdaa5ea6c2346713dde8e1eb4e0b4 Mon Sep 17 00:00:00 2001
From: "Peter A. Bigot" <pab@pabigot.com>
Date: Fri, 20 Apr 2018 18:20:17 -0500
Subject: [PATCH] Envelope: ensure const objects cannot be changed through
 _body

In commit 00b81949d3736470362e9f5b5324b8e1973867bb where Message and
Envelope objects were made uncopiable the Envelope _body member was
changed from const char * to char * and a const_cast introduced to
remove the qualifier from the pointer passed to the constructor.

This technically produces undefined behavior when constructing an
Envelope() to publish data from a const buffer.  It also risks future
bugs if a new subclass of Envelope mutates const objects through the
_body pointer.

Envelope does not need to support mutable or owned content.  Move this
capability down to Message which is used in restricted situations where
the body size is set once and its content built up piecewise.
---
 include/amqpcpp/envelope.h | 16 +++-------------
 include/amqpcpp/message.h  | 30 +++++++++++++++++++-----------
 2 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/include/amqpcpp/envelope.h b/include/amqpcpp/envelope.h
index 91dd7c7..5f1bf73 100644
--- a/include/amqpcpp/envelope.h
+++ b/include/amqpcpp/envelope.h
@@ -32,7 +32,7 @@ protected:
      *  Pointer to the body data (the memory is not managed by the AMQP library!)
      *  @var    const char *
      */
-    char *_body;
+    const char *_body;
 
     /**
      *  Size of the data
@@ -40,12 +40,6 @@ protected:
      */
     uint64_t _bodySize;
     
-    /**
-     *  Was the data allocated by this object?
-     *  @var    bool
-     */
-    bool _allocated = false;
-
 public:
     /**
      *  Constructor
@@ -56,7 +50,7 @@ public:
      *  @param  body
      *  @param  size
      */
-    Envelope(const char *body, uint64_t size) : MetaData(), _body(const_cast<char *>(body)), _bodySize(size) {}
+    Envelope(const char *body, uint64_t size) : MetaData(), _body(body), _bodySize(size) {}
 
     /**
      *  Disabled copy constructor
@@ -68,11 +62,7 @@ public:
     /**
      *  Destructor
      */
-    virtual ~Envelope()
-    {
-        // deallocate the data
-        if (_allocated) free(_body);
-    }
+    virtual ~Envelope() {}
 
     /**
      *  Access to the full message data
diff --git a/include/amqpcpp/message.h b/include/amqpcpp/message.h
index ee6fd0e..0a35838 100644
--- a/include/amqpcpp/message.h
+++ b/include/amqpcpp/message.h
@@ -38,6 +38,13 @@ class DeferredReceiver;
  */
 class Message : public Envelope
 {
+private:
+   /**
+     *  An allocated and mutable block of memory underlying _body
+     *  @var    char *
+     */
+    char *_mutableBody = nullptr;
+
 protected:
     /**
      *  The exchange to which it was originally published
@@ -88,13 +95,13 @@ protected:
     bool append(const char *buffer, uint64_t size)
     {
         // is the body already allocated?
-        if (_allocated)
+        if (_mutableBody)
         {
             // prevent overflow
             size = std::min(size, _bodySize - _filled);
             
             // append more data
-            memcpy(_body + _filled, buffer, (size_t)size);
+            memcpy(_mutableBody + _filled, buffer, (size_t)size);
             
             // update filled data
             _filled += (size_t)size;
@@ -103,21 +110,19 @@ protected:
         {
             // we do not have to combine multiple frames, so we can store
             // the buffer pointer in the message 
-            _body = const_cast<char *>(buffer);
+            _body = buffer;
         }
         else
         {
             // allocate the buffer
-            _body = (char *)malloc((size_t)_bodySize);
+            _mutableBody = (char *)malloc((size_t)_bodySize);
             
-            // remember that the buffer was allocated, so that the destructor can get rid of it
-            _allocated = true;
+            // expose the body in its immutable form
+            _body = _mutableBody;
             
-            // append more data
-            memcpy(_body, buffer, std::min((size_t)size, (size_t)_bodySize));
-            
-            // update filled data
+            // store the initial data
             _filled = std::min((size_t)size, (size_t)_bodySize);
+            memcpy(_mutableBody, buffer, _filled);
         }
             
         // check if we're done
@@ -144,7 +149,10 @@ public:
     /**
      *  Destructor
      */
-    virtual ~Message() = default;
+    virtual ~Message()
+    {
+        if (_mutableBody) free(_mutableBody);
+    }
 
     /**
      *  The exchange to which it was originally published