From 5fe9ccc0969ad253446afe162dd5463c209ba5e8 Mon Sep 17 00:00:00 2001
From: Benjamin Zikarsky <benjamin.zikarsky@jaumo.com>
Date: Mon, 29 Jul 2019 19:43:48 +0200
Subject: [PATCH] Fix bug with UDP chunk-size calculation

The previous code ignored the size of the UDo chunk header when the message was
chunked. This allowed for chunks with a size exceeding the defined max
chunk-size
---
 src/Gelf/Transport/UdpTransport.php           | 19 +++++++++++++------
 .../Gelf/Test/Transport/UdpTransportTest.php  | 14 +++++++++-----
 2 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/src/Gelf/Transport/UdpTransport.php b/src/Gelf/Transport/UdpTransport.php
index c7dfbbc..cb0ee43 100644
--- a/src/Gelf/Transport/UdpTransport.php
+++ b/src/Gelf/Transport/UdpTransport.php
@@ -13,6 +13,7 @@
 
 use Gelf\MessageInterface as Message;
 use Gelf\Encoder\CompressedJsonEncoder as DefaultEncoder;
+use InvalidArgumentException;
 use RuntimeException;
 
 /**
@@ -30,6 +31,7 @@ class UdpTransport extends AbstractTransport
     const CHUNK_MAX_COUNT = 128; // as per GELF spec
     const CHUNK_SIZE_LAN = 8154;
     const CHUNK_SIZE_WAN = 1420;
+    const CHUNK_HEADER_LENGTH = 12; // GELF ID (2b), id (8b) , sequence (2b)
 
     const DEFAULT_HOST = "127.0.0.1";
     const DEFAULT_PORT = 12201;
@@ -47,9 +49,9 @@ class UdpTransport extends AbstractTransport
     /**
      * Class constructor
      *
-     * @param string $host      when NULL or empty DEFAULT_HOST is used
-     * @param int    $port      when NULL or empty DEFAULT_PORT is used
-     * @param int    $chunkSize defaults to CHUNK_SIZE_WAN,
+     * @param string $host when NULL or empty DEFAULT_HOST is used
+     * @param int $port when NULL or empty DEFAULT_PORT is used
+     * @param int $chunkSize defaults to CHUNK_SIZE_WAN,
      *                          0 disables chunks completely
      */
     public function __construct(
@@ -65,6 +67,11 @@ public function __construct(
         $this->chunkSize = $chunkSize;
 
         $this->messageEncoder = new DefaultEncoder();
+
+        if ($chunkSize > 0 && $chunkSize <= self::CHUNK_HEADER_LENGTH) {
+            throw new InvalidArgumentException('Chunk-size has to exceed ' . self::CHUNK_HEADER_LENGTH
+                . ' which is the number of bytes reserved for the chunk-header');
+        }
     }
 
     /**
@@ -94,16 +101,16 @@ public function send(Message $message)
     /**
      * Sends given string in multiple chunks
      *
-     * @param  string $rawMessage
+     * @param string $rawMessage
      * @return int
      *
      * @throws RuntimeException on too large messages which would exceed the
-                                maximum number of possible chunks
+     * maximum number of possible chunks
      */
     protected function sendMessageInChunks($rawMessage)
     {
         // split to chunks
-        $chunks = str_split($rawMessage, $this->chunkSize);
+        $chunks = str_split($rawMessage, $this->chunkSize - self::CHUNK_HEADER_LENGTH);
         $numChunks = count($chunks);
 
         if ($numChunks > self::CHUNK_MAX_COUNT) {
diff --git a/tests/Gelf/Test/Transport/UdpTransportTest.php b/tests/Gelf/Test/Transport/UdpTransportTest.php
index e81b440..8443eea 100644
--- a/tests/Gelf/Test/Transport/UdpTransportTest.php
+++ b/tests/Gelf/Test/Transport/UdpTransportTest.php
@@ -106,13 +106,17 @@ public function testSendUnchunked()
 
     public function testSendChunked()
     {
-        $chunkSize = 10;
-        $transport = $this->getTransport(10);
-        $expectedMessageCount =  strlen($this->testMessage) / $chunkSize;
+        $chunkSize = 20 + UdpTransport::CHUNK_HEADER_LENGTH;
+        $transport = $this->getTransport($chunkSize);
+        $expectedMessageCount =  strlen($this->testMessage) / ($chunkSize - UdpTransport::CHUNK_HEADER_LENGTH);
 
+        $test = $this;
         $this->socketClient
             ->expects($this->exactly($expectedMessageCount))
-            ->method('write');
+            ->method('write')
+            ->willReturnCallback(function ($data) use ($chunkSize, $test) {
+                $test->assertLessThanOrEqual($chunkSize, strlen($data));
+            });
 
         $transport->send($this->message);
     }
@@ -122,7 +126,7 @@ public function testSendChunked()
      */
     public function testInvalidChunkNumber()
     {
-        $transport = $this->getTransport(1);
+        $transport = $this->getTransport(UdpTransport::CHUNK_HEADER_LENGTH + 1);
         $transport->send($this->message);
     }
 }