CODEWITHSUNDEEP
c++socketscharhextelnet
Aelion
Aelion

Reputation: 389

Hex in char array resulting in random character when writing to socket

When communicating with an external system they want me to add a null byte to the end of my response. However I'm facing some problems:

//reading from socket

//sending response
std::string response = "hardcodedResponse";
int bufferSize = response.size() + 1; // +1 for the trailing zero
char buffer[bufferSize];    

for (int i = 0; i < response.size(); i++)
{
    buffer[i] = response[i];
}

buffer[bufferSize] = 0x00;

int socketfd = 1;
unsigned bytesWritten = 0;

while (bytesWritten < bufferSize)
{
    bytesWritten += ::write(socketfd, buffer[bytesWritten], bytesToWrite - bytesWritten);
}

When I use telnet to send something to the socket, I do receive a response: "hardcodedResponse" followed by a ▒. I figured that made sense since 0x00 is null. However if I add 0x41 (A) to the end, I receive "hardcodedResponse" + a (seemingly) random character. If I print the last character of the buffer before writing to the socket, it does print 'A'. This wouldn't really matter if the external system would also receive the correct byte, but alas, they receive the random one.

I'm at a loss as to why this is happening. Hopefully someone can help me understand.

Upvotes: 0

Views: 160

Answers (2)

Remy Lebeau
Remy Lebeau

Reputation: 597385

First, you are relying on a non-standard feature known as "Variable Length Arrays". Only a few C++ compilers support VLAs, and only then as an optional extension. You should use the STL std::vector container instead for a dynamically sized array.

But, more importantly, you are writing the null byte to the wrong array index. The valid range of indexes in buffer[] is 0..bufferSize-1. Writing to buffer[bufferSize] will not append a null terminator at the correct location at the end of the data, and it will trash surrounding memory.

You need to change this:

buffer[bufferSize] = 0x00;

To this:

buffer[bufferSize-1] = 0x00;

You are also not managing the call to write() correctly, either. In particular, it can return -1 on error, which you are not handling. And you should be using bufferSize instead of bytesToWrite.

That being said, you don't actually need the buffer[] array at all, you can just send the std::string data as-is instead:

//sending response
std::string response = "hardcodedResponse";
const char *buffer = response.c_str();
const size_t bufferSize = response.size() + 1; // +1 for the trailing zero

size_t totalWritten = 0;
ssize_t bytesWritten;

do
{
    bytesWritten = ::write(socketfd, buffer[totalWritten], bufferSize - totalWritten);
    if (bytesWritten <= 0)
    {
        // handle error...
        break;
    }
    totalWritten += bytesWritten;
}
while (totalWritten < bufferSize);

Or:

//sending response
std::string response = "hardcodedResponse";
const char *buffer = response.c_str();
size_t bufferSize = response.size() + 1; // +1 for the trailing zero

size_t totalWritten = 0;
ssize_t bytesWritten;

do
{
    bytesWritten = ::write(socketfd, buffer, bufferSize);
    if (bytesWritten <= 0)
    {
        // handle error...
        break;
    }
    buffer += bytesWritten;
    bufferSize -= bytesWritten;
}
while (bufferSize > 0);

Upvotes: 1

doctorlove
doctorlove

Reputation: 19272

Aside from all the socket, look at the buffer length and where you put the null:

std::string response = "hardcodedResponse";
int bufferSize = response.size() + 1; // +1 for the trailing zero
char buffer[bufferSize];    //Up to, but not including bufferSize
                            //0, 1, ... bufferSize-1

for (int i = 0; i < response.size(); i++)
{
    buffer[i] = response[i];
}

buffer[bufferSize] = 0x00;  //BOOM udefined behaviour

You made one extra char for the null, not two, so put the null at the end not beyond the end.

buffer[bufferSize-1] = 0x00;

Upvotes: 1

Related Questions