0

I'm trying to use Google's ProtocolBuffer to send/receive data in a server/client architecture. I am able to connect the two with winsock and I am able to send and receive data from ProtocolBuffer with it but there seems to be a problem with deserializing the data on the server (I haven't done the opposite so I can't tell if that works yet)

Here's the code I use to create a sample packet of data to send to the server:

MessageID *message = new MessageID();
message->set_type(MessageID::Type::MessageID_Type_PLAYERDATA);

PlayerData::Vector2 *position = new PlayerData::Vector2();
position->set_x(10.0f);
position->set_y(25.0f);

PlayerData *data = new PlayerData();
data->set_health((UINT32)50);
data->set_allocated_position(position);

auto inventory = data->add_items();
inventory->set_itemid((INT32)1);
inventory->set_count((INT32)5);

message->set_allocated_playerdata(data);

m_pNetworkObject->Send(message);

This is the code that actually sends the data over a TCP connection:

// Serialize to string.
std::string sendData;
data->SerializeToString(&sendData);

// Convert data to const char*
const char* dataToSend = sendData.c_str();

int iResult;
iResult = send( ConnectSocket, dataToSend, data->ByteSize(), 0 );
if (iResult == SOCKET_ERROR) 
{
    printf("send failed with error: %d\n", WSAGetLastError());
    closesocket(ConnectSocket);
    WSACleanup();
    exit(1);
}

So, this was code that is run on the client. Now, when I receive this on the server I get the following errors from ProtocolBuffer:

Can't parse message of type "MessageID" because it is missing required fields: type
Can't parse message of type "PlayerData" because it is missing required fields: Position, Health

The code on the server that decompiles the winsock data is like this:

// decompile to a proto file and check ID
MessageID* receivedData = new MessageID();
receivedData->ParseFromArray(&recv, strlen(recv));

// check and redeserialize
switch (receivedData->type())
{
case MessageID::Type::MessageID_Type_PLAYERDATA:
    {
    PlayerData* playerData = new PlayerData();
    playerData->ParseFromArray(&recv, strlen(recv));
    UsePlayerData(sock, playerData);
    break;
    }
case MessageID::Type::MessageID_Type_WORLDDATA:
    {
    WorldData* worldData = new WorldData();
    worldData->ParseFromArray(&recv, strlen(recv));
    UseWorldData(sock, worldData);
    break;
    }
}

The weird thing is that, although the error says that the data doesn't contain the Type value it does set it to 1 and the switch statement works. After that when I try to take a look at the Position and Health data everything is 0.

I have no idea what I'm doing wrong. I read most of the ProtocolBuffer tutorials and help but it seems to no avail.

EDIT: I tried serializing and then deserializing the data in the same application (without the networking involved) and with the following code it worked to do just that:

std::string sendData;
message->SerializeToString(&sendData);

MessageID* receivedData = new MessageID();
receivedData->ParseFromString(sendData);

If I check the data in receivedData it is exactly the same (so, the right values) as in the original object "message"

Dries
  • 995
  • 2
  • 16
  • 45
  • As a side note: Why are you creating `MessageID` with `new`? It's not necessary IIRC. – πάντα ῥεῖ Jul 08 '14 at 14:48
  • Looks like I can't spot from the code you give here what's wrong with the serialized/received from network message data. You may start out with a simpler testcase, that doesn't involve sending the data over the network. Also be sure to receive complete messages, do you have the data length prepended for sending and receiving? – πάντα ῥεῖ Jul 08 '14 at 14:52
  • The data length should be okay since, when I call the winsock send method, I give it the data->ByteSize() as length. Where data is the serialized object from the ProtoclBuffer. I'll try and "receive" it in the same application and see what that does. – Dries Jul 08 '14 at 15:34
  • I'm using pointers by the way because they are used in the tutorials as well. Here's the link: https://developers.google.com/protocol-buffers/docs/cpptutorial – Dries Jul 08 '14 at 15:58
  • They aren't using pointers for the "root" message: `tutorial::AddressBook address_book;`, for the other stuff in sub-messages there are pointers used, because that's what you get when creating a new item with `add_xxx()`, etc. – πάντα ῥεῖ Jul 08 '14 at 16:03
  • 1
    `receivedData->ParseFromArray(&recv, strlen(recv));` will break when your data contains a 0-byte. Make sure you parse based on the data length actually `recv()`'ed. You are not working with 0-terminated strings. – Erik Jul 08 '14 at 16:50
  • Erik: So what you suggest is to check the length of the data first before parsing? And also lookup 0-terminated strings? (I've heard about that but don't know how it works) – Dries Jul 08 '14 at 17:54
  • Heh - strlen() in network code, again:( – Martin James Jul 14 '14 at 11:57
  • I solved it. Thanks :) (I think I forgot to accept the answer so you know it was fixed) – Dries Jul 15 '14 at 15:03

1 Answers1

1

Your problem is that you are using strlen() to find the length of the message. strlen() just looks for the first zero-valued byte and assumes that's the end. This is the convention for text strings, but does not apply to binary messages like Protocol Buffers. In this case, strlen() is returning a size that is too short and so some fields are missing from the message, but the opposite can happen too -- strlen() can run off the end of the buffer and return a size that is too long, or even crash your program.

So, you need to make sure the actual exact size of the message is communicated from the sender to the receiver.

Kenton Varda
  • 41,353
  • 8
  • 121
  • 105
  • Thanks, I'll check it out. I have it working at the moment but I'll definitely keep this in mind because I didn't know that. I'm currently using the return value from recv() as the length of the message (which is exactly what it returns) so I guess I should be fine? Also, I do the parsing before sending it to the second piece of code. (so I don't use strlen(recv) there anymore as well – Dries Jul 08 '14 at 22:17
  • Using the return value of recv() might appear to work when you are sending only one small message at a time, but isn't quite right. It's possible that a message will be broken up so that you have to call recv() multiple times to get all the bytes, and it's also possible that one recv() will return multiple messages. If you send multiple messages, you will need to delimit them somehow, like prefixing each one with the size. There's some code for this here: http://stackoverflow.com/questions/2340730/are-there-c-equivalents-for-the-protocol-buffers-delimited-i-o-functions-in-ja/22927149#22927149 – Kenton Varda Jul 09 '14 at 07:27