Skip to content

Commit ce16c02

Browse files
authored
Merge pull request #481 from clue-labs/refactor-requestdata
Internal refactoring to remove internal `RequestData` and rename internal `Request` to `ClientRequestStream`
2 parents 8095cd1 + 212a3bb commit ce16c02

8 files changed

+241
-448
lines changed

src/Client/Client.php

+6-5
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@
22

33
namespace React\Http\Client;
44

5+
use Psr\Http\Message\RequestInterface;
56
use React\EventLoop\LoopInterface;
6-
use React\Socket\ConnectorInterface;
7+
use React\Http\Io\ClientRequestStream;
78
use React\Socket\Connector;
9+
use React\Socket\ConnectorInterface;
810

911
/**
1012
* @internal
@@ -22,10 +24,9 @@ public function __construct(LoopInterface $loop, ConnectorInterface $connector =
2224
$this->connector = $connector;
2325
}
2426

25-
public function request($method, $url, array $headers = array(), $protocolVersion = '1.0')
27+
/** @return ClientRequestStream */
28+
public function request(RequestInterface $request)
2629
{
27-
$requestData = new RequestData($method, $url, $headers, $protocolVersion);
28-
29-
return new Request($this->connector, $requestData);
30+
return new ClientRequestStream($this->connector, $request);
3031
}
3132
}

src/Client/RequestData.php

-127
This file was deleted.

src/Client/Request.php src/Io/ClientRequestStream.php

+31-13
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
<?php
22

3-
namespace React\Http\Client;
3+
namespace React\Http\Io;
44

