res_http_websocket.c: prevent avoidable disconnections caused by write errors

Updated ast_websocket_write to encode the entire frame in to one
write operation, to ensure that we don't end up with a situation
where the websocket header has been sent, while the body can not
be written.

Previous to August's patch in commit b9bd3c14, certain network
conditions could cause the header to be written, and then the
sub-sequent body to fail - which would cause the next successful
write to contain a new header, and a new body (resulting in
the peer receiving two headers - the second of which would be
read as part of the body for the first header).

This was patched to have both write operations individually fail
by closing the websocket.

In a case available to the submitter of this patch, the same
body which would consistently fail to write, would succeed
if written at the same time as the header.

This update merges the two operations in to one, adds debug messages
indicating the reason for a websocket connection being closed during
a write operation, and clarifies some variable names for code legibility.

Change-Id: I4db7a586af1c7a57184c31d3d55bf146f1a40598

diff --git a/include/asterisk/http_websocket.h b/include/asterisk/http_websocket.h
index 5adc089255cf66dc5c7db829259bc9a90ba576bb..d8e7d063ba311a6b146da7e094626eebc030c9ee 100644 (file)
--- a/include/asterisk/http_websocket.h
+++ b/include/asterisk/http_websocket.h
@@ -265,12 +265,12 @@ AST_OPTIONAL_API(int, ast_websocket_read_string,
  * \param session Pointer to the WebSocket session
  * \param opcode WebSocket operation code to place in the frame
  * \param payload Optional pointer to a payload to add to the frame
- * \param actual_length Length of the payload (0 if no payload)
+ * \param payload_size Length of the payload (0 if no payload)
  *
  * \retval 0 if successfully written
  * \retval -1 if error occurred
  */
-AST_OPTIONAL_API(int, ast_websocket_write, (struct ast_websocket *session, enum ast_websocket_opcode opcode, char *payload, uint64_t actual_length), { errno = ENOSYS; return -1;});
+AST_OPTIONAL_API(int, ast_websocket_write, (struct ast_websocket *session, enum ast_websocket_opcode opcode, char *payload, uint64_t payload_size), { errno = ENOSYS; return -1;});
 
 /*!
  * \brief Construct and transmit a WebSocket frame containing string data.

diff --git a/res/res_http_websocket.c b/res/res_http_websocket.c
index b6baa3cf92947b92c67a7623cd409c95844457ff..e16c23c0cffc9df8124a83181220435cafcbc5d8 100644 (file)
--- a/res/res_http_websocket.c
+++ b/res/res_http_websocket.c
@@ -330,18 +330,19 @@ static const char *websocket_opcode2str(enum ast_websocket_opcode opcode)
 }
 
 /*! \brief Write function for websocket traffic */
-int AST_OPTIONAL_API_NAME(ast_websocket_write)(struct ast_websocket *session, enum ast_websocket_opcode opcode, char *payload, uint64_t actual_length)
+int AST_OPTIONAL_API_NAME(ast_websocket_write)(struct ast_websocket *session, enum ast_websocket_opcode opcode, char *payload, uint64_t payload_size)
 {
        size_t header_size = 2; /* The minimum size of a websocket frame is 2 bytes */
        char *frame;
        uint64_t length;
+       uint64_t frame_size;
 
        ast_debug(3, "Writing websocket %s frame, length %" PRIu64 "\n",
-                       websocket_opcode2str(opcode), actual_length);
+                       websocket_opcode2str(opcode), payload_size);
 
-       if (actual_length < 126) {
-               length = actual_length;
-       } else if (actual_length < (1 << 16)) {
+       if (payload_size < 126) {
+               length = payload_size;
+       } else if (payload_size < (1 << 16)) {
                length = 126;
                /* We need an additional 2 bytes to store the extended length */
                header_size += 2;
@@ -351,37 +352,37 @@ int AST_OPTIONAL_API_NAME(ast_websocket_write)(struct ast_websocket *session, en
                header_size += 8;
        }
 
-       frame = ast_alloca(header_size);
-       memset(frame, 0, header_size);
+       frame_size = header_size + payload_size;
+
+       frame = ast_alloca(frame_size + 1);
+       memset(frame, 0, frame_size + 1);
 
        frame[0] = opcode | 0x80;
        frame[1] = length;
 
        /* Use the additional available bytes to store the length */
        if (length == 126) {
-               put_unaligned_uint16(&frame[2], htons(actual_length));
+               put_unaligned_uint16(&frame[2], htons(payload_size));
        } else if (length == 127) {
-               put_unaligned_uint64(&frame[2], htonll(actual_length));
+               put_unaligned_uint64(&frame[2], htonll(payload_size));
        }
 
+       memcpy(&frame[header_size], payload, payload_size);
+
        ao2_lock(session);
        if (session->closing) {
                ao2_unlock(session);
                return -1;
        }
-       if (ast_careful_fwrite(session->f, session->fd, frame, header_size, session->timeout)) {
-               ao2_unlock(session);
-               /* 1011 - server terminating connection due to not being able to fulfill the request */
-               ast_websocket_close(session, 1011);
-               return -1;
-       }
 
-       if (ast_careful_fwrite(session->f, session->fd, payload, actual_length, session->timeout)) {
+       if (ast_careful_fwrite(session->f, session->fd, frame, frame_size, session->timeout)) {
                ao2_unlock(session);
                /* 1011 - server terminating connection due to not being able to fulfill the request */
+               ast_debug(1, "Closing WS with 1011 because we can't fulfill a write request\n");
                ast_websocket_close(session, 1011);
                return -1;
        }
+
        fflush(session->f);
        ao2_unlock(session);