How to process with "Connection: upgrade" requests

[mtymek]

Hi Aaron,

I'm playing around with building simple WebSocket implementation over icicle libs - for fun and learning purposes. I was able to easily hack something around base icicle library, but interacting with icicle/http is a different story. I can complete the handshake and send headers back, but after that I'm not able to read input anymore. Here's what I'm trying to do:

class WebSocketMiddleware
{
    public function __invoke(RequestInterface $request, ClientInterface $client)
    {
        echo "Handshake\n";
        $response = new Response("101");
        $response = $response->withHeader('Upgrade', 'websocket')
            ->withHeader('Connection', 'Upgrade')
            ->withHeader(
                "Sec-WebSocket-Accept",
                base64_encode(
                    sha1(
                        $request->getHeaderLine('Sec-WebSocket-Key') . "258EAFA5-E914-47DA-95CA-C5AB0DC85B11",
                        true
                    )
                )
            );

        yield $response;

        echo "Read loop\n";
        $stream = $request->getBody();
        while ($stream->isReadable()) {
            // this does nothing
            $x = (yield $stream->read(1));
            echo $x, "\n";
        }
    }
}

$server = new Server(new WebSocketMiddleware());
$server->listen(8080);
Loop\run();

After sending response headers, I want to echo everything I receive from user, but nothing happens. Can you point me in right direction?

[trowski]

The callback given to Server is expected to return either a ResponseInterface object or a promise that resolves to a ResponseInterface. This means if the function is a generator, the last thing it yields must be the ResponseInterface object (this will become so much clearer in PHP 7 when generators can return values).

So the issue in the code above is that the coroutine yields the Response object, but then continues trying to read from the client. Server doesn't actually send the response until the callback function returns (or coroutine finishes). So to accomplish what you want, another coroutine must be created to which the first does not yield.

class WebSocketMiddleware
{
    public function __invoke(RequestInterface $request, ClientInterface $client)
    {
        echo "Handshake\n";
        $response = new Response(101);
        $response = $response->withHeader('Upgrade', 'websocket')
            ->withHeader('Connection', 'Upgrade')
            ->withHeader(
                "Sec-WebSocket-Accept",
                base64_encode(
                    sha1(
                        $request->getHeaderLine('Sec-WebSocket-Key')
                        . "258EAFA5-E914-47DA-95CA-C5AB0DC85B11",
                        true
                    )
                )
            );

        // Start a separate coroutine that the current coroutine does not yield.
        // Use $client, not $request->getBody() as that stream will be empty.
        (new Coroutine($this->process($client)))->done();

        yield $response;
    }

    private function process(ClientInterface $client)
    {
        echo "Read loop\n";
        while ($client->isReadable()) {
            // This will now echo bytes read from the WebSocket.
            $x = (yield $client->read(1));
            echo bin2hex($x), "\n";
        }
    }
}

$server = new Server(new WebSocketMiddleware());
$server->listen(8080);

Loop\run();

I may add another parameter callable $onUpgrade = null to the Server constructor that is called if the connection header is upgrade. That callback could initiate the second Coroutine and would only be called once the response has been successfully sent to the client.

A WebSocket component for Icicle is near the top of my todo list. I have some code written from almost 2 years ago based on what eventually became Icicle, but it uses callbacks instead of coroutines so I have some refactoring to do. Would you be interested in contributing or testing?

[mtymek]

Thanks for your reply!

Indeed working with coroutines is not always straightforward, sometimes feels a bit like a magic. Still, in principle it is much easier to understand and work with than React PHP and its event system.

Currently my knowledge about WS details is not great (I actually wanted to write simple implementation to learn this protocol from the very bottom), and my time is limited, but I think I can still be helpful. Put what you have on Github, and I'll see where to start :)

[trowski]

I'll revise the WebSocket code I wrote sometime in the next week so it works with Icicle. I'll make another post here when I actually put something up on GitHub.

I added an $onUpgrade callback to the constructor for Server, so that the code from before could become this:

$server = new Server(
    function (RequestInterface $request) {
        echo "Handshake\n";
        $response = new Response(101);
        $response = $response->withHeader('Upgrade', 'websocket')
            ->withHeader('Connection', 'Upgrade')
            ->withHeader(
                "Sec-WebSocket-Accept",
                base64_encode(
                    sha1(
                        $request->getHeaderLine('Sec-WebSocket-Key')
                        . "258EAFA5-E914-47DA-95CA-C5AB0DC85B11",
                        true
                    )
                )
            );

        return $response;
    },
    null,
    function (ClientInterface $client) {
        echo "Read loop\n";
        while ($client->isReadable()) {
            // This will now echo bytes read from the WebSocket.
            $x = (yield $client->read(1));
            echo bin2hex($x), "\n";
        }
    }
);

$server->listen(8080);

Loop\run();

$onUpgrade is called only if the Connection header of the response object is upgrade and only once the response has been successfully written to the client stream. Thoughts?

[mtymek]

Looks good and logical for me, at least with my current knowledge of HTTP internals :-) I'll try to build something out of it and come back to you if I have any questions.

[mtymek]

Playing around with updated lib, I found that onUpgrade callback makes it less flexible.

My use case: when new user connects, I want to access Request object at any time (even when I receive WS frame later). In order to do so, I want to use your original solution (coroutine):

    public function handshake(RequestInterface $request, ClientInterface $client)
    {
        if ($request->getHeaderLine('Connection') != 'Upgrade') {
            $response = new Response(426);
            return $response;
        }

        if (strtolower($request->getHeaderLine('Upgrade') != 'websocket')
            || !$request->getHeaderLine('Sec-WebSocket-Key')
        ) {
            $response = new Response(400);
            return $response;
        }

        $response = new Response(101);
        $response = $response->withHeader('Upgrade', 'websocket')
            ->withHeader('Connection', 'Upgrade')
            ->withHeader(
                "Sec-WebSocket-Accept",
                base64_encode(
                    sha1(
                        $request->getHeaderLine('Sec-WebSocket-Key')
                        . "258EAFA5-E914-47DA-95CA-C5AB0DC85B11",
                        true
                    )
                )
            );

        $connection = new WebSocketConnection($request, $client);

        (new Coroutine($this->handleWsRequest($connection)))->done();

        return $response;
    }

