[Solved] WinSock TCP - Crash if buffer over around 5000

Started by
10 comments, last by irbaboon 12 years, 6 months ago
Your networking code contains a number of errors. send() is documented as potentially "partially sending" the specified buffer. You need to call send() in a loop to ensure all the bytes are sent.

Something like this: (warning: code neither compiled nor tested)

bool SendData(SOCKET socket, string data, bool log)
{
const char *buffer = data.c_str(); // What is Char() ?
int sent = 0;
while(sent < data.length())
{
int bytes = send(socket, buffer, data.length() - sent, 0);
if(bytes == SOCKET_ERROR)
{
if(log)
{
cout << "\nError while trying to send data: " << data.c_str() << endl;
cout << '\n' << sent << " bytes sent" << endl;
cout << "Error: " << WSAGetLastError() << endl;
}
return false;
}
else
{
sent += bytes;
buffer += bytes;
}
}

if(log)
{
cout << "\nSuccessfully sent data: " << data.c_str() << endl;
}
return true;
}


Likewise, your recv() code is perfectly willing to accept partial messages and treat them as whole ones, or to merge multiple messages and treat them as a single one.

To solve this, you need to come up with a protocol that allows the sender to specify which data makes up which messages. Two common solutions are to prefix each message with the number of bytes in the message, or to use a delimiter value to break up the messages. Binary protocols usually use the former, while text protocols often use the latter. Another solution is to use packet type identifiers - each identifier maps to a particular packet type, with a known length.

You'll need a persistent application buffer per-client to do this. This buffer needs to be passed into successive calls with the same socket. A good idea is to package the buffer and socket into a structure to keep them together.

It might look something like this:

struct Client
{
SOCKET socket;
uint32_t nextPacketSize;
std::vector<char> buffer;

Client(SOCKET socket)
:
socket(socket),
nextPacketSize(0),
buffer(BUFFER_SIZE)
{
}

bool RecvData(string &message, bool log);
};

bool Client::RecvData(string &message, bool log)
{
int bytes = recv(socket, buffer, buffer.size(), 0);
if (bytes <= 0)
{
if (log)
{
cout << "\nError while trying to receive data" << endl;
cout << "Error: " << WSAGetLastError() << endl;
}
return false;
}

if (log)
{
cout << "\nSuccessfully received " << bytes << " bytes." << endl;
}

// We haven't determined how big the next packet is yet
if(nextPacketSize == 0)
{
// Check if there is a full length prefix in the buffer...
if(buffer.size() > sizeof(nextPacketSize))
{
// Function removes sizeof(uint32_t) from buffer, in a known endian format
nextPacketSize = extractUint32(buffer);

// Sanity check the client packet length
if(nextPacketSize == 0 || nextPacketSize > BUFFER_SIZE)
{
// Handle protocol error...
return false;
}
}
else
{
// Haven't got a full message length prefix yet.
return false;
}
}

// Does our buffer contains a full message?
if(nextPacketSize <= buffer.size())
{
// Copy the data
message.assign(buffer.begin(), buffer.begin() + nextPacketSize);
// Clear that part of the buffer
buffer.erase(buffer.begin(), buffer.begin() + nextPacketSize);
// We're waiting for the packet header again
nextPacketSize = 0;
// Signal success
return true;
}
// Still waiting for a full message...
return false;
}

Again, code neither compiled nor tested.
Advertisement
Thanks, rip-off, I'll fix that stuff up : )

The[color="#800080"] Char[color="#808000"]([color="#0000FF"]string[color="#808000"]) function returned a [color="#0000FF"]char[color="#808000"]* of the [color="#0000FF"]string given, I just like to make functions for stuff to make it cleaner...cleaner for me anyhow : )

This topic is closed to new replies.

Advertisement