55
use Evenement\EventEmitter;
6+
use Psr\Http\Message\RequestInterface;
67
use React\Promise;
78
use React\Socket\ConnectionInterface;
89
use React\Socket\ConnectorInterface;
@@ -16,28 +17,33 @@
1617
* @event end
1718
* @internal
1819
*/
19-
class Request extends EventEmitter implements WritableStreamInterface
20+
class ClientRequestStream extends EventEmitter implements WritableStreamInterface
2021
{
2122
const STATE_INIT = 0;
2223
const STATE_WRITING_HEAD = 1;
2324
const STATE_HEAD_WRITTEN = 2;
2425
const STATE_END = 3;
2526

27+
/** @var ConnectorInterface */
2628
private $connector;
27-
private $requestData;
2829

30+
/** @var RequestInterface */
31+
private $request;
32+
33+
/** @var ?ConnectionInterface */
2934
private $stream;
35+
3036
private $buffer;
3137
private $responseFactory;
3238
private $state = self::STATE_INIT;
3339
private $ended = false;
3440

3541
private $pendingWrites = '';
3642

37-
public function __construct(ConnectorInterface $connector, RequestData $requestData)
43+
public function __construct(ConnectorInterface $connector, RequestInterface $request)
3844
{
3945
$this->connector = $connector;
40-
$this->requestData = $requestData;
46+
$this->request = $request;
4147
}
4248

4349
public function isWritable()
@@ -49,28 +55,36 @@ private function writeHead()
4955
{
5056
$this->state = self::STATE_WRITING_HEAD;
5157

52-
$requestData = $this->requestData;
58+
$request = $this->request;
5359
$streamRef = &$this->stream;
5460
$stateRef = &$this->state;
5561
$pendingWrites = &$this->pendingWrites;
5662
$that = $this;
5763

5864
$promise = $this->connect();
5965
$promise->then(
60-
function (ConnectionInterface $stream) use ($requestData, &$streamRef, &$stateRef, &$pendingWrites, $that) {
66+
function (ConnectionInterface $stream) use ($request, &$streamRef, &$stateRef, &$pendingWrites, $that) {
6167
$streamRef = $stream;
68+
assert($streamRef instanceof ConnectionInterface);
6269

6370
$stream->on('drain', array($that, 'handleDrain'));
6471
$stream->on('data', array($that, 'handleData'));
6572
$stream->on('end', array($that, 'handleEnd'));
6673
$stream->on('error', array($that, 'handleError'));
6774
$stream->on('close', array($that, 'handleClose'));
6875

69-
$headers = (string) $requestData;
76+
assert($request instanceof RequestInterface);
77+
$headers = "{$request->getMethod()} {$request->getRequestTarget()} HTTP/{$request->getProtocolVersion()}\r\n";
78+
foreach ($request->getHeaders() as $name => $values) {
79+
foreach ($values as $value) {
80+
$headers .= "$name: $value\r\n";
81+
}
82+
}
7083

71-
$more = $stream->write($headers . $pendingWrites);
84+
$more = $stream->write($headers . "\r\n" . $pendingWrites);
7285

73-
$stateRef = Request::STATE_HEAD_WRITTEN;
86+
assert($stateRef === ClientRequestStream::STATE_WRITING_HEAD);
87+
$stateRef = ClientRequestStream::STATE_HEAD_WRITTEN;
7488

7589
// clear pending writes if non-empty
7690
if ($pendingWrites !== '') {
@@ -217,20 +231,24 @@ public function close()
217231

218232
protected function connect()
219233
{
220-
$scheme = $this->requestData->getScheme();
234+
$scheme = $this->request->getUri()->getScheme();
221235
if ($scheme !== 'https' && $scheme !== 'http') {
222236
return Promise\reject(
223237
new \InvalidArgumentException('Invalid request URL given')
224238
);
225239
}
226240

227-
$host = $this->requestData->getHost();
228-
$port = $this->requestData->getPort();
241+
$host = $this->request->getUri()->getHost();
242+
$port = $this->request->getUri()->getPort();
229243

230244
if ($scheme === 'https') {
231245
$host = 'tls://' . $host;
232246
}
233247

248+
if ($port === null) {
249+
$port = $scheme === 'https' ? 443 : 80;
250+
}
251+
234252
return $this->connector
235253
->connect($host . ':' . $port);
236254
}

src/Io/Sender.php

+12-4
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,9 @@ public function __construct(HttpClient $http)
7474
*/
7575
public function send(RequestInterface $request)
7676
{
77+
// support HTTP/1.1 and HTTP/1.0 only, ensured by `Browser` already
78+
assert(\in_array($request->getProtocolVersion(), array('1.0', '1.1'), true));
79+
7780
$body = $request->getBody();
7881
$size = $body->getSize();
7982

@@ -91,12 +94,17 @@ public function send(RequestInterface $request)
9194
$size = 0;
9295
}
9396

94-
$headers = array();
95-
foreach ($request->getHeaders() as $name => $values) {
96-
$headers[$name] = implode(', ', $values);
97+
// automatically add `Connection: close` request header for HTTP/1.1 requests to avoid connection reuse
98+
if ($request->getProtocolVersion() === '1.1' && !$request->hasHeader('Connection')) {
99+
$request = $request->withHeader('Connection', 'close');
100+
}
101+
102+
// automatically add `Authorization: Basic …` request header if URL includes `user:pass@host`
103+
if ($request->getUri()->getUserInfo() !== '' && !$request->hasHeader('Authorization')) {
104+
$request = $request->withHeader('Authorization', 'Basic ' . \base64_encode($request->getUri()->getUserInfo()));
97105
}
98106

99-
$requestStream = $this->http->request($request->getMethod(), (string)$request->getUri(), $headers, $request->getProtocolVersion());
107+
$requestStream = $this->http->request($request);
100108

101109
$deferred = new Deferred(function ($_, $reject) use ($requestStream) {
102110
// close request stream if request is cancelled

tests/Client/FunctionalIntegrationTest.php

+7-6
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use Psr\Http\Message\ResponseInterface;
66
use React\EventLoop\Loop;
77
use React\Http\Client\Client;
8+
use React\Http\Message\Request;
89
use React\Promise\Deferred;
910
use React\Promise\Stream;
1011
use React\Socket\ConnectionInterface;
@@ -45,7 +46,7 @@ public function testRequestToLocalhostEmitsSingleRemoteConnection()
4546
$port = parse_url($socket->getAddress(), PHP_URL_PORT);
4647

4748
$client = new Client(Loop::get());
48-
$request = $client->request('GET', 'http://localhost:' . $port);
49+
$request = $client->request(new Request('GET', 'http://localhost:' . $port, array(), '', '1.0'));
4950

5051
$promise = Stream\first($request, 'close');
5152
$request->end();
@@ -62,7 +63,7 @@ public function testRequestLegacyHttpServerWithOnlyLineFeedReturnsSuccessfulResp
6263
});
6364

6465
$client = new Client(Loop::get());
65-
$request = $client->request('GET', str_replace('tcp:', 'http:', $socket->getAddress()));
66+
$request = $client->request(new Request('GET', str_replace('tcp:', 'http:', $socket->getAddress()), array(), '', '1.0'));
6667

6768
$once = $this->expectCallableOnceWith('body');
6869
$request->on('response', function (ResponseInterface $response, ReadableStreamInterface $body) use ($once) {
@@ -83,7 +84,7 @@ public function testSuccessfulResponseEmitsEnd()
8384

8485
$client = new Client(Loop::get());
8586

86-
$request = $client->request('GET', 'http://www.google.com/');
87+
$request = $client->request(new Request('GET', 'http://www.google.com/', array(), '', '1.0'));
8788

8889
$once = $this->expectCallableOnce();
8990
$request->on('response', function (ResponseInterface $response, ReadableStreamInterface $body) use ($once) {
@@ -109,7 +110,7 @@ public function testPostDataReturnsData()
109110
$client = new Client(Loop::get());
110111

111112
$data = str_repeat('.', 33000);
112-
$request = $client->request('POST', 'https://' . (mt_rand(0, 1) === 0 ? 'eu.' : '') . 'httpbin.org/post', array('Content-Length' => strlen($data)));
113+
$request = $client->request(new Request('POST', 'https://' . (mt_rand(0, 1) === 0 ? 'eu.' : '') . 'httpbin.org/post', array('Content-Length' => strlen($data)), '', '1.0'));
113114

114115
$deferred = new Deferred();
115116
$request->on('response', function (ResponseInterface $response, ReadableStreamInterface $body) use ($deferred) {
@@ -141,7 +142,7 @@ public function testPostJsonReturnsData()
141142
$client = new Client(Loop::get());
142143

143144
$data = json_encode(array('numbers' => range(1, 50)));
144-
$request = $client->request('POST', 'https://httpbin.org/post', array('Content-Length' => strlen($data), 'Content-Type' => 'application/json'));
145+
$request = $client->request(new Request('POST', 'https://httpbin.org/post', array('Content-Length' => strlen($data), 'Content-Type' => 'application/json'), '', '1.0'));
145146

146147
$deferred = new Deferred();
147148
$request->on('response', function (ResponseInterface $response, ReadableStreamInterface $body) use ($deferred) {
@@ -170,7 +171,7 @@ public function testCancelPendingConnectionEmitsClose()
170171

171172
$client = new Client(Loop::get());
172173

173-
$request = $client->request('GET', 'http://www.google.com/');
174+
$request = $client->request(new Request('GET', 'http://www.google.com/', array(), '', '1.0'));
174175
$request->on('error', $this->expectCallableNever());
175176
$request->on('close', $this->expectCallableOnce());
176177
$request->end();

0 commit comments

Comments
 (0)