We've been experiencing issues where the qemu websocket server closes
connections from noVNC clients for no apparent reason. Debugging shows
that certain web browsers are injecting ping and pong frames when the
connection becomes idle. Some browsers send those frames without a
payload, which also is causing closure. This patch series addresses
these issues by making the websocket server more conformant to RFC
6455 - The WebSocket Protocol.

Remembering the opcode is sufficient for handling fragmented frames from
the client, which may be introduced by an intermediary server/proxy.
Respond to pings and ignore pongs rather than close the connection as
many browsers use ping/pong to test an idle connection. Close
connections according to the RFC, including providing a reason code and
message to aid debugging of unexpected disconnects. Empty payloads
should not cause a disconnect.

While updating the websocket code, several other bugs were discovered
for which patches are also included early in the set.

Brandon Carpenter (6):
  io: Always remove an old channel watch before adding a new one
  io: Small updates in preparation for websocket changes
  io: Add support for fragmented websocket binary frames
  io: Allow empty websocket payload
  io: Ignore websocket PING and PONG frames
  io: Reply to ping frames

 include/io/channel-websock.h |   1 +
 io/channel-websock.c         | 207 ++++++++++++++++++++++---------------------
 ui/vnc-auth-vencrypt.c       |   3 +
 ui/vnc-ws.c                  |   6 ++
 ui/vnc.c                     |   4 +
 5 files changed, 122 insertions(+), 99 deletions(-)

-- 
2.14.1


-- 


CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, is 
for the sole use of the intended recipient(s) and may contain proprietary, 
confidential or privileged information or otherwise be protected by law. 
Any unauthorized review, use, disclosure or distribution is prohibited. If 
you are not the intended recipient, please notify the sender and destroy 
all copies and the original message.

On 09/08/2017 12:37 PM, Brandon Carpenter wrote:
> We've been experiencing issues where the qemu websocket server closes
> connections from noVNC clients for no apparent reason. Debugging shows
> that certain web browsers are injecting ping and pong frames when the
> connection becomes idle. Some browsers send those frames without a
> payload, which also is causing closure. This patch series addresses
> these issues by making the websocket server more conformant to RFC
> 6455 - The WebSocket Protocol.

meta-comment: sending v2 in-reply-to v1 makes it harder for automated
tooling to recognize that you are sending new patches.  We prefer that
v2 be a top-level thread.  More patch submission hints at:
https://wiki.qemu.org/index.php/Contribute/SubmitAPatch

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

My apologies. I read that document before submitting the patch, but 
there was a lot to take in. Would you like me to resend the patch 
without the references?
--
Brandon Carpenter | Software Engineer
Cypherpath, Inc.
400 Columbia Point Drive Ste 101 | Richland, Washington USA
Office: (650) 713-3060

On Fri, Sep 8, 2017 at 11:01 AM, Eric Blake <eblake@redhat.com> wrote:
> On 09/08/2017 12:37 PM, Brandon Carpenter wrote:
>>  We've been experiencing issues where the qemu websocket server 
>> closes
>>  connections from noVNC clients for no apparent reason. Debugging 
>> shows
>>  that certain web browsers are injecting ping and pong frames when 
>> the
>>  connection becomes idle. Some browsers send those frames without a
>>  payload, which also is causing closure. This patch series addresses
>>  these issues by making the websocket server more conformant to RFC
>>  6455 - The WebSocket Protocol.
> 
> meta-comment: sending v2 in-reply-to v1 makes it harder for automated
> tooling to recognize that you are sending new patches.  We prefer that
> v2 be a top-level thread.  More patch submission hints at:
> https://wiki.qemu.org/index.php/Contribute/SubmitAPatch
> 
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
> 

-- 


CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, is 
for the sole use of the intended recipient(s) and may contain proprietary, 
confidential or privileged information or otherwise be protected by law. 
Any unauthorized review, use, disclosure or distribution is prohibited. If 
you are not the intended recipient, please notify the sender and destroy 
all copies and the original message.

On 09/08/2017 01:11 PM, Brandon Carpenter wrote:
> My apologies. I read that document before submitting the patch, but
> there was a lot to take in. Would you like me to resend the patch
> without the references?

Not a problem - we all had to submit our first patch at some point in
time in the past :)

Re-sending at this point shouldn't be necessary unless a maintainer
specifically asks for it; it's more information for next time (if you
have to send a v3, for whatever reason).


> 
> CONFIDENTIALITY NOTICE: This e-mail message, including any attachments,
> is for the sole use of the intended recipient(s) and may contain
> proprietary, confidential or privileged information or otherwise be
> protected by law. Any unauthorized review, use, disclosure or
> distribution is prohibited. If you are not the intended recipient,
> please notify the sender and destroy all copies and the original message.

By the way, your employer's disclaimer is unenforceable on a
publicly-archived mailing list; still, it may be worth replying from a
person account to avoid the risk of someone refusing to reply to your
mail on principle.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org