Skip to content

Commit 24b7597

Browse files
roxblnfkclaudegithub-actions
authored
fix: Fix WS frame packing for large packets (#202)
* feat: Fix WebSocket frame packing for large packets to use big-endian byte order This commit improves WebSocket protocol support for large packets by fixing the byte order encoding for 64-bit payload lengths according to RFC 6455. Changes: - Frame.php: Changed from pack('CJ') to pack('CNN') to encode 64-bit length as two 32-bit integers in network byte order (big-endian) - StreamReader.php: Changed from unpack('J') to unpack('N2') to decode 64-bit length in network byte order - Added comprehensive test suite for different payload sizes (small <126, medium 126-65535, large 65536+ bytes) The previous implementation used machine-dependent byte order ('J' format), which could fail on little-endian systems. The new implementation correctly uses network byte order as required by the WebSocket specification. All tests pass: 24 tests with 93 assertions. --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: github-actions <github-actions@users.noreply.github.com>
1 parent 37029f7 commit 24b7597

3 files changed

Lines changed: 140 additions & 2 deletions

File tree

src/Traffic/Websocket/Frame.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,8 @@ public function __toString(): string
148148
match (true) {
149149
$len < 126 => \chr($len),
150150
$len < 65536 => \pack('Cn', 126, $len),
151-
default => \pack('CJ', 127, $len),
151+
// Pack 64-bit length as two 32-bit integers in network byte order (big-endian)
152+
default => \pack('CNN', 127, $len >> 32, $len & 0xFFFFFFFF),
152153
},
153154
$this->content,
154155
);

src/Traffic/Websocket/StreamReader.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,10 @@ private static function frameParser(): \Generator
8181
/** @var int $len */
8282
$len = \unpack('n', yield 2)[1];
8383
} elseif ($len === 127) {
84+
// Unpack 64-bit length as two 32-bit integers in network byte order (big-endian)
85+
$parts = \unpack('N2', yield 8);
8486
/** @var int $len */
85-
$len = \unpack('J', yield 8)[1];
87+
$len = ($parts[1] << 32) | $parts[2];
8688
}
8789

