Bug 1595453 - Allow serializing zero-length ByteBufs. r=nika

If someone tried to serialize a zero-size ByteBuf, it could add a
zero-length segment to the `BufferList` and cause an assertion failure
later when trying to send the message.  This patch makes it a no-op (and
frees the supplied buffer, because the BufferList becomes its owner).

We previously asserted against adding zero-*capacity* segments (likely
also zero size, but possibly not) with WriteBytesZeroCopy, but only on
debug builds, and it was likely happening on release builds despite
that.  That case is now allowed.

Also, error handling for `BufferList::WriteBytesZeroCopy` has been
improved.  (This doesn't affect `Pickle` because it's using infallible
allocation, and no other instances of `BufferList` seem to use
`WriteBytesZeroCopy` at this time.)

Differential Revision: https://phabricator.services.mozilla.com/D192531
This commit is contained in:
Jed Davis
2023-11-02 18:05:31 +00:00
parent 69e37ea633
commit 91c3deb06a
2 changed files with 16 additions and 7 deletions

View File

@@ -455,8 +455,10 @@ bool Pickle::WriteBytesZeroCopy(void* data, uint32_t data_len,
data = realloc(data, new_capacity);
}
#endif
buffers_.WriteBytesZeroCopy(reinterpret_cast<char*>(data), data_len,
new_capacity);
// Shouldn't fail, because we're using InfallibleAllocPolicy.
MOZ_ALWAYS_TRUE(buffers_.WriteBytesZeroCopy(reinterpret_cast<char*>(data),
data_len, new_capacity));
EndWrite(data_len);
return true;

View File

@@ -367,17 +367,24 @@ class BufferList : private AllocPolicy {
}
// This takes ownership of the data
void* WriteBytesZeroCopy(char* aData, size_t aSize, size_t aCapacity) {
MOZ_ASSERT(aCapacity != 0);
MOZ_ASSERT(aSize <= aCapacity);
[[nodiscard]] bool WriteBytesZeroCopy(char* aData, size_t aSize,
size_t aCapacity) {
MOZ_ASSERT(mOwning);
MOZ_ASSERT(aSize <= aCapacity);
// Don't create zero-length segments; that can cause problems for
// consumers of the data (bug 1595453).
if (aSize == 0) {
this->free_(aData, aCapacity);
return true;
}
if (!mSegments.append(Segment(aData, aSize, aCapacity))) {
this->free_(aData, aCapacity);
return nullptr;
return false;
}
mSize += aSize;
return aData;
return true;
}
// Truncate this BufferList at the given iterator location, discarding all