Skip to content

Commit

Permalink
Handle upgrade and CONNECT requests similarly
Browse files Browse the repository at this point in the history
  • Loading branch information
trowski committed Nov 21, 2019
1 parent 00498c2 commit f0da49c
Showing 1 changed file with 33 additions and 21 deletions.
54 changes: 33 additions & 21 deletions src/Connection/Http1Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ public function getStream(Request $request): Promise

private function free(): Promise
{
$this->socket = null;
$this->estimatedClose = 0;

if ($this->timeoutWatcher !== null) {
Expand Down Expand Up @@ -313,6 +314,7 @@ private function readResponse(
$response,
'connection'
));

if (!isset($connection['upgrade'])) {
throw new HttpException('Switching protocols response missing "Connection: upgrade" header');
}
Expand All @@ -321,33 +323,19 @@ private function readResponse(
throw new HttpException('Switching protocols response missing "Upgrade" header');
}

if (($onUpgrade = $request->getUpgradeHandler()) === null) {
throw new HttpException('Received switching protocols response without upgrade handler callback');
}

$socket = new UpgradedSocket($this->socket, $parser->getBuffer());
$this->socket = null;
asyncCall($onUpgrade, $socket, clone $request, $response);

$this->free(); // Close this connection without closing socket.

$trailersDeferred->resolve($trailers);

return $response;
return $this->handleUpgradeResponse($request, $response, $parser->getBuffer());
}

if ($status < Http\Status::OK) {
if ($status < Http\Status::OK) { // 1XX responses (excluding 101, handled above)
$chunk = $parser->getBuffer();
$parser = new Http1Parser($request, $bodyCallback, $trailersCallback);
goto parseChunk;
}

if ($status === Http\Status::OK && $request->getMethod() === 'CONNECT' && $request->getUpgradeHandler() !== null) {
$socket = new UpgradedSocket($this->socket, $parser->getBuffer());
$this->socket = null;
asyncCall($request->getUpgradeHandler(), $socket, clone $request, $response);

return $response;
if ($status >= Http\Status::OK && $status < Http\Status::MULTIPLE_CHOICES && $request->getMethod() === 'CONNECT') {
$trailersDeferred->resolve($trailers);
return $this->handleUpgradeResponse($request, $response, $parser->getBuffer());
}

$bodyCancellationSource = new CancellationTokenSource;
Expand Down Expand Up @@ -451,6 +439,28 @@ private function readResponse(
}
}

private function handleUpgradeResponse(Request $request, Response $response, string $buffer): Response
{
if (($onUpgrade = $request->getUpgradeHandler()) === null) {
throw new HttpException('CONNECT or upgrade request made without upgrade handler callback');
}

$socket = new UpgradedSocket($this->socket, $buffer);
$this->free(); // Mark this connection as unusable without closing socket.

asyncCall(function () use ($onUpgrade, $socket, $request, $response): \Generator {
try {
yield call($onUpgrade, $socket, clone $request, $response);
} catch (\Throwable $exception) {
throw new HttpException('Upgrade handler threw an exception', 0, $exception);
} finally {
$socket->close();

This comment has been minimized.

Copy link
@kelunik

kelunik Nov 23, 2019

Member

I guess we should only close on exception here, otherwise the promise returned from the upgrade handler must not return.

}
});

return $response;
}

/**
* @return int Approximate number of milliseconds remaining until the connection is closed.
*/
Expand Down Expand Up @@ -689,12 +699,14 @@ private function generateRawHeader(Request $request, string $protocolVersion): s
$requestUri .= '?' . $query;
}

if ($request->getMethod() === 'CONNECT') {
$method = $request->getMethod();

if ($method === 'CONNECT') {
$defaultPort = $uri->getScheme() === 'https' ? 443 : 80;
$requestUri = $uri->getHost() . ':' . ($uri->getPort() ?? $defaultPort);
}

$header = $request->getMethod() . ' ' . $requestUri . ' HTTP/' . $protocolVersion . "\r\n";
$header = $method . ' ' . $requestUri . ' HTTP/' . $protocolVersion . "\r\n";

try {
$header .= Rfc7230::formatHeaders($request->getHeaders());
Expand Down

0 comments on commit f0da49c

Please sign in to comment.