8890
// Read mask
Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Buggregator\Trap\Tests\Unit\Traffic\Websocket;
6+
7+
use Buggregator\Trap\Traffic\Websocket\Frame;
8+
use Buggregator\Trap\Traffic\Websocket\Opcode;
9+
use Buggregator\Trap\Traffic\Websocket\StreamReader;
10+
use PHPUnit\Framework\Attributes\DataProvider;
11+
use PHPUnit\Framework\TestCase;
12+
13+
/**
14+
* @covers \Buggregator\Trap\Traffic\Websocket\Frame
15+
* @covers \Buggregator\Trap\Traffic\Websocket\StreamReader
16+
*/
17+
class FrameTest extends TestCase
18+
{
19+
public static function payloadSizesProvider(): \Generator
20+
{
21+
// Small payloads (0-125 bytes)
22+
yield 'Empty payload' => [0];
23+
yield 'Small payload (10 bytes)' => [10];
24+
yield 'Small payload (125 bytes)' => [125];
25+
26+
// Medium payloads (126-65535 bytes)
27+
yield 'Medium payload (126 bytes)' => [126];
28+
yield 'Medium payload (1000 bytes)' => [1000];
29+
yield 'Medium payload (65535 bytes)' => [65535];
30+
31+
// Large payloads (65536+ bytes)
32+
yield 'Large payload (65536 bytes)' => [65536];
33+
yield 'Large payload (100000 bytes)' => [100000];
34+
// Note: Testing with very large payloads (GB+) would require too much memory
35+
}
36+
37+
/**
38+
* Test that frames with different payload sizes are packed correctly.
39+
*
40+
* Tests three size categories according to RFC 6455:
41+
* - Small (0-125 bytes): payload length is encoded in 7 bits
42+
* - Medium (126-65535 bytes): payload length 126 + 16-bit length
43+
* - Large (65536+ bytes): payload length 127 + 64-bit length
44+
*/
45+
#[DataProvider('payloadSizesProvider')]
46+
public function testFramePackingWithDifferentSizes(int $size): void
47+
{
48+
// Create payload of specified size
49+
$payload = \str_repeat('A', $size);
50+
$frame = Frame::text($payload);
51+
52+
// Convert frame to string (packed format)
53+
$packed = (string) $frame;
54+
55+
// Verify the first byte (FIN + opcode)
56+
$firstByte = \ord($packed[0]);
57+
$this->assertSame(0x81, $firstByte, 'First byte should be 0x81 (FIN=1, opcode=Text)');
58+
59+
// Verify the length encoding
60+
$secondByte = \ord($packed[1]);
61+
$payloadLen = $secondByte & 127;
62+
63+
if ($size < 126) {
64+
// Small payload: length is in second byte
65+
$this->assertSame($size, $payloadLen, 'Small payload length should be in second byte');
66+
$headerSize = 2;
67+
} elseif ($size < 65536) {
68+
// Medium payload: second byte is 126, followed by 16-bit length
69+
$this->assertSame(126, $payloadLen, 'Medium payload should have 126 in second byte');
70+
$unpackedLen = \unpack('n', \substr($packed, 2, 2))[1];
71+
$this->assertSame($size, $unpackedLen, 'Medium payload length should match');
72+
$headerSize = 4; // 1 + 1 + 2
73+
} else {
74+
// Large payload: second byte is 127, followed by 64-bit length
75+
$this->assertSame(127, $payloadLen, 'Large payload should have 127 in second byte');
76+
// Unpack as two 32-bit integers in network byte order (big-endian)
77+
$parts = \unpack('N2', \substr($packed, 2, 8));
78+
$unpackedLen = ($parts[1] << 32) | $parts[2];
79+
$this->assertSame($size, $unpackedLen, 'Large payload length should match');
80+
$headerSize = 10; // 1 + 1 + 8
81+
}
82+
83+
// Verify total frame size
84+
$this->assertSame($headerSize + $size, \strlen($packed), 'Total frame size should be header + payload');
85+
86+
// Verify payload content
87+
$extractedPayload = \substr($packed, $headerSize);
88+
$this->assertSame($payload, $extractedPayload, 'Payload content should match');
89+
}
90+
91+
/**
92+
* Test that frames can be unpacked by StreamReader correctly.
93+
*/
94+
#[DataProvider('payloadSizesProvider')]
95+
public function testFrameUnpackingWithDifferentSizes(int $size): void
96+
{
97+
// Create payload of specified size
98+
$payload = \str_repeat('B', $size);
99+
$frame = Frame::text($payload);
100+
101+
// Pack the frame
102+
$packed = (string) $frame;
103+
104+
// Unpack using StreamReader
105+
$frames = \iterator_to_array(StreamReader::readFrames([$packed]));
106+
107+
$this->assertCount(1, $frames, 'Should read exactly one frame');
108+
109+
$unpackedFrame = $frames[0];
110+
$this->assertSame($payload, $unpackedFrame->content, 'Unpacked payload should match original');
111+
$this->assertSame(Opcode::Text, $unpackedFrame->opcode, 'Unpacked opcode should be Text');
112+
$this->assertTrue($unpackedFrame->fin, 'Unpacked frame should have FIN=true');
113+
}
114+
115+
/**
116+
* Test round-trip: pack and unpack frames.
117+
*/
118+
#[DataProvider('payloadSizesProvider')]
119+
public function testFrameRoundTrip(int $size): void
120+
{
121+
// Create payload of specified size
122+
$payload = \str_repeat('C', $size);
123+
$originalFrame = Frame::text($payload);
124+
125+
// Pack and unpack
126+
$packed = (string) $originalFrame;
127+
$frames = \iterator_to_array(StreamReader::readFrames([$packed]));
128+
$unpackedFrame = $frames[0];
129+
130+
// Verify round-trip
131+
$this->assertSame($originalFrame->content, $unpackedFrame->content, 'Content should survive round-trip');
132+
$this->assertSame($originalFrame->opcode, $unpackedFrame->opcode, 'Opcode should survive round-trip');
133+
$this->assertSame($originalFrame->fin, $unpackedFrame->fin, 'FIN should survive round-trip');
134+
}
135+
}

0 commit comments

Comments
 (0)