[Spice-devel] [PATCH spice-server 09/23] websocket: Better encapsulation
Frediano Ziglio
fziglio at redhat.com
Tue Jun 25 16:11:33 UTC 2019
Move websocket structure declarations to C file.
Make some functions static as now not used externally.
Introduce a websocket_free function for symmetry.
Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
---
server/red-stream.c | 31 ++--------------------
server/websocket.c | 64 ++++++++++++++++++++++++++++++++++++++++++---
server/websocket.h | 32 +++--------------------
3 files changed, 65 insertions(+), 62 deletions(-)
diff --git a/server/red-stream.c b/server/red-stream.c
index fd3fdced8..1e0103b08 100644
--- a/server/red-stream.c
+++ b/server/red-stream.c
@@ -436,7 +436,7 @@ void red_stream_free(RedStream *s)
SSL_free(s->priv->ssl);
}
- g_free(s->priv->ws);
+ websocket_free(s->priv->ws);
red_stream_remove_watch(s);
socket_close(s->socket);
@@ -1184,38 +1184,11 @@ static ssize_t stream_websocket_writev(RedStream *s, const struct iovec *iov, in
*/
bool red_stream_is_websocket(RedStream *stream, const void *buf, size_t len)
{
- char rbuf[4096];
- int rc;
-
if (stream->priv->ws) {
return false;
}
- memcpy(rbuf, buf, len);
- rc = stream->priv->read(stream, rbuf + len, sizeof(rbuf) - len - 1);
- if (rc <= 0) {
- return false;
- }
- len += rc;
- rbuf[len] = 0;
-
- /* TODO: this has a theoretical flaw around packet buffering
- that is not likely to occur in practice. That is,
- to be fully correct, we should repeatedly read bytes until
- either we get the end of the GET header (\r\n\r\n), or until
- an amount of time has passed. Instead, we just read for
- 16 bytes, and then read up to the sizeof rbuf. So if the
- GET request is only partially complete at this point we
- will fail.
-
- A typical GET request is 520 bytes, and it's difficult to
- imagine a real world case where that will come in fragmented
- such that we trigger this failure. Further, the spice reds
- code has no real mechanism to do variable length/time based reads,
- so it seems wisest to live with this theoretical flaw.
- */
-
- stream->priv->ws = websocket_new(rbuf, stream,
+ stream->priv->ws = websocket_new(buf, len, stream,
(websocket_read_cb_t) stream->priv->read,
(websocket_write_cb_t) stream->priv->write,
(websocket_writev_cb_t) stream->priv->writev);
diff --git a/server/websocket.c b/server/websocket.c
index 03d931023..01bcef82e 100644
--- a/server/websocket.c
+++ b/server/websocket.c
@@ -63,6 +63,31 @@
#define WEBSOCKET_GUID "258EAFA5-E914-47DA-95CA-C5AB0DC85B11"
+#define WEBSOCKET_MAX_HEADER_SIZE (1 + 9 + 4)
+
+typedef struct {
+ int type;
+ int masked;
+ uint8_t header[WEBSOCKET_MAX_HEADER_SIZE];
+ int header_pos;
+ int frame_ready:1;
+ uint8_t mask[4];
+ uint64_t relayed;
+ uint64_t expected_len;
+} websocket_frame_t;
+
+struct RedsWebSocket {
+ bool closed;
+
+ websocket_frame_t read_frame;
+ uint64_t write_remainder;
+
+ void *raw_stream;
+ websocket_read_cb_t raw_read;
+ websocket_write_cb_t raw_write;
+ websocket_writev_cb_t raw_writev;
+};
+
static void websocket_ack_close(void *opaque, websocket_write_cb_t write_cb);
/* Perform a case insensitive search for needle in haystack.
@@ -453,7 +478,7 @@ static void websocket_ack_close(void *opaque, websocket_write_cb_t write_cb)
write_cb(opaque, header, sizeof(header));
}
-bool websocket_is_start(char *buf)
+static bool websocket_is_start(char *buf)
{
if (strncmp(buf, "GET ", 4) == 0 &&
// TODO strip, do not assume a single space
@@ -466,7 +491,7 @@ bool websocket_is_start(char *buf)
return false;
}
-void websocket_create_reply(char *buf, char *outbuf)
+static void websocket_create_reply(char *buf, char *outbuf)
{
char *key;
@@ -479,9 +504,35 @@ void websocket_create_reply(char *buf, char *outbuf)
g_free(key);
}
-RedsWebSocket *websocket_new(char *rbuf, struct RedStream *stream, websocket_read_cb_t read_cb,
+RedsWebSocket *websocket_new(const void *buf, size_t len, void *stream, websocket_read_cb_t read_cb,
websocket_write_cb_t write_cb, websocket_writev_cb_t writev_cb)
{
+ char rbuf[4096];
+
+ memcpy(rbuf, buf, len);
+ int rc = read_cb(stream, rbuf + len, sizeof(rbuf) - len - 1);
+ if (rc <= 0) {
+ return NULL;
+ }
+ len += rc;
+ rbuf[len] = 0;
+
+ /* TODO: this has a theoretical flaw around packet buffering
+ that is not likely to occur in practice. That is,
+ to be fully correct, we should repeatedly read bytes until
+ either we get the end of the GET header (\r\n\r\n), or until
+ an amount of time has passed. Instead, we just read for
+ 16 bytes, and then read up to the sizeof rbuf. So if the
+ GET request is only partially complete at this point we
+ will fail.
+
+ A typical GET request is 520 bytes, and it's difficult to
+ imagine a real world case where that will come in fragmented
+ such that we trigger this failure. Further, the spice reds
+ code has no real mechanism to do variable length/time based reads,
+ so it seems wisest to live with this theoretical flaw.
+ */
+
if (!websocket_is_start(rbuf)) {
return NULL;
}
@@ -489,7 +540,7 @@ RedsWebSocket *websocket_new(char *rbuf, struct RedStream *stream, websocket_rea
char outbuf[1024];
websocket_create_reply(rbuf, outbuf);
- int rc = write_cb(stream, outbuf, strlen(outbuf));
+ rc = write_cb(stream, outbuf, strlen(outbuf));
if (rc != strlen(outbuf)) {
return NULL;
}
@@ -503,3 +554,8 @@ RedsWebSocket *websocket_new(char *rbuf, struct RedStream *stream, websocket_rea
return ws;
}
+
+void websocket_free(RedsWebSocket *ws)
+{
+ g_free(ws);
+}
diff --git a/server/websocket.h b/server/websocket.h
index ab9eb8126..b586e3035 100644
--- a/server/websocket.h
+++ b/server/websocket.h
@@ -15,41 +15,15 @@
* License along with this library; if not, see <http://www.gnu.org/licenses/>.
*/
-#define WEBSOCKET_MAX_HEADER_SIZE (1 + 9 + 4)
-
-struct RedStream;
-
-typedef struct {
- int type;
- int masked;
- uint8_t header[WEBSOCKET_MAX_HEADER_SIZE];
- int header_pos;
- int frame_ready:1;
- uint8_t mask[4];
- uint64_t relayed;
- uint64_t expected_len;
-} websocket_frame_t;
-
typedef ssize_t (*websocket_read_cb_t)(void *opaque, void *buf, size_t nbyte);
typedef ssize_t (*websocket_write_cb_t)(void *opaque, const void *buf, size_t nbyte);
typedef ssize_t (*websocket_writev_cb_t)(void *opaque, struct iovec *iov, int iovcnt);
-typedef struct {
- int closed;
-
- websocket_frame_t read_frame;
- uint64_t write_remainder;
-
- struct RedStream *raw_stream;
- websocket_read_cb_t raw_read;
- websocket_write_cb_t raw_write;
- websocket_writev_cb_t raw_writev;
-} RedsWebSocket;
+typedef struct RedsWebSocket RedsWebSocket;
-RedsWebSocket *websocket_new(char *buf, struct RedStream *s, websocket_read_cb_t read_cb,
+RedsWebSocket *websocket_new(const void *buf, size_t len, void *stream, websocket_read_cb_t read_cb,
websocket_write_cb_t write_cb, websocket_writev_cb_t writev_cb);
-bool websocket_is_start(char *buf);
-void websocket_create_reply(char *buf, char *outbuf);
+void websocket_free(RedsWebSocket *ws);
int websocket_read(RedsWebSocket *ws, uint8_t *buf, size_t len);
int websocket_write(RedsWebSocket *ws, const void *buf, size_t len);
int websocket_writev(RedsWebSocket *ws, const struct iovec *iov, int iovcnt);
--
2.20.1
More information about the Spice-devel
mailing list