Skip to content

Commit 16bd920

Browse files
clueclue
committed
Refactor to move request body delimiting to request parser
1 parent 8e5e77a commit 16bd920

File tree

3 files changed

+147
-68
lines changed

3 files changed

+147
-68
lines changed

‎src/Io/RequestHeaderParser.php‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,34 @@ public function handle(ConnectionInterface $conn)
6969
return;
7070
}
7171

72+
$contentLength = 0;
73+
$stream = new CloseProtectionStream($conn);
74+
if ($request->hasHeader('Transfer-Encoding')) {
75+
$contentLength = null;
76+
$stream = new ChunkedDecoder($stream);
77+
} elseif ($request->hasHeader('Content-Length')) {
78+
$contentLength = (int)$request->getHeaderLine('Content-Length');
79+
}
80+
81+
if ($contentLength !== null) {
82+
$stream = new LengthLimitedStream($stream, $contentLength);
83+
}
84+
85+
$request = $request->withBody(new HttpBodyStream($stream, $contentLength));
86+
7287
$bodyBuffer = isset($buffer[$endOfHeader + 4]) ? \substr($buffer, $endOfHeader + 4) : '';
7388
$buffer = '';
74-
$that->emit('headers', array($request, $bodyBuffer, $conn));
89+
$that->emit('headers', array($request, $conn));
90+
91+
if ($bodyBuffer !== '') {
92+
$conn->emit('data', array($bodyBuffer));
93+
}
94+
95+
// happy path: request body is known to be empty => immediately end stream
96+
if ($contentLength === 0) {
97+
$stream->emit('end');
98+
$stream->close();
99+
}
75100
});
76101

