Bug in OutputStream::writeString()? Weird bug/behavior sending JSON as base64 Signed text to php server

TL;DR: Why is an extra byte written to a memoryBlock when using OutputStream::writeString(str)? It’s making my server fail to decode JSON sent from my desktop app. testing with JUCE 8.


I’ve come across a weird bug when using LibSodium’s crypto_sign to sign a block of data before it is sent to a php server.
The data is a base64-encoded (juce::JSON::toString( juce::var )) string stored in a juce::MemoryBlock, which will be signed.

The issue is when using MemoryOutputStream::writeString to write a string to the memory block that will be signed, vs using this approach:

auto blockToSign = juce::MemoryBlock(str.toRawUTF8(), str.getNumBytesAsUTF8());

The writeString approach appends one extra byte to the end of the buffer that the other approach does not:

bool OutputStream::writeString (const String& text)
{
    auto numBytes = text.getNumBytesAsUTF8() + 1;

   #if (JUCE_STRING_UTF_TYPE == 8)
    return write (text.toRawUTF8(), numBytes);

On the server side of things, the server verifies the signature using the appropriate crypto_box_sign_open function. This function gives me back this string:
base64-encoded (juce::JSON::toString( juce::var ))

I’ve noticed that sometimes when the juce::var contains nested objects, the extra byte introduced by writeString() causes the base64-decoding step to fail on the server.

The only thing I’ve found in the juce code that points to an oddity of how that string is written into the block is here, in MemoryOutputStream::prepareToWrite()

char* MemoryOutputStream::prepareToWrite (size_t numBytes)
{
    jassert ((ssize_t) numBytes >= 0);
    auto storageNeeded = position + numBytes;

    char* data;

    if (blockToUse != nullptr)
    {
        if (storageNeeded >= blockToUse->getSize())
            blockToUse->ensureSize ((storageNeeded + jmin (storageNeeded / 2, (size_t) (1024 * 1024)) + 32) & ~31u);

ensureSize’s 2nd parameter bool initialiseToZero is not provided and defaults to false. This means that final byte being copied into the blockToUse might be garbage because it was never zero’d.
Setting this parameter to true doesn’t seem to make a difference, either, because it means the final byte is still a ‘0’, and that’s not being trimmed by the base64 decoder on the server (because why would the server trim off that zero?)

Here’s a picture of what that extra null character looks like, when inspected in VS Code:

A few questions:
So, why is the + 1 needed when writing a string into a memoryOutputStream?
Is it for the terminating null character?

Personally, I want to use writeString() to write these JSON strings into this memoryBlock that I’m sending to the server (as base64 text).
But the JSON is failing to decode correctly because of this extra byte in the JSON, so I have to use that other approach:

auto blockToSign = juce::MemoryBlock(str.toRawUTF8(), str.getNumBytesAsUTF8());

If I check if the final byte in the block being converted to base64 before being signed is 0, and remove it, the block decodes on the server correctly.

                    if( block[block.getSize() - 1] == 0 )
                    {
                        jassertfalse; // this is hit every time when using `writeString`
                        block.setSize(block.getSize() - 1);
                    }

It’s an ugly hack, but it works.

Any ideas why this + 1 is needed for writing a string into a juce::MemoryBlock?

For the sake of context, here is how I’m using writeString to test and confirm that this extra byte is the cause:

                auto blockToSign = convertStringToMemoryBlock(dataValue);
                
                //for debugging why the server is failing to decode the JSON string 'dataValue'
                {
                    auto block = juce::MemoryBlock();
                    {
                        juce::MemoryOutputStream mos(block, false);
                        mos.writeString(dataValue);
                    }
                    
                    DBG( "dataValueMB.size() " << dataValueMB.getSize() );
                    DBG( "block.size()       " << block.getSize() );
                    jassertfalse;
                    if( block[block.getSize() - 1] == 0 )
                    {
                        jassertfalse;
                        
                        // comment the line below to see the json decoding bug
                        block.setSize(block.getSize() - 1); 
                    }
                    
                    blockToSign = block; 
                }
                
                unsigned char signed_message[crypto_sign_BYTES + blockToSign.getSize()];
                unsigned long long signed_message_length;
                
                auto result = crypto_sign(signed_message, //the output of the signed message
                            &signed_message_length, //pointer to unsigned long long, which stores the final signed message length
                            static_cast<const unsigned char*>(blockToSign.getData()), //the message to sign
                            blockToSign.getSize(), //the length of the message being signed
                            static_cast<const unsigned char*>(keyForSigning.getData())); //the secret key to sign with
                if( result != 0 )
                {
                    DBG( "crypto_sign failed: " << result );
                    jassertfalse;
                    return;
                }