secure receiving of full messages with TCP, NIO code

Shared Code
#1

Hello.
I’ve needed to implement proper handling of messages if TCP deceides to split them into 2 or more packets (so I’ve heard :slight_smile: ). I did it and here’s the code, it reads short value in message header for message length and reads until it has full message, even if it is in multiple reads from channel. For this to work you’ll need to put message length at beggining as short value (2 bytes).
Only thing is I didn’t fully tested it yet, it’s good for normal transfers when message comes in one TCP packet but I can’t know if TCP split the packets while I was testing… and I’m too lazy to write a test case with custom length header and send the message 2 times :slight_smile:
Any comments welcome.


// vars needed:
int bytes_read = 0;		// Kova: bytes read from channel
boolean full_message_arrived = true;		// Kova: is whole message transfered in received TCP packet
int bytes_left = 0;		// Kova: when whole message didn't arrive, this much must be read to complete it on next channel read
int packet_size = 0;		// Kova: header of message contains message size so it can be read properly if TCP dosen't send all data in one transfer
int read_times = 0;		// Kova: for reading bytes from read buffer n times to message buffer
boolean keep_read_buffer_data = false;        // Kova: if message header about message size is split, hold data until rest of header arrives and then process it
....
....
// before reading you usually clear the buffer, add condition to that
if (!keep_read_buffer_data)
    read_buffer.clear();
....
....
while ((bytes_read = sc.read(read_buffer)) > 0)  {
	System.out.println("BYTES READ: " + bytes_read); :'(
	read_buffer.flip();
	while (read_buffer.hasRemaining()) {
		if (full_message_arrived) {        // Kova: Start reading new message from packet
                        if (read_buffer.remaining() == 1)  {
                            keep_read_buffer_data = true;
                            byte save_me = read_buffer.get();
                            read_buffer.clear();    // Kova: gets rid of last, processed message
                            read_buffer.put(save_me);    // ... but saves 1 byte that you need
                            break;    // Kova: only 1 byte left, 1 more missing for short, wait for it in next packet
                        }
			packet_size = read_buffer.getShort();
			if (packet_size <= read_buffer.remaining()) {        // Kova: whole message arrived in packet
				read_times = packet_size;
				bytes_left = 0;        // Kova: mark full message arrived so it's get processed
			} else {
				full_message_arrived = false;
				bytes_left = packet_size - read_buffer.remaining();
				read_times = read_buffer.remaining();
			}
			message.clear();
			for (int i=0; i < read_times; i++)
			message.put(read_buffer.get());
		} else {        // Kova: reading last message wasn't complete, continue reading
			read_times = (read_buffer.remaining() >= bytes_left)? bytes_left : read_buffer.remaining();
			for (int i=0; i < read_times; i++) {
				message.put(read_buffer.get());

				bytes_left--;
			}
		}
		if (bytes_left == 0) {        // Kova: full message here, process it
			full_message_arrived = true;
			message.flip();
			processClientMessage(message, sc);
		}
	}
} // end: while loop for reading from channel

EDIT: blahblahblahh was so nice and took a look at the code, he discovered a bug, details below, I edited this code, bug is fixed.

#2

what if the short hasn’t arrived yet? :slight_smile:

#3

… hmmm… good question
cutting message exactly between 2 bytes of that short :smiley: … ah well… for now I’m willing to take that risk :slight_smile:
… using 1 byte would mean maximum message size of 128B (256B if you simulate unsigned byte), and I though that is small for modern games. I know for one more way, reading until you read null byte, but I don’t reall get what null byte is (all bits 0 ? ). If it is what I think it is then it could conflict with minimum byte value if program uses it and sends it.

#4

That;s not a “risk” - it means your code doesn’t work. To use NIO you really need to fully understand that you never know which clump of bytes will arrive when.

You can still use two bytes - you just need to check there are “at least N bytes in the buffer” before trying to read the number (presumably 2 for a short)

#5

yep
tnx, I’ll implement it and post it (but not in couple of days).

EDIT: Ok I’ve finally got back to this… although it sounds logical thing to do, keep data in buffer until other 1 byte of that short value header arrives, I’m not very confortable with the way I did it… don’t know why exactly… :slight_smile:
I’ve editet first post and corrected code. The modifications are added:
First of all, declare new boolean:


keep_read_buffer_data = false;

After "if (full_message_arrived) { " add this:


if (read_buffer.remaining() == 1)  {
    keep_read_buffer_data = true;
    byte save_me = read_buffer.get();
    read_buffer.clear();    // Kova: gets rid of last, processed message
    read_buffer.put(save_me);    // ... but saves 1 byte that you need
    break;    // Kova: only 1 byte left, 1 more missing for short, wait for it in next packet
}

and before all that reading, I didn’t inculded becouse it’s common sense you need to do it, you need to clear the buffer… add option if you want it clean:


if (!keep_read_buffer_data)
    read_buffer.clear();

It’s like 4:30 am now here… so I hope I got it right.