77102
$conn->on('close', function () use (&$buffer, &$fn) {

‎src/StreamingServer.php‎

Lines changed: 1 addition & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -112,12 +112,8 @@ public function __construct($requestHandler)
112112
$this->parser = new RequestHeaderParser();
113113

114114
$that = $this;
115-
$this->parser->on('headers', function (ServerRequestInterface $request, $bodyBuffer, ConnectionInterface $conn) use ($that) {
115+
$this->parser->on('headers', function (ServerRequestInterface $request, ConnectionInterface $conn) use ($that) {
116116
$that->handleRequest($conn, $request);
117-
118-
if ($bodyBuffer !== '') {
119-
$conn->emit('data', array($bodyBuffer));
120-
}
121117
});
122118

123119
$this->parser->on('error', function(\Exception $e, ConnectionInterface $conn) use ($that) {
@@ -182,18 +178,6 @@ public function listen(ServerInterface $socket)
182178
/** @internal */
183179
public function handleRequest(ConnectionInterface $conn, ServerRequestInterface $request)
184180
{
185-
$contentLength = 0;
186-
$stream = new CloseProtectionStream($conn);
187-
if ($request->hasHeader('Transfer-Encoding')) {
188-
$contentLength = null;
189-
$stream = new ChunkedDecoder($stream);
190-
} elseif ($request->hasHeader('Content-Length')) {
191-
$contentLength = (int)$request->getHeaderLine('Content-Length');
192-
$stream = new LengthLimitedStream($stream, $contentLength);
193-
}
194-
195-
$request = $request->withBody(new HttpBodyStream($stream, $contentLength));
196-
197181
if ($request->getProtocolVersion() !== '1.0' && '100-continue' === \strtolower($request->getHeaderLine('Expect'))) {
198182
$conn->write("HTTP/1.1 100 Continue\r\n\r\n");
199183
}
@@ -217,12 +201,6 @@ public function handleRequest(ConnectionInterface $conn, ServerRequestInterface
217201
});
218202
}
219203

220-
// happy path: request body is known to be empty => immediately end stream
221-
if ($contentLength === 0) {
222-
$stream->emit('end');
223-
$stream->close();
224-
}
225-
226204
// happy path: response returned, handle and return immediately
227205
if ($response instanceof ResponseInterface) {
228206
return $this->handleResponse($conn, $request, $response);

‎tests/Io/RequestHeaderParserTest.php‎

Lines changed: 120 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use React\Http\Io\RequestHeaderParser;
66
use React\Tests\Http\TestCase;
7+
use Psr\Http\Message\ServerRequestInterface;
78

89
class RequestHeaderParserTest extends TestCase
910
{
@@ -59,24 +60,21 @@ public function testFeedTwoRequestsOnSeparateConnections()
5960
$this->assertEquals(2, $called);
6061
}
6162

62-
public function testHeadersEventShouldReturnRequestAndBodyBufferAndConnection()
63+
public function testHeadersEventShouldEmitRequestAndConnection()
6364
{
6465
$request = null;
65-
$bodyBuffer = null;
6666
$conn = null;
6767

6868
$parser = new RequestHeaderParser();
69-
$parser->on('headers', function ($parsedRequest, $parsedBodyBuffer, $connection) use (&$request, &$bodyBuffer, &$conn) {
69+
$parser->on('headers', function ($parsedRequest, $connection) use (&$request, &$conn) {
7070
$request = $parsedRequest;
71-
$bodyBuffer = $parsedBodyBuffer;
7271
$conn = $connection;
7372
});
7473

7574
$connection = $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->setMethods(null)->getMock();
7675
$parser->handle($connection);
7776

7877
$data = $this->createGetRequest();
79-
$data .= 'RANDOM DATA';
8078
$connection->emit('data', array($data));
8179

8280
$this->assertInstanceOf('Psr\Http\Message\RequestInterface', $request);
@@ -85,36 +83,143 @@ public function testHeadersEventShouldReturnRequestAndBodyBufferAndConnection()
8583
$this->assertSame('1.1', $request->getProtocolVersion());
8684
$this->assertSame(array('Host' => array('example.com'), 'Connection' => array('close')), $request->getHeaders());
8785

88-
$this->assertSame('RANDOM DATA', $bodyBuffer);
89-
9086
$this->assertSame($connection, $conn);
9187
}
9288

93-
public function testHeadersEventShouldReturnBinaryBodyBuffer()
89+
public function testHeadersEventShouldEmitRequestWhichShouldEmitEndForStreamingBodyWithoutContentLengthFromInitialRequestBody()
9490
{
95-
$bodyBuffer = null;
91+
$parser = new RequestHeaderParser();
92+
93+
$ended = false;
94+
$that = $this;
95+
$parser->on('headers', function (ServerRequestInterface $request) use (&$ended, $that) {
96+
$body = $request->getBody();
97+
$that->assertInstanceOf('React\Stream\ReadableStreamInterface', $body);
98+
99+
$body->on('end', function () use (&$ended) {
100+
$ended = true;
101+
});
102+
});
103+
104+
$connection = $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->setMethods(null)->getMock();
105+
$parser->handle($connection);
106+
107+
$data = "GET / HTTP/1.0\r\n\r\n";
108+
$connection->emit('data', array($data));
96109

110+
$this->assertTrue($ended);
111+
}
112+
113+
public function testHeadersEventShouldEmitRequestWhichShouldEmitStreamingBodyDataFromInitialRequestBody()
114+
{
97115
$parser = new RequestHeaderParser();
98-
$parser->on('headers', function ($parsedRequest, $parsedBodyBuffer) use (&$bodyBuffer) {
99-
$bodyBuffer = $parsedBodyBuffer;
116+
117+
$buffer = '';
118+
$that = $this;
119+
$parser->on('headers', function (ServerRequestInterface $request) use (&$buffer, $that) {
120+
$body = $request->getBody();
121+
$that->assertInstanceOf('React\Stream\ReadableStreamInterface', $body);
122+
123+
$body->on('data', function ($chunk) use (&$buffer) {
124+
$buffer .= $chunk;
125+
});
126+
$body->on('end', function () use (&$buffer) {
127+
$buffer .= '.';
128+
});
100129
});
101130

102131
$connection = $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->setMethods(null)->getMock();
103132
$parser->handle($connection);
104133

105-
$data = $this->createGetRequest();
106-
$data .= "\0x01\0x02\0x03\0x04\0x05";
134+
$data = "POST / HTTP/1.0\r\nContent-Length: 11\r\n\r\n";
135+
$data .= 'RANDOM DATA';
136+
$connection->emit('data', array($data));
137+
138+
$this->assertSame('RANDOM DATA.', $buffer);
139+
}
140+
141+
public function testHeadersEventShouldEmitRequestWhichShouldEmitStreamingBodyWithPlentyOfDataFromInitialRequestBody()
142+
{
143+
$parser = new RequestHeaderParser();
144+
145+
$buffer = '';
146+
$that = $this;
147+
$parser->on('headers', function (ServerRequestInterface $request) use (&$buffer, $that) {
148+
$body = $request->getBody();
149+
$that->assertInstanceOf('React\Stream\ReadableStreamInterface', $body);
150+
151+
$body->on('data', function ($chunk) use (&$buffer) {
152+
$buffer .= $chunk;
153+
});
154+
});
155+
156+
$connection = $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->setMethods(null)->getMock();
157+
$parser->handle($connection);
158+
159+
$size = 10000;
160+
$data = "POST / HTTP/1.0\r\nContent-Length: $size\r\n\r\n";
161+
$data .= str_repeat('x', $size);
162+
$connection->emit('data', array($data));
163+
164+
$this->assertSame($size, strlen($buffer));
165+
}
166+
167+
public function testHeadersEventShouldEmitRequestWhichShouldNotEmitStreamingBodyDataWithoutContentLengthFromInitialRequestBody()
168+
{
169+
$parser = new RequestHeaderParser();
170+
171+
$buffer = '';
172+
$that = $this;
173+
$parser->on('headers', function (ServerRequestInterface $request) use (&$buffer, $that) {
174+
$body = $request->getBody();
175+
$that->assertInstanceOf('React\Stream\ReadableStreamInterface', $body);
176+
177+
$body->on('data', function ($chunk) use (&$buffer) {
178+
$buffer .= $chunk;
179+
});
180+
});
181+
182+
$connection = $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->setMethods(null)->getMock();
183+
$parser->handle($connection);
184+
185+
$data = "POST / HTTP/1.0\r\n\r\n";
186+
$data .= 'RANDOM DATA';
107187
$connection->emit('data', array($data));
108188

109-
$this->assertSame("\0x01\0x02\0x03\0x04\0x05", $bodyBuffer);
189+
$this->assertSame('', $buffer);
190+
}
191+
192+
public function testHeadersEventShouldEmitRequestWhichShouldEmitStreamingBodyDataUntilContentLengthBoundaryFromInitialRequestBody()
193+
{
194+
$parser = new RequestHeaderParser();
195+
196+
$buffer = '';
197+
$that = $this;
198+
$parser->on('headers', function (ServerRequestInterface $request) use (&$buffer, $that) {
199+
$body = $request->getBody();
200+
$that->assertInstanceOf('React\Stream\ReadableStreamInterface', $body);
201+
202+
$body->on('data', function ($chunk) use (&$buffer) {
203+
$buffer .= $chunk;
204+
});
205+
});
206+
207+
$connection = $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->setMethods(null)->getMock();
208+
$parser->handle($connection);
209+
210+
$data = "POST / HTTP/1.0\r\nContent-Length: 6\r\n\r\n";
211+
$data .= 'RANDOM DATA';
212+
$connection->emit('data', array($data));
213+
214+
$this->assertSame('RANDOM', $buffer);
110215
}
111216

112217
public function testHeadersEventShouldParsePathAndQueryString()
113218
{
114219
$request = null;
115220

116221
$parser = new RequestHeaderParser();
117-
$parser->on('headers', function ($parsedRequest, $parsedBodyBuffer) use (&$request) {
222+
$parser->on('headers', function ($parsedRequest) use (&$request) {
118223
$request = $parsedRequest;
119224
});
120225

@@ -197,35 +302,6 @@ public function testHeaderOverflowShouldEmitError()
197302
$this->assertSame($connection, $passedConnection);
198303
}
199304

200-
public function testHeaderOverflowShouldNotEmitErrorWhenDataExceedsMaxHeaderSize()
201-
{
202-
$request = null;
203-
$bodyBuffer = null;
204-
205-
$parser = new RequestHeaderParser();
206-
$parser->on('headers', function ($parsedRequest, $parsedBodyBuffer) use (&$request, &$bodyBuffer) {
207-
$request = $parsedRequest;
208-
$bodyBuffer = $parsedBodyBuffer;
209-
});
210-
211-
$connection = $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->setMethods(null)->getMock();
212-
$parser->handle($connection);
213-
214-
$data = $this->createAdvancedPostRequest();
215-
$body = str_repeat('A', 8193 - strlen($data));
216-
$data .= $body;
217-
$connection->emit('data', array($data));
218-
219-
$headers = array(
220-
'Host' => array('example.com'),
221-
'User-Agent' => array('react/alpha'),
222-
'Connection' => array('close'),
223-
);
224-
$this->assertSame($headers, $request->getHeaders());
225-
226-
$this->assertSame($body, $bodyBuffer);
227-
}
228-
229305
public function testInvalidEmptyRequestHeadersParseException()
230306
{
231307
$error = null;

0 commit comments

Comments
 (0)