res_http_websocket: read/write string fixup

There was a problem when reading a string from the websocket. It assumed the
received data had a null terminator and tried to write the data to an ast_str.
This of course could/would read past the end of the given buffer while
writing the data to the internal buffer of ast_str. Modified the the code to
correctly place a null terminator on the result string.

git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@416394 65c4cc65-6c06-0410-ace0-fbb531ad65f3

diff --git a/include/asterisk/http_websocket.h b/include/asterisk/http_websocket.h
index d95e6068ebd8b51ef365d147a3e3c3d3e4482448..074ae120240bbb7e67e61f6a1e43d004fca02ac2 100644 (file)
--- a/include/asterisk/http_websocket.h
+++ b/include/asterisk/http_websocket.h
@@ -156,6 +156,8 @@ AST_OPTIONAL_API(int, ast_websocket_read, (struct ast_websocket *session, char *
 /*!
  * \brief Read a WebSocket frame containing string data.
  *
+ * \note The caller is responsible for freeing the output "buf".
+ *
  * \param ws pointer to the websocket
  * \param buf string buffer to populate with data read from socket
  * \retval -1 on error
@@ -164,7 +166,7 @@ AST_OPTIONAL_API(int, ast_websocket_read, (struct ast_websocket *session, char *
  * \note Once an AST_WEBSOCKET_OPCODE_CLOSE opcode is received the socket will be closed
  */
 AST_OPTIONAL_API(int, ast_websocket_read_string,
-                (struct ast_websocket *ws, struct ast_str **buf),
+                (struct ast_websocket *ws, char **buf),
                 { errno = ENOSYS; return -1;});
 
 /*!
@@ -189,7 +191,7 @@ AST_OPTIONAL_API(int, ast_websocket_write, (struct ast_websocket *session, enum
  * \retval -1 if error occurred
  */
 AST_OPTIONAL_API(int, ast_websocket_write_string,
-                (struct ast_websocket *ws, const struct ast_str *buf),
+                (struct ast_websocket *ws, const char *buf),
                 { errno = ENOSYS; return -1;});
 /*!
  * \brief Close a WebSocket session by sending a message with the CLOSE opcode and an optional code

diff --git a/res/res_http_websocket.c b/res/res_http_websocket.c
index 744521507b850129d1f14635b381b90c81fd08c9..07fcd9e2ee66a68103c250be0d5c1f0d56b7818f 100644 (file)
--- a/res/res_http_websocket.c
+++ b/res/res_http_websocket.c
@@ -497,7 +497,6 @@ int AST_OPTIONAL_API_NAME(ast_websocket_read)(struct ast_websocket *session, cha
                if (ws_safe_read(session, (*payload), (*payload_len), opcode)) {
                        return 0;
                }
-
                /* If a mask is present unmask the payload */
                if (mask_present) {
                        unsigned int pos;
@@ -1199,19 +1198,13 @@ struct ast_websocket *AST_OPTIONAL_API_NAME(ast_websocket_client_create)
 }
 
 int AST_OPTIONAL_API_NAME(ast_websocket_read_string)
-       (struct ast_websocket *ws, struct ast_str **buf)
+       (struct ast_websocket *ws, char **buf)
 {
        char *payload;
        uint64_t payload_len;
        enum ast_websocket_opcode opcode;
        int fragmented = 1;
 
-       if (!*buf && !(*buf = ast_str_create(512))) {
-               ast_log(LOG_ERROR, "Client Websocket string read - "
-                       "Unable to allocate string buffer");
-               return -1;
-       }
-
        while (fragmented) {
                if (ast_websocket_read(ws, &payload, &payload_len,
                                       &opcode, &fragmented)) {
@@ -1220,6 +1213,10 @@ int AST_OPTIONAL_API_NAME(ast_websocket_read_string)
                        return -1;
                }
 
+               if (opcode == AST_WEBSOCKET_OPCODE_CONTINUATION) {
+                       continue;
+               }
+
                if (opcode == AST_WEBSOCKET_OPCODE_CLOSE) {
                        return -1;
                }
@@ -1229,17 +1226,21 @@ int AST_OPTIONAL_API_NAME(ast_websocket_read_string)
                                "non string data received\n");
                        return -1;
                }
+       }
 
-               ast_str_append(buf, 0, "%s", payload);
+       if (!(*buf = ast_malloc(payload_len + 1))) {
+               return -1;
        }
-       return ast_str_size(*buf);
+
+       ast_copy_string(*buf, payload, payload_len + 1);
+       return payload_len + 1;
 }
 
 int AST_OPTIONAL_API_NAME(ast_websocket_write_string)
-       (struct ast_websocket *ws, const struct ast_str *buf)
+       (struct ast_websocket *ws, const char *buf)
 {
        return ast_websocket_write(ws, AST_WEBSOCKET_OPCODE_TEXT,
-                                  ast_str_buffer(buf), ast_str_strlen(buf));
+                                  (char *)buf, strlen(buf));
 }
 
 static int load_module(void)

diff --git a/tests/test_websocket_client.c b/tests/test_websocket_client.c
index 726ee1cfccf3f12d216568830c7a018b8c9fb0bf..dcf1a1773c81ce03e920b89ef025d57b4b13879e 100644 (file)
--- a/tests/test_websocket_client.c
+++ b/tests/test_websocket_client.c
@@ -48,8 +48,8 @@ AST_TEST_DEFINE(websocket_client_create_and_connect)
        RAII_VAR(struct ast_websocket *, client, NULL, ao2_cleanup);
 
        enum ast_websocket_result result;
-       struct ast_str *write_buf;
-       struct ast_str *read_buf;
+       const char write_buf[] = "this is only a test";
+       RAII_VAR(char *, read_buf, NULL, ast_free);
 
        switch (cmd) {
        case TEST_INIT:
@@ -62,16 +62,12 @@ AST_TEST_DEFINE(websocket_client_create_and_connect)
                break;
        }
 
-       write_buf = ast_str_alloca(20);
-       read_buf = ast_str_alloca(20);
-
        ast_test_validate(test, (client = ast_websocket_client_create(
                                         REMOTE_URL, "echo", NULL, &result)));
 
-       ast_str_set(&write_buf, 0, "this is only a test");
        ast_test_validate(test, !ast_websocket_write_string(client, write_buf));
        ast_test_validate(test, ast_websocket_read_string(client, &read_buf) > 0);
-       ast_test_validate(test, !strcmp(ast_str_buffer(write_buf), ast_str_buffer(read_buf)));
+       ast_test_validate(test, !strcmp(write_buf, read_buf));
 
        return AST_TEST_PASS;
 }