Add supported protocols to server and client

[ChristianVermeulen]

We needed a bit more control over the handshake. Since it is quite common for websockets to adhere to a specific protocol I chose to specifically implement this. In the Client it was already possible to set a protocol in the handshake header, but the Server had no such way.

With this update it is now possible to define which protocols are supported by the server, which will be validated and replied to the client during the handshake.

[coveralls]

coverage: 99.466% (-0.5%) from 100.0%
when pulling 75fa385 on ChristianVermeulen:add-supported-sub-protocols
into b59cf5d on sirn-se:v2.1-main.

[sirn-se]

Thanks for the contribution!

But reading up on RFC the logic of this PR does not appear to be fully correct.

As far as I understand it, the Client may request one or more subprotocols by adding headers. But these should be considered as a "can I use any of these" request.
If the Server support any of the requested subprotocols, it should reply with a header indicating which it prefers. If it does not support any of them, it must not respond with a header.

So it would be up to the Client to close connection if it require a subprotocol the Server do not support.

As per https://datatracker.ietf.org/doc/html/rfc6455#section-1.9

The |Sec-WebSocket-Protocol| header field is used in the WebSocket opening handshake. It is sent from the client to the server and back from the server to the client to confirm the subprotocol of the connection. This enables scripts to both select a subprotocol and be sure that the server agreed to serve that subprotocol.

The |Sec-WebSocket-Protocol| header field MAY appear multiple times in an HTTP request (which is logically the same as a single |Sec-WebSocket-Protocol| header field that contains all values). However, the |Sec-WebSocket-Protocol| header field MUST NOT appear more than once in an HTTP response.

The Server may handle headers in current code by using the onConnect listener.
It is also possible to automize subprotocol negotiation by creating and adding a Middleware.

[ChristianVermeulen]

That makes sense. What do you prefer? Shall I edit the code so it just replies without the header if there is no match or completely remove it an add a middleware to add this feature to the server?

My personal preference would be to keep it natively supported since it is in the spec :-) Middleware can be used for anything specific to the implementation on top of the spec.

Let me know, i'll be happy to update!

[sirn-se]

As of version 2.x I've preferred a more modular design, to allow users to replace functionality if needed. As an example, I added the Ping/Pong and Close logic as standard Middlewares rather than incorporating them in the main code, even if they provide required functionality of the WebSocket protocol.

So my preference here would be a Middleware for Subprotocol negotiation. Then add it to the list of "standard" middlewares in documentation.

Available middlewares are added in src/Middleware/. For this it should implement the processHttpIncomingInterface.php and ProcessHttpOutgoingInterface.php interfaces.

[ChristianVermeulen]

Available middlewares are added in src/Middleware/. For this it should implement the processHttpIncomingInterface.php and ProcessHttpOutgoingInterface.php interfaces.

Maybe I misunderstand, but shouldn't it be the processIncomingInterface.php and ProcessOutgoingInterface.php

[sirn-se]

Those are middlewares for WebSocket messages. To get and modify the http request/response during handshake you would need the ones with "http" in the name.

As example, to add subprotocol headers to client request, you would do something like;

public function processHttpOutgoing(ProcessHttpStack $stack, Connection $connection, Message $message): Message
{
    // Only for outgoing requests by Client
    if ($message instance of Request) {
        foreach ($this->subprotocols as $subprotocol) {
            $message = $message->withHeader('Sec-WebSocket-Protocol', $subprotocol);
        }
    }
    // Continue processing, when all added middlewares are served it will write $message on connection
    return $stack->handleOutgoing($message);
}

[ChristianVermeulen]

Ah right, we are working with websockets at the moment, but i'll try to add both in there :-) I'll keep you posted!

[ChristianVermeulen]

@sirn-se After some testing and debugging, it seems a middleware can not be used. The performHandshake() in the Server-object creates a brand new instance of the response AND sends it right away without ever touching the middleware. On top of that, the handshake is performed before the middleware is called, so a middleware is too late in the process.

This means that changing the handshakeResponse in the middleware has no effect at all i'm afraid :-(.

I have updated the current method to be more aligned with the spec and not fail when there is no matching protocol.

Can you get behind this, or do you have other ideas?

[sirn-se]

@ChristianVermeulen The middleware stack can be a bit tricky at first. I use the same strategy as PSR-15: HTTP Server Request Handlers where a middleware need to consider to change things before or after it delegates further down the chain.

I did get it to work, prototype here; https://github.com/sirn-se/websocket-php/pull/35/files

Found a small bug though, repo currently do not allow repeated headers.

[ChristianVermeulen]

@sirn-se Ah, that works perfectly 👌 Thanks for that! I've been looking into writing tests but I can't seem to wrap my head around the StackTrait and it looks like we need to be able to test MiddleWare on server level instead of connection level as well 😅.

Do you have any idea when you might be able to release this?

[ChristianVermeulen]

closing in favor of #35.