After your latest patch, I get following fatal error:

PHP Fatal error:  Uncaught exception 'Icicle\Http\Exception\LogicException' with message 'No callback given for upgrade responses.' in /home/mat/public_html/icicle-websocket/vendor/icicleio/http/src/Server/Server.php:299
Stack trace:
#0 [internal function]: Icicle\Http\Server\Server::upgrade(Object(Icicle\Socket\Client\Client))
#1 /home/mat/public_html/icicle-websocket/vendor/icicleio/icicle/src/Coroutine/Coroutine.php(74): Generator->current()
#2 /home/mat/public_html/icicle-websocket/vendor/icicleio/icicle/src/Loop/Structures/CallableQueue.php(106): Icicle\Coroutine\Coroutine->Icicle\Coroutine\{closure}()
#3 /home/mat/public_html/icicle-websocket/vendor/icicleio/icicle/src/Loop/AbstractLoop.php(205): Icicle\Loop\Structures\CallableQueue->call()
#4 /home/mat/public_html/icicle-websocket/vendor/icicleio/icicle/src/Loop/AbstractLoop.php(225): Icicle\Loop\AbstractLoop->tick()
#5 /home/mat/public_html/icicle-websocket/vendor/icicleio/icicle/src/Loop/functions.php(92): Icicle\Loop\AbstractLoop->run()
#6 /home/mat/public_html/icicl in /home/mat/public_html/icicle-websocket/vendor/icicleio/http/src/Server/Server.php on line 299

Process finished with exit code 255

Of course I could do some workaround, but likely it won't be so elegant anymore. Suggestions?

[trowski]

I was thinking I should have passed the request and response objects to the onUpgrade callback, looks like I should have. I pushed another update, upgrade and try out the code below, with the upgrade method as the onUpdate callback. I've designed it so that onUpgrade should not return (or the coroutine should run) until the client should be disconnected, let me know how that works for you.

public function handshake(RequestInterface $request, ClientInterface $client)
{
    if ($request->getHeaderLine('Connection') != 'Upgrade') {
        $response = new Response(426);
        return $response;
    }

    if (strtolower($request->getHeaderLine('Upgrade') != 'websocket')
        || !$request->getHeaderLine('Sec-WebSocket-Key')
    ) {
        $response = new Response(400);
        return $response;
    }

    $response = new Response(101);
    $response = $response->withHeader('Upgrade', 'websocket')
        ->withHeader('Connection', 'Upgrade')
        ->withHeader(
            "Sec-WebSocket-Accept",
            base64_encode(
                sha1(
                    $request->getHeaderLine('Sec-WebSocket-Key')
                    . "258EAFA5-E914-47DA-95CA-C5AB0DC85B11",
                    true
                )
            )
        );

    return $response;
}

public function upgrade(
    RequestInterface $request,
    ResponseInterface $response,
    ClientInterface $client
) {
    $connection = new WebSocketConnection($request, $client);

    yield $this->handleWsRequest($connection);
}

[mtymek]

I managed to get simple websocket server to work. It allows to add nice handlers that can do something with incoming connection:

class WebSocketEcho
{
    public function __invoke(WebSocketConnection $connection, Frame $frame)
    {
        yield $connection->send(
            sprintf(
                "TO %s:%s: ECHO: %s",
                $connection->getClient()->getRemoteAddress(),
                $connection->getClient()->getRemotePort(),
                $frame->getData()
            )
        );
    }
}

here's my onConnection that does the processing:

public function handleWsRequest(
    RequestInterface $request,
    ResponseInterface $response,
    ClientInterface $client
) {
    $parser = new FrameReader();
    $frame = (yield $parser->read($client));

    if ($handler = $this->onMessage) {
    yield $handler(new WebSocketConnection($request, $response, $client), $frame);
    }
}

(Full class is here)

WebSocketConnection is a helper class aggregating all request-sepcifics (request, response and client) and provides handy function send that creates WebSocket frame.

What is still not perfect is creating WebSocketConnection every time new data frame arrives. At some point it will have to be persisted somewhere, to make implementing things like WAMP easier (and I really don't want to copy SplObjectStorage horror from Ratchet :-( )

What do you think? Maybe responsibility for upgrade process should be moved to another class?

[trowski]

I envisioned onUpgrade to be used to create something like a WebSocketClient object that would loop, reading frames from the WebSocket connection, sort of like an event loop for that client (though written as a coroutine, it would execute cooperatively with other connected clients). What you've written will only echo the first message and then closes the connection. Quickly working with what you wrote, handleWsRequest() should look more like this (also changing $this->onMessage so it cannot be null):

    public function handleWsRequest(
        RequestInterface $request,
        ResponseInterface $response,
        ClientInterface $client
    ) {
        $connection = new WebSocketConnection($request, $response, $client);
        $parser = new FrameReader();
        $handler = $this->onMessage;

        while ($frame = (yield $parser->read($client))) {
            yield $handler($connection, $frame);
        }
    }

The rest of what you've written looks pretty good though. I'll post what I have this weekend or early next week. Hopefully we can work on it together and build a solid WebSocket implementation.

[mtymek]

Thanks, it's exactly what I was looking for! Looking forward to see your WS code.