Skip to content

Commit

Permalink
Reintroduce WebsocketAcceptor interface
Browse files Browse the repository at this point in the history
Provides separation between RequestHandler and the different purpose of these classes.
  • Loading branch information
trowski committed Sep 9, 2023
1 parent 3235078 commit 8a57456
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 40 deletions.
9 changes: 4 additions & 5 deletions src/AllowOriginAcceptor.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,9 @@
use Amp\Http\HttpStatus;
use Amp\Http\Server\ErrorHandler;
use Amp\Http\Server\Request;
use Amp\Http\Server\RequestHandler;
use Amp\Http\Server\Response;

final class AllowOriginAcceptor implements RequestHandler
final class AllowOriginAcceptor implements WebsocketAcceptor
{
use ForbidCloning;
use ForbidSerialization;
Expand All @@ -21,16 +20,16 @@ final class AllowOriginAcceptor implements RequestHandler
public function __construct(
private readonly array $allowOrigins,
private readonly ErrorHandler $errorHandler = new Internal\UpgradeErrorHandler(),
private readonly RequestHandler $acceptor = new Rfc6455Acceptor(),
private readonly WebsocketAcceptor $acceptor = new Rfc6455Acceptor(),
) {
}

public function handleRequest(Request $request): Response
public function handleHandshake(Request $request): Response
{
if (!\in_array($request->getHeader('origin'), $this->allowOrigins, true)) {
return $this->errorHandler->handleError(HttpStatus::FORBIDDEN, 'Origin forbidden', $request);
}

return $this->acceptor->handleRequest($request);
return $this->acceptor->handleHandshake($request);
}
}
29 changes: 2 additions & 27 deletions src/Rfc6455Acceptor.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,10 @@
use Amp\Http\HttpStatus;
use Amp\Http\Server\ErrorHandler;
use Amp\Http\Server\Request;
use Amp\Http\Server\RequestHandler;
use Amp\Http\Server\Response;
use function Amp\Websocket\generateAcceptFromKey;

final class Rfc6455Acceptor implements RequestHandler
final class Rfc6455Acceptor implements WebsocketAcceptor
{
use ForbidCloning;
use ForbidSerialization;
Expand All @@ -20,31 +19,7 @@ public function __construct(private readonly ErrorHandler $errorHandler = new In
{
}

/**
* Respond to websocket handshake requests.
*
* If a websocket application doesn't wish to impose any special constraints on the
* handshake it doesn't have to do anything beyond the request handling provided by this
* method. All valid websocket handshakes will be accepted. It is recommended to do
* validation of the connected client as part of the protocol implemented by the
* {@see WebsocketClientHandler}.
*
* Most web applications should check the `origin` header to restrict access,
* as websocket connections aren't subject to browser's same-origin-policy. See
* {@see AllowOriginAcceptor} for such an implementation.
*
* Another implementation may delegate to this class to accept the client, then validate
* the cookies of the request before returning the response provided by the method
* below, or returning an error response.
*
* The response provided by the upgrade handler is made available to {@see WebsocketClientHandler::handleClient()}.
*
* @param Request $request The HTTP request that instigated the handshake
*
* @return Response Return a response with a status code other than
* {@link HttpStatus::SWITCHING_PROTOCOLS} to deny the handshake request.
*/
public function handleRequest(Request $request): Response
public function handleHandshake(Request $request): Response
{
if ($request->getMethod() !== 'GET') {
$response = $this->errorHandler->handleError(HttpStatus::METHOD_NOT_ALLOWED, request: $request);
Expand Down
4 changes: 2 additions & 2 deletions src/Websocket.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ final class Websocket implements RequestHandler
public function __construct(
HttpServer $httpServer,
private readonly PsrLogger $logger,
private readonly RequestHandler $acceptor,
private readonly WebsocketAcceptor $acceptor,
private readonly WebsocketClientHandler $clientHandler,
private readonly WebsocketClientFactory $clientFactory = new Rfc6455ClientFactory(),
) {
Expand All @@ -42,7 +42,7 @@ public function __construct(

public function handleRequest(Request $request): Response
{
$response = $this->acceptor->handleRequest($request);
$response = $this->acceptor->handleHandshake($request);

if ($response->getStatus() !== HttpStatus::SWITCHING_PROTOCOLS) {
$response->removeHeader('sec-websocket-accept');
Expand Down
36 changes: 36 additions & 0 deletions src/WebsocketAcceptor.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?php declare(strict_types=1);

namespace Amp\Websocket\Server;

use Amp\Http\Server\Request;
use Amp\Http\Server\Response;

interface WebsocketAcceptor
{
/**
* Respond to websocket handshake requests.
*
* If a websocket application doesn't wish to impose any special constraints on the
* handshake it may use {@see Rfc6455Acceptor} to accept websocket requests on a
* {@see Websocket} endpoint.
*
* Most web applications should check the `origin` header to restrict access,
* as websocket connections aren't subject to browser's same-origin-policy. See
* {@see AllowOriginAcceptor} for such an implementation.
*
* This method provides an opportunity to set application-specific headers, including
* cookies, on the websocket response. Although any non-101 status code can be used
* to reject the websocket connection, we generally recommended using a 4xx status
* code that is descriptive of why the handshake was rejected. It is suggested that an
* instance of {@see ErrorHandler} is used to generate such a response.
*
* The response provided by the upgrade handler is made available to
* {@see WebsocketClientHandler::handleClient()}.
*
* @param Request $request The websocket HTTP handshake request.
*
* @return Response Return a response with a status code other than
* {@link HttpStatus::SWITCHING_PROTOCOLS} to deny the handshake request.
*/
public function handleHandshake(Request $request): Response;
}
11 changes: 5 additions & 6 deletions test/WebsocketTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
use Amp\Http\Server\Driver\Client as HttpClient;
use Amp\Http\Server\ErrorHandler;
use Amp\Http\Server\Request;
use Amp\Http\Server\RequestHandler;
use Amp\Http\Server\Response;
use Amp\Http\Server\SocketHttpServer;
use Amp\PHPUnit\AsyncTestCase;
Expand Down Expand Up @@ -110,20 +109,20 @@ public function handleClient(WebsocketClient $client, Request $request, Response
*/
public function testHandshake(Request $request, int $status, array $expectedHeaders = []): void
{
$upgradeHandler = $this->createMock(RequestHandler::class);
$acceptor = $this->createMock(WebsocketAcceptor::class);

$delegateHandler = new Rfc6455Acceptor();
$upgradeHandler->expects(self::once())
->method('handleRequest')
->willReturnCallback($delegateHandler->handleRequest(...));
$acceptor->expects(self::once())
->method('handleHandshake')
->willReturnCallback($delegateHandler->handleHandshake(...));

$logger = new NullLogger();
$server = SocketHttpServer::createForDirectAccess($logger);
$server->expose(new Socket\InternetAddress('127.0.0.1', 0));
$websocket = new Websocket(
httpServer: $server,
logger: $logger,
acceptor: $upgradeHandler,
acceptor: $acceptor,
clientHandler: $this->createMock(WebsocketClientHandler::class),
);
$server->start($websocket, $this->createMock(ErrorHandler::class));
Expand Down

0 comments on commit 8a57456

Please sign in to comment.