Re: [PATCH] io/channel-websock: treat 'binary' and no sub-protocol as the same

["Daniel P. Berrangé" <berrange@redhat.com>]

On Sat, Nov 23, 2019 at 11:43:06AM +0800, Yu-Chen Lin wrote:
> noVNC doesn't use 'binary' protocol by default after
> commit c912230309806aacbae4295faf7ad6406da97617.
> 
> It will cause qemu return 400 when handshaking.
> 
> To overcome this problem and remain compatibility of
> older noVNC client.
> 
> We treat 'binary' and no sub-protocol as the same
> so that we can support different version of noVNC
> client.
> 
> Tested on noVNC before c912230 and after c912230.
> 
> Buglink: https://bugs.launchpad.net/qemu/+bug/1849644
> 
> Signed-off-by: Yu-Chen Lin <npes87184@gmail.com>
> ---
>  io/channel-websock.c | 35 +++++++++++++++++++++++------------
>  1 file changed, 23 insertions(+), 12 deletions(-)

Thank you, I have queued this for 5.0.

  Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

Also CC'ing stable for the first 4.2 bug fix release.

> 
> diff --git a/io/channel-websock.c b/io/channel-websock.c
> index fc36d44eba..918e09ea3f 100644
> --- a/io/channel-websock.c
> +++ b/io/channel-websock.c
> @@ -49,13 +49,20 @@
>      "Server: QEMU VNC\r\n"                       \
>      "Date: %s\r\n"
>  
> +#define QIO_CHANNEL_WEBSOCK_HANDSHAKE_WITH_PROTO_RES_OK \
> +    "HTTP/1.1 101 Switching Protocols\r\n"              \
> +    QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_COMMON            \
> +    "Upgrade: websocket\r\n"                            \
> +    "Connection: Upgrade\r\n"                           \
> +    "Sec-WebSocket-Accept: %s\r\n"                      \
> +    "Sec-WebSocket-Protocol: binary\r\n"                \
> +    "\r\n"
>  #define QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_OK    \
>      "HTTP/1.1 101 Switching Protocols\r\n"      \
>      QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_COMMON    \
>      "Upgrade: websocket\r\n"                    \
>      "Connection: Upgrade\r\n"                   \
>      "Sec-WebSocket-Accept: %s\r\n"              \
> -    "Sec-WebSocket-Protocol: binary\r\n"        \
>      "\r\n"
>  #define QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_NOT_FOUND \
>      "HTTP/1.1 404 Not Found\r\n"                    \
> @@ -336,6 +343,7 @@ qio_channel_websock_find_header(QIOChannelWebsockHTTPHeader *hdrs,
>  
>  static void qio_channel_websock_handshake_send_res_ok(QIOChannelWebsock *ioc,
>                                                        const char *key,
> +                                                      const bool use_protocols,
>                                                        Error **errp)
>  {
>      char combined_key[QIO_CHANNEL_WEBSOCK_CLIENT_KEY_LEN +
> @@ -361,8 +369,13 @@ static void qio_channel_websock_handshake_send_res_ok(QIOChannelWebsock *ioc,
>      }
>  
>      date = qio_channel_websock_date_str();
> -    qio_channel_websock_handshake_send_res(
> -        ioc, QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_OK, date, accept);
> +    if (use_protocols) {
> +            qio_channel_websock_handshake_send_res(
> +                ioc, QIO_CHANNEL_WEBSOCK_HANDSHAKE_WITH_PROTO_RES_OK, date, accept);
> +    } else {
> +            qio_channel_websock_handshake_send_res(
> +                ioc, QIO_CHANNEL_WEBSOCK_HANDSHAKE_RES_OK, date, accept);
> +    }
>  
>      g_free(date);
>      g_free(accept);
> @@ -387,10 +400,6 @@ static void qio_channel_websock_handshake_process(QIOChannelWebsock *ioc,
>  
>      protocols = qio_channel_websock_find_header(
>          hdrs, nhdrs, QIO_CHANNEL_WEBSOCK_HEADER_PROTOCOL);
> -    if (!protocols) {
> -        error_setg(errp, "Missing websocket protocol header data");
> -        goto bad_request;
> -    }
>  
>      version = qio_channel_websock_find_header(
>          hdrs, nhdrs, QIO_CHANNEL_WEBSOCK_HEADER_VERSION);
> @@ -430,10 +439,12 @@ static void qio_channel_websock_handshake_process(QIOChannelWebsock *ioc,
>      trace_qio_channel_websock_http_request(ioc, protocols, version,
>                                             host, connection, upgrade, key);
>  
> -    if (!g_strrstr(protocols, QIO_CHANNEL_WEBSOCK_PROTOCOL_BINARY)) {
> -        error_setg(errp, "No '%s' protocol is supported by client '%s'",
> -                   QIO_CHANNEL_WEBSOCK_PROTOCOL_BINARY, protocols);
> -        goto bad_request;
> +    if (protocols) {
> +            if (!g_strrstr(protocols, QIO_CHANNEL_WEBSOCK_PROTOCOL_BINARY)) {
> +                error_setg(errp, "No '%s' protocol is supported by client '%s'",
> +                           QIO_CHANNEL_WEBSOCK_PROTOCOL_BINARY, protocols);
> +                goto bad_request;
> +            }
>      }
>  
>      if (!g_str_equal(version, QIO_CHANNEL_WEBSOCK_SUPPORTED_VERSION)) {
> @@ -467,7 +478,7 @@ static void qio_channel_websock_handshake_process(QIOChannelWebsock *ioc,
>          goto bad_request;
>      }
>  
> -    qio_channel_websock_handshake_send_res_ok(ioc, key, errp);
> +    qio_channel_websock_handshake_send_res_ok(ioc, key, !!protocols, errp);
>      return;
>  
>   bad_request:
> -- 
> 2.17.1
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|