Skip to content

Commit

Permalink
Fix bug with UDP chunk-size calculation
Browse files Browse the repository at this point in the history
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
bzikarsky committed Jul 29, 2019
1 parent 2344540 commit 5fe9ccc
Showing 2 changed files with 22 additions and 11 deletions.
19 changes: 13 additions & 6 deletions src/Gelf/Transport/UdpTransport.php
Original file line number Diff line number Diff line change
@@ -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) {
14 changes: 9 additions & 5 deletions tests/Gelf/Test/Transport/UdpTransportTest.php
Original file line number Diff line number Diff line change
@@ -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);
}
}

0 comments on commit 5fe9ccc

Please sign in to comment.