[asterisk-commits] kharwell: trunk r416394 - in /trunk: include/asterisk/ res/ tests/

[SVN commits to the Asterisk project]

Author: kharwell
Date: Mon Jun 16 11:22:33 2014
New Revision: 416394

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=416394
Log:
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.

Modified:
    trunk/include/asterisk/http_websocket.h
    trunk/res/res_http_websocket.c
    trunk/tests/test_websocket_client.c

Modified: trunk/include/asterisk/http_websocket.h
URL: http://svnview.digium.com/svn/asterisk/trunk/include/asterisk/http_websocket.h?view=diff&rev=416394&r1=416393&r2=416394
==============================================================================
--- trunk/include/asterisk/http_websocket.h (original)
+++ trunk/include/asterisk/http_websocket.h Mon Jun 16 11:22:33 2014
@@ -156,6 +156,8 @@
 /*!
  * \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 @@
  * \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 @@
  * \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

Modified: trunk/res/res_http_websocket.c
URL: http://svnview.digium.com/svn/asterisk/trunk/res/res_http_websocket.c?view=diff&rev=416394&r1=416393&r2=416394
==============================================================================
--- trunk/res/res_http_websocket.c (original)
+++ trunk/res/res_http_websocket.c Mon Jun 16 11:22:33 2014
@@ -497,7 +497,6 @@
 		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,18 +1198,12 @@
 }
 
 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,
@@ -1220,6 +1213,10 @@
 			return -1;
 		}
 
+		if (opcode == AST_WEBSOCKET_OPCODE_CONTINUATION) {
+			continue;
+		}
+
 		if (opcode == AST_WEBSOCKET_OPCODE_CLOSE) {
 			return -1;
 		}
@@ -1229,17 +1226,21 @@
 				"non string data received\n");
 			return -1;
 		}
-
-		ast_str_append(buf, 0, "%s", payload);
-	}
-	return ast_str_size(*buf);
+	}
+
+	if (!(*buf = ast_malloc(payload_len + 1))) {
+		return -1;
+	}
+
+	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)

Modified: trunk/tests/test_websocket_client.c
URL: http://svnview.digium.com/svn/asterisk/trunk/tests/test_websocket_client.c?view=diff&rev=416394&r1=416393&r2=416394
==============================================================================
--- trunk/tests/test_websocket_client.c (original)
+++ trunk/tests/test_websocket_client.c Mon Jun 16 11:22:33 2014
@@ -48,8 +48,8 @@
 	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 @@
 		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;
 }