Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor internal Transaction to avoid assigning dynamic properties (PHP 8.2+ compatibility) #459

Merged
merged 1 commit into from Jun 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 16 additions & 0 deletions src/Io/ClientRequestState.php
@@ -0,0 +1,16 @@
<?php

namespace React\Http\Io;

/** @internal */
class ClientRequestState
{
/** @var int */
public $numRequests = 0;

/** @var ?\React\Promise\PromiseInterface */
public $pending = null;

/** @var ?\React\EventLoop\TimerInterface */
public $timeout = null;
}
79 changes: 37 additions & 42 deletions src/Io/Transaction.php
Expand Up @@ -67,32 +67,31 @@ public function withOptions(array $options)

public function send(RequestInterface $request)
{
$deferred = new Deferred(function () use (&$deferred) {
if (isset($deferred->pending)) {
$deferred->pending->cancel();
unset($deferred->pending);
$state = new ClientRequestState();
$deferred = new Deferred(function () use ($state) {
if ($state->pending !== null) {
$state->pending->cancel();
$state->pending = null;
}
});

$deferred->numRequests = 0;

// use timeout from options or default to PHP's default_socket_timeout (60)
$timeout = (float)($this->timeout !== null ? $this->timeout : ini_get("default_socket_timeout"));

$loop = $this->loop;
$this->next($request, $deferred)->then(
function (ResponseInterface $response) use ($deferred, $loop, &$timeout) {
if (isset($deferred->timeout)) {
$loop->cancelTimer($deferred->timeout);
unset($deferred->timeout);
$this->next($request, $deferred, $state)->then(
function (ResponseInterface $response) use ($state, $deferred, $loop, &$timeout) {
if ($state->timeout !== null) {
$loop->cancelTimer($state->timeout);
$state->timeout = null;
}
$timeout = -1;
$deferred->resolve($response);
},
function ($e) use ($deferred, $loop, &$timeout) {
if (isset($deferred->timeout)) {
$loop->cancelTimer($deferred->timeout);
unset($deferred->timeout);
function ($e) use ($state, $deferred, $loop, &$timeout) {
if ($state->timeout !== null) {
$loop->cancelTimer($state->timeout);
$state->timeout = null;
}
$timeout = -1;
$deferred->reject($e);
Expand All @@ -106,67 +105,65 @@ function ($e) use ($deferred, $loop, &$timeout) {
$body = $request->getBody();
if ($body instanceof ReadableStreamInterface && $body->isReadable()) {
$that = $this;
$body->on('close', function () use ($that, $deferred, &$timeout) {
$body->on('close', function () use ($that, $deferred, $state, &$timeout) {
if ($timeout >= 0) {
$that->applyTimeout($deferred, $timeout);
$that->applyTimeout($deferred, $state, $timeout);
}
});
} else {
$this->applyTimeout($deferred, $timeout);
$this->applyTimeout($deferred, $state, $timeout);
}

return $deferred->promise();
}

/**
* @internal
* @param Deferred $deferred
* @param number $timeout
* @param number $timeout
* @return void
*/
public function applyTimeout(Deferred $deferred, $timeout)
public function applyTimeout(Deferred $deferred, ClientRequestState $state, $timeout)
{
$deferred->timeout = $this->loop->addTimer($timeout, function () use ($timeout, $deferred) {
$state->timeout = $this->loop->addTimer($timeout, function () use ($timeout, $deferred, $state) {
$deferred->reject(new \RuntimeException(
'Request timed out after ' . $timeout . ' seconds'
));
if (isset($deferred->pending)) {
$deferred->pending->cancel();
unset($deferred->pending);
if ($state->pending !== null) {
$state->pending->cancel();
$state->pending = null;
}
});
}

private function next(RequestInterface $request, Deferred $deferred)
private function next(RequestInterface $request, Deferred $deferred, ClientRequestState $state)
{
$this->progress('request', array($request));

$that = $this;
++$deferred->numRequests;
++$state->numRequests;

$promise = $this->sender->send($request);

if (!$this->streaming) {
$promise = $promise->then(function ($response) use ($deferred, $that) {
return $that->bufferResponse($response, $deferred);
$promise = $promise->then(function ($response) use ($deferred, $state, $that) {
return $that->bufferResponse($response, $deferred, $state);
});
}

$deferred->pending = $promise;
$state->pending = $promise;

return $promise->then(
function (ResponseInterface $response) use ($request, $that, $deferred) {
return $that->onResponse($response, $request, $deferred);
function (ResponseInterface $response) use ($request, $that, $deferred, $state) {
return $that->onResponse($response, $request, $deferred, $state);
}
);
}

/**
* @internal
* @param ResponseInterface $response
* @return PromiseInterface Promise<ResponseInterface, Exception>
*/
public function bufferResponse(ResponseInterface $response, $deferred)
public function bufferResponse(ResponseInterface $response, Deferred $deferred, ClientRequestState $state)
{
$stream = $response->getBody();

Expand Down Expand Up @@ -205,26 +202,24 @@ function ($e) use ($stream, $maximumSize) {
}
);

$deferred->pending = $promise;
$state->pending = $promise;

return $promise;
}

/**
* @internal
* @param ResponseInterface $response
* @param RequestInterface $request
* @throws ResponseException
* @return ResponseInterface|PromiseInterface
*/
public function onResponse(ResponseInterface $response, RequestInterface $request, $deferred)
public function onResponse(ResponseInterface $response, RequestInterface $request, Deferred $deferred, ClientRequestState $state)
{
$this->progress('response', array($response, $request));

// follow 3xx (Redirection) response status codes if Location header is present and not explicitly disabled
// @link https://tools.ietf.org/html/rfc7231#section-6.4
if ($this->followRedirects && ($response->getStatusCode() >= 300 && $response->getStatusCode() < 400) && $response->hasHeader('Location')) {
return $this->onResponseRedirect($response, $request, $deferred);
return $this->onResponseRedirect($response, $request, $deferred, $state);
}

// only status codes 200-399 are considered to be valid, reject otherwise
Expand All @@ -242,19 +237,19 @@ public function onResponse(ResponseInterface $response, RequestInterface $reques
* @return PromiseInterface
* @throws \RuntimeException
*/
private function onResponseRedirect(ResponseInterface $response, RequestInterface $request, Deferred $deferred)
private function onResponseRedirect(ResponseInterface $response, RequestInterface $request, Deferred $deferred, ClientRequestState $state)
{
// resolve location relative to last request URI
$location = Uri::resolve($request->getUri(), $response->getHeaderLine('Location'));

$request = $this->makeRedirectRequest($request, $location);
$this->progress('redirect', array($request));

if ($deferred->numRequests >= $this->maxRedirects) {
if ($state->numRequests >= $this->maxRedirects) {
throw new \RuntimeException('Maximum number of redirects (' . $this->maxRedirects . ') exceeded');
}

return $this->next($request, $deferred);
return $this->next($request, $deferred, $state);
}

/**
Expand Down
3 changes: 3 additions & 0 deletions tests/HttpServerTest.php
Expand Up @@ -17,6 +17,9 @@ final class HttpServerTest extends TestCase
private $connection;
private $socket;

/** @var ?int */
private $called = null;

/**
* @before
*/
Expand Down
3 changes: 3 additions & 0 deletions tests/Io/StreamingServerTest.php
Expand Up @@ -17,6 +17,9 @@ class StreamingServerTest extends TestCase
private $connection;
private $socket;

/** @var ?int */
private $called = null;

/**
* @before
*/
Expand Down