Modify the transfer protocol in a couple ways.

Replace the 'D' header with an unsigned byte that indicates the protocol version. This will allow easier changes to protocol in the future.
Replace the signed integer used for message length with an unsigned 32-bit integer. There is no need for a signed value here as a message length must always be positive. This also doubles the max message length.

2 files changed, 36 insertions, 28 deletions

diff --git a/deluge/tests/test_transfer.py b/deluge/tests/test_transfer.py
index a1af5fc29..a04830325 100644
--- a/deluge/tests/test_transfer.py
+++ b/deluge/tests/test_transfer.py
@@ -116,7 +116,7 @@ class DelugeTransferProtocolTestCase(unittest.TestCase):
     def setUp(self):  # NOQA: N803
         """
         The expected messages corresponds to the test messages (msg1, msg2) after they've been processed
-        by DelugeTransferProtocol.send, which means that they've first been encoded with pickle,
+        by DelugeTransferProtocol.send, which means that they've first been encoded with rencode,
         and then compressed with zlib.
         The expected messages are encoded in base64 to easily including it here in the source.
         So before comparing the results with the expected messages, the expected messages must be decoded,
@@ -141,11 +141,11 @@ class DelugeTransferProtocolTestCase(unittest.TestCase):
         )
 
         self.msg1_expected_compressed_base64 = (
-            b'RAAAADF4nDvKwJjenp1aGZ+ZV+Lgxfv9PYRXXFLU'
+            b'AQAAADF4nDvKwJjenp1aGZ+ZV+Lgxfv9PYRXXFLU'
             b'XZyfm6oAZGTmpad3gAST8vNznAEAJhSQ'
         )
         self.msg2_expected_compressed_base64 = (
-            b'RAAAAF14nDvGxJzemZ1aGZ+Wk59Y4uTmpKib3g3il+ZlJuenpH'
+            b'AQAAAF14nDvGxJzemZ1aGZ+Wk59Y4uTmpKib3g3il+ZlJuenpH'
             b'YX5+emKhSXFGXmpadPBkmkZCaXxJdnlmTEl5QW5KRCdIOZhxmB'
             b'hrUDuTmZxSWHWRpNnRyupaUBAHYlJxI='
         )
@@ -331,7 +331,7 @@ class DelugeTransferProtocolTestCase(unittest.TestCase):
         The next part contains the rest of the message.
 
         This is a special case, as DelugeTransferProtocol can't start parsing
-        a message until it has at least 4 bytes (the size of the header) to be able
+        a message until it has at least 5 bytes (the size of the header) to be able
         to read and parse the size of the payload.
 
         """
diff --git a/deluge/transfer.py b/deluge/transfer.py
index 81f78d6d0..10f6d51e0 100644
--- a/deluge/transfer.py
+++ b/deluge/transfer.py
@@ -1,6 +1,7 @@
 # -*- coding: utf-8 -*-
 #
 # Copyright (C) 2012 Bro <bro.development@gmail.com>
+# Copyright (C) 2018 Andrew Resch <andrewresch@gmail.com>
 #
 # This file is part of Deluge and is licensed under GNU General Public License 3.0, or later, with
 # the additional special exception to link portions of this program with the OpenSSL library.
@@ -18,15 +19,26 @@ from twisted.internet.protocol import Protocol
 
 log = logging.getLogger(__name__)
 
-MESSAGE_HEADER_SIZE = 5
+PROTOCOL_VERSION = 1
+MESSAGE_HEADER_FORMAT = '!BI'
+MESSAGE_HEADER_SIZE = struct.calcsize(MESSAGE_HEADER_FORMAT)
 
 
 class DelugeTransferProtocol(Protocol, object):
     """
-    Data messages are transfered using very a simple protocol.
-    Data messages are transfered with a header containing
-    the length of the data to be transfered (payload).
+    Deluge RPC wire protocol.
 
+    Data messages are transfered with a header containing a protocol version
+    and the length of the data to be transfered (payload).
+
+    The format is:
+
+            ubyte    uint4     bytestring
+        |.version.|..size..|.....body.....|
+
+    The version is an unsigned byte that indicates the protocol version.
+    The size is a unsigned 32-bit integer that is equal to the length of the body bytestring.
+    The body is the compressed rencoded byte string of the data object.
     """
 
     def __init__(self):
@@ -39,21 +51,18 @@ class DelugeTransferProtocol(Protocol, object):
         """
         Transfer the data.
 
-        The data will be serialized and compressed before being sent.
-        First a header is sent - containing the length of the compressed payload
-        to come as a signed integer. After the header, the payload is transfered.
-
         :param data: data to be transfered in a data structure serializable by rencode.
-
         """
-        compressed = zlib.compress(rencode.dumps(data))
-        size_data = len(compressed)
-        # Store length as a signed integer (using 4 bytes). "!" denotes network byte order.
-        payload_len = struct.pack('!i', size_data)
-        header = b'D' + payload_len
-        self._bytes_sent += len(header) + len(compressed)
-        self.transport.write(header)
-        self.transport.write(compressed)
+        body = zlib.compress(rencode.dumps(data))
+        body_len = len(body)
+        message = struct.pack(
+            '{}{}s'.format(MESSAGE_HEADER_FORMAT, body_len),
+            PROTOCOL_VERSION,
+            body_len,
+            body,
+        )
+        self._bytes_sent += len(message)
+        self.transport.write(message)
 
     def dataReceived(self, data):  # NOQA: N802
         """
@@ -91,15 +100,14 @@ class DelugeTransferProtocol(Protocol, object):
         try:
             # Read the first bytes of the message (MESSAGE_HEADER_SIZE bytes)
             header = self._buffer[:MESSAGE_HEADER_SIZE]
-            payload_len = header[1:MESSAGE_HEADER_SIZE]
-            if header[0:1] != b'D':
+            # Extract the length stored as an unsigned 32-bit integer
+            version, self._message_length = struct.unpack(MESSAGE_HEADER_FORMAT, header)
+            if version != PROTOCOL_VERSION:
                 raise Exception(
-                    'Invalid header format. First byte is %d' % ord(header[0:1])
+                    'Received invalid protocol version: {}. PROTOCOL_VERSION is {}.'.format(
+                        version, PROTOCOL_VERSION
+                    )
                 )
-            # Extract the length stored as a signed integer (using 4 bytes)
-            self._message_length = struct.unpack('!i', payload_len)[0]
-            if self._message_length < 0:
-                raise Exception('Message length is negative: %d' % self._message_length)
             # Remove the header from the buffer
             self._buffer = self._buffer[MESSAGE_HEADER_SIZE:]
         except Exception as ex: