Envelope: ensure const objects cannot be changed through _body

In commit 00b81949d3 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.
This commit is contained in:
Peter A. Bigot 2018-04-20 18:20:17 -05:00
parent 42c7010fe9
commit d0b5189d1e
2 changed files with 22 additions and 24 deletions

View File

@ -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

View File

@ -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