From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41770) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZYyh7-0004As-BJ for qemu-devel@nongnu.org; Mon, 07 Sep 2015 11:50:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZYyh4-00027p-Kf for qemu-devel@nongnu.org; Mon, 07 Sep 2015 11:50:57 -0400 Received: from mx1.redhat.com ([209.132.183.28]:36788) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZYyh3-00027S-UP for qemu-devel@nongnu.org; Mon, 07 Sep 2015 11:50:54 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 856508EA27 for ; Mon, 7 Sep 2015 15:50:53 +0000 (UTC) Date: Mon, 7 Sep 2015 16:50:49 +0100 From: "Daniel P. Berrange" Message-ID: <20150907155049.GT29882@redhat.com> References: <1441294768-8712-1-git-send-email-berrange@redhat.com> <1441294768-8712-14-git-send-email-berrange@redhat.com> <20150907154417.GE2337@work-vm> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20150907154417.GE2337@work-vm> Subject: Re: [Qemu-devel] [PATCH FYI 13/46] io: add QIOChannelWebsock class Reply-To: "Daniel P. Berrange" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert" Cc: Juan Quintela , qemu-devel@nongnu.org, Gerd Hoffmann , Amit Shah , Paolo Bonzini On Mon, Sep 07, 2015 at 04:44:18PM +0100, Dr. David Alan Gilbert wrote: > * Daniel P. Berrange (berrange@redhat.com) wrote: > > Add a QIOChannel subclass that can run the websocket protocol over > > the top of another QIOChannel instance. This initial implementation > > is only capable of acting as a websockets server. There is no support > > for acting as a websockets client yet. > > > > Signed-off-by: Daniel P. Berrange > > --- > > include/io/channel-websock.h | 108 +++++ > > io/Makefile.objs | 1 + > > io/channel-websock.c | 965 +++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 1074 insertions(+) > > create mode 100644 include/io/channel-websock.h > > create mode 100644 io/channel-websock.c > > diff --git a/io/channel-websock.c b/io/channel-websock.c > > new file mode 100644 > > index 0000000..0345b90 > > --- /dev/null > > +++ b/io/channel-websock.c > > @@ -0,0 +1,965 @@ > > + > > +#include "io/channel-websock.h" > > +#include "crypto/hash.h" > > + > > +#include > > + > > +/* #define DEBUG_IOC */ > > + > > +#ifdef DEBUG_IOC > > +#define DPRINTF(fmt, ...) \ > > + do { fprintf(stderr, "channel-websock: " fmt, ## __VA_ARGS__); } while (0) > > +#else > > +#define DPRINTF(fmt, ...) \ > > + do { } while (0) > > +#endif > > + > > +/* Max amount to allow in rawinput/rawoutput buffers */ > > +#define QIO_CHANNEL_WEBSOCK_MAX_BUFFER 8192 > > + > > +#define B64LEN(__x) (((__x + 2) / 3) * 12 / 3) > > Where is that magic calculation used? I copied these constants from the current ui/vnc-ws.h header file originally. In there it was referenced by another constant WS_ACCEPT_LEN, which was itself unused. I deleted WS_ACCEPT_LEN when I found it unused, but forgot to delete this B64LEN too. > > +static int qio_channel_websock_handshake_read(QIOChannelWebsock *ioc, > > + Error **errp) > > +{ > > + char *handshake_end; > > + ssize_t ret; > > + /* Typical HTTP headers from novnc are 512 bytes, so limiting > > + * total header size to 4096 is easily enough. */ > > + size_t want = 4096 - ioc->encinput.offset; > > + qio_buffer_reserve(&ioc->encinput, want); > > + ret = qio_channel_read(ioc->master, > > + (char *)qio_buffer_end(&ioc->encinput), want, errp); > > + if (ret < 0) { > > + return -1; > > + } > > + ioc->encinput.offset += ret; > > + > > + handshake_end = g_strstr_len((char *)ioc->encinput.buffer, > > + ioc->encinput.offset, > > + QIO_CHANNEL_WEBSOCK_HANDSHAKE_END); > > + if (!handshake_end) { > > + if (ioc->encinput.offset >= 4096) { > > + error_setg(errp, "%s", > > + _("End of headers not found in first 4096 bytes")); > > + return -1; > > + } else { > > + return 0; > > + } > > + } > > + > > + if (qio_channel_websock_handshake_process(ioc, > > + (char *)ioc->encinput.buffer, > > + ioc->encinput.offset, > > + errp) < 0) { > > + return -1; > > + } > > + > > + qio_buffer_advance(&ioc->encinput, > > + handshake_end - (char *)ioc->encinput.buffer + > > + strlen(QIO_CHANNEL_WEBSOCK_HANDSHAKE_END)); > > + return 1; > > Can you comment the return values for this function; I guess -1 is error, > 1 is good, what's 0 ? 1 means we've read the full handshake response, 0 means we need to receive more data still. > > +static void qio_channel_websock_encode(QIOChannelWebsock *ioc) > > +{ > > + size_t header_size = 0; > > + unsigned char opcode = QIO_CHANNEL_WEBSOCK_OPCODE_BINARY_FRAME; > > + union { > > + char buf[QIO_CHANNEL_WEBSOCK_HEAD_MAX_LEN]; > > + QIOChannelWebsockHeader ws; > > + } header; > > + > > + DPRINTF("Encoding pending raw output %zu %p\n", > > + ioc->rawoutput.offset, ioc); > > + if (!ioc->rawoutput.offset) { > > + return; > > + } > > + > > + header.ws.b0 = 0x80 | (opcode & 0x0f); > > There are quite a few magic header sizes 125, (and I think I saw > some other sizes below) - some comments on them, or constants? This code was derived from ui/vnc-ws.c which is full of magic, so I've mostly just inherited decisions made by that original author. I'll fix up this new code to be less magic and use constants and/or comments to clarify > > > + if (ioc->rawoutput.offset <= 125) { > > Dave > > > + header.ws.b1 = (uint8_t)ioc->rawoutput.offset; > > + header_size = 2; > > + } else if (ioc->rawoutput.offset < 65536) { > > + header.ws.b1 = 0x7e; > > + header.ws.u.s16.l16 = cpu_to_be16((uint16_t)ioc->rawoutput.offset); > > + header_size = 4; > > + } else { > > + header.ws.b1 = 0x7f; > > + header.ws.u.s64.l64 = cpu_to_be64(ioc->rawoutput.offset); > > + header_size = 10; > > + } > > + > > + qio_buffer_reserve(&ioc->encoutput, header_size + ioc->rawoutput.offset); > > + qio_buffer_append(&ioc->encoutput, header.buf, header_size); > > + qio_buffer_append(&ioc->encoutput, ioc->rawoutput.buffer, > > + ioc->rawoutput.offset); > > + qio_buffer_reset(&ioc->rawoutput); > > + DPRINTF("Have %zu bytes encoded output %p\n", > > + ioc->encoutput.offset, ioc); > > +} Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|