[Asterisk-code-review] res http websocket: respond to CLOSE opcode (asterisk[13])

[Joshua C. Colp]

Joshua C. Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/10856 )

Change subject: res_http_websocket: respond to CLOSE opcode
......................................................................

res_http_websocket: respond to CLOSE opcode

This ensures that Asterisk responds properly to frames received from a
client with opcode 8 (CLOSE) by echoing back the status code in its own
CLOSE frame.

Handling of the CLOSE opcode is moved up with the rest of the opcodes so
that unmasking gets applied. The payload is no longer returned to the
caller, but neither ARI nor the chan_sip nor pjsip made use of the
payload, which is a good thing since it was masked.

ASTERISK-28231 #close

Change-Id: Icb1b60205fc77ee970ddc91d1f545671781344cf
---
M res/res_http_websocket.c
1 file changed, 13 insertions(+), 24 deletions(-)

Approvals:
  Sean Bright: Looks good to me, but someone else must approve
  Joshua C. Colp: Looks good to me, but someone else must approve; Approved for Submit
  Kevin Harwell: Looks good to me, approved



diff --git a/res/res_http_websocket.c b/res/res_http_websocket.c
index d52e6b3..2ac5541 100644
--- a/res/res_http_websocket.c
+++ b/res/res_http_websocket.c
@@ -100,6 +100,7 @@
 	unsigned int closing:1;             /*!< Bit to indicate that the session is in the process of being closed */
 	unsigned int close_sent:1;          /*!< Bit to indicate that the session close opcode has been sent and no further data will be sent */
 	struct websocket_client *client;    /*!< Client object when connected as a client websocket */
+	uint16_t close_status_code;         /*!< Status code sent in a CLOSE frame upon shutdown */
 };
 
 /*! \brief Hashing function for protocols */
@@ -181,7 +182,7 @@
 	struct ast_websocket *session = obj;
 
 	if (session->f) {
-		ast_websocket_close(session, 0);
+		ast_websocket_close(session, session->close_status_code);
 		if (session->f) {
 			fclose(session->f);
 			ast_verb(2, "WebSocket connection %s '%s' closed\n", session->client ? "to" : "from",
@@ -566,7 +567,7 @@
 	*opcode = buf[0] & 0xf;
 	*payload_len = buf[1] & 0x7f;
 	if (*opcode == AST_WEBSOCKET_OPCODE_TEXT || *opcode == AST_WEBSOCKET_OPCODE_BINARY || *opcode == AST_WEBSOCKET_OPCODE_CONTINUATION ||
-	    *opcode == AST_WEBSOCKET_OPCODE_PING || *opcode == AST_WEBSOCKET_OPCODE_PONG) {
+	    *opcode == AST_WEBSOCKET_OPCODE_PING || *opcode == AST_WEBSOCKET_OPCODE_PONG  || *opcode == AST_WEBSOCKET_OPCODE_CLOSE) {
 		fin = (buf[0] >> 7) & 1;
 		mask_present = (buf[1] >> 7) & 1;
 
@@ -622,6 +623,16 @@
 			return 0;
 		}
 
+		/* Save the CLOSE status code which will be sent in our own CLOSE in the destructor */
+		if (*opcode == AST_WEBSOCKET_OPCODE_CLOSE) {
+			session->closing = 1;
+			if (*payload_len >= 2) {
+				session->close_status_code = ntohs(get_unaligned_uint16(*payload));
+			}
+			*payload_len = 0;
+			return 0;
+		}
+
 		if (*payload_len) {
 			if (!(new_payload = ast_realloc(session->payload, (session->payload_len + *payload_len)))) {
 				ast_log(LOG_WARNING, "Failed allocation: %p, %zu, %"PRIu64"\n",
@@ -661,28 +672,6 @@
 			*payload = session->payload;
 			session->payload_len = 0;
 		}
-	} else if (*opcode == AST_WEBSOCKET_OPCODE_CLOSE) {
-		session->closing = 1;
-
-		/* Make the payload available so the user can look at the reason code if they so desire */
-		if (!*payload_len) {
-			return 0;
-		}
-
-		if (!(new_payload = ast_realloc(session->payload, *payload_len))) {
-			ast_log(LOG_WARNING, "Failed allocation: %p, %"PRIu64"\n",
-					session->payload, *payload_len);
-			*payload_len = 0;
-			return -1;
-		}
-
-		session->payload = new_payload;
-		if (ws_safe_read(session, &buf[frame_size], *payload_len, opcode)) {
-			return -1;
-		}
-		memcpy(session->payload, &buf[frame_size], *payload_len);
-		*payload = session->payload;
-		frame_size += *payload_len;
 	} else {
 		ast_log(LOG_WARNING, "WebSocket unknown opcode %u\n", *opcode);
 		/* We received an opcode that we don't understand, the RFC states that 1003 is for a type of data that can't be accepted... opcodes

-- 
To view, visit https://gerrit.asterisk.org/10856
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: merged
Gerrit-Change-Id: Icb1b60205fc77ee970ddc91d1f545671781344cf
Gerrit-Change-Number: 10856
Gerrit-PatchSet: 3
Gerrit-Owner: Jeremy Lainé <jeremy.laine at m4x.org>
Gerrit-Reviewer: Friendly Automation (1000185)
Gerrit-Reviewer: Jeremy Lainé <jeremy.laine at m4x.org>
Gerrit-Reviewer: Joshua C. Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Sean Bright <sean.bright at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20190122/d6d36873/attachment-0001.html>