]> git.ipfire.org Git - thirdparty/asterisk.git/commitdiff
res_http_websocket: Close websocket correctly and use careful fwrite
authorMatthew Jordan <mjordan@digium.com>
Thu, 26 Jun 2014 12:06:22 +0000 (12:06 +0000)
committerMatthew Jordan <mjordan@digium.com>
Thu, 26 Jun 2014 12:06:22 +0000 (12:06 +0000)
When a client takes a long time to process information received from Asterisk,
a write operation using fwrite may fail to write all information. This causes
the underlying file stream to be in an unknown state, such that the socket
must be disconnected. Unfortunately, there are two problems with this in
Asterisk's existing websocket code:
1. Periodically, during the read loop, Asterisk must write to the connected
   websocket to respond to pings. As such, Asterisk maintains a reference to
   the session during the loop. When ast_http_websocket_write fails, it may
   cause the session to decrement its ref count, but this in and of itself
   does not break the read loop. The read loop's write, on the other hand,
   does not break the loop if it fails. This causes the socket to get in a
   'stuck' state, preventing the client from reconnecting to the server.
2. More importantly, however, is that the fwrite in ast_http_websocket_write
   fails with a large volume of data when the client takes awhile to process
   the information. When it does fail, it fails writing only a portion of
   the bytes. With some debugging, it was shown that this was failing in a
   similar fashion to ASTERISK-12767. Switching this over to ast_careful_fwrite
   with a long enough timeout solved the problem.

ASTERISK-23917 #close
Reported by: Matt Jordan

Review: https://reviewboard.asterisk.org/r/3624/

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

UPGRADE.txt
channels/chan_sip.c
channels/sip/include/sip.h
configs/sip.conf.sample
include/asterisk/http_websocket.h
res/res_http_websocket.c

index 74fc9310c7e7ff93154e54d33384420dec782d01..71c21ae3a0c375a05836543228c917ed361cb5c2 100644 (file)
 ===
 ===========================================================
 
+from 11.10.0 to 11.11.0
+ - Added a compatibility option for chan_sip, 'websocket_write_timeout'.
+   When a websocket connection exists where Asterisk writes a substantial
+   amount of data to the connected client, and the connected client is slow
+   to process the received data, the socket may be disconnected. In such
+   cases, it may be necessary to adjust this value. Default is 100 ms.
+
 from 11.10.0 to 11.10.1
  - MixMonitor AMI actions now require users to have authorization classes.
    * MixMonitor - system
index 594bc56a0577f57f8d66faacbc0971c40ae57d27..55d724a829986bb626da0fd6a716cb3a787262f0 100644 (file)
@@ -2578,6 +2578,10 @@ static void sip_websocket_callback(struct ast_websocket *session, struct ast_var
                goto end;
        }
 
+       if (ast_websocket_set_timeout(session, sip_cfg.websocket_write_timeout)) {
+               goto end;
+       }
+
        while ((res = ast_wait_for_input(ast_websocket_fd(session), -1)) > 0) {
                char *payload;
                uint64_t payload_len;
@@ -32241,6 +32245,12 @@ static int reload_config(enum channelreloadreason reason)
                        ast_copy_string(default_parkinglot, v->value, sizeof(default_parkinglot));
                } else if (!strcasecmp(v->name, "refer_addheaders")) {
                        global_refer_addheaders = ast_true(v->value);
+               } else if (!strcasecmp(v->name, "websocket_write_timeout")) {
+                       if (sscanf(v->value, "%30d", &sip_cfg.websocket_write_timeout) != 1
+                               || sip_cfg.websocket_write_timeout < 0) {
+                               ast_log(LOG_WARNING, "'%s' is not a valid websocket_write_timeout value at line %d. Using default '%d'.\n", v->value, v->lineno, AST_DEFAULT_WEBSOCKET_WRITE_TIMEOUT);
+                               sip_cfg.websocket_write_timeout = AST_DEFAULT_WEBSOCKET_WRITE_TIMEOUT;
+                       }
                }
        }
 
index 99a0dae33dc390a7c0855b4066ea19a77615fa85..0b4fc3192ae9b5068d0db9c66073552d28c1532d 100644 (file)
@@ -778,6 +778,7 @@ struct sip_settings {
        struct ast_format_cap *caps; /*!< Supported codecs */
        int tcp_enabled;
        int default_max_forwards;    /*!< Default max forwards (SIP Anti-loop) */
+       int websocket_write_timeout; /*!< Socket write timeout for websocket transports, in ms */
 };
 
 /*! \brief The SIP socket definition */
index 962806e123b9eadd64cf6e6b4b69ff485a0d372a..e240cdf90301fa0ab4439edf9286eb965f1d4453 100644 (file)
@@ -229,6 +229,12 @@ tcpbindaddr=0.0.0.0             ; IP address for TCP server to bind to (0.0.0.0
                                ; unauthenticated sessions that will be allowed
                                 ; to connect at any given time. (default: 100)
 
+;websocket_write_timeout = 100  ; Default write timeout to set on websocket transports.
+                                ; This value may need to be adjusted for connections where
+                                ; Asterisk must write a substantial amount of data and the
+                                ; receiving clients are slow to process the received information.
+                                ; Value is in milliseconds; default is 100 ms.
+
 transport=udp                   ; Set the default transports.  The order determines the primary default transport.
                                 ; If tcpenable=no and the transport set is tcp, we will fallback to UDP.
 
index d59bc25cdcd83efbc138f4c0da271ce18c8a09e7..5ddd1fbb592a0fd233b5206e7d96a7c5f3a892f1 100644 (file)
@@ -21,6 +21,9 @@
 
 #include "asterisk/optional_api.h"
 
+/*! \brief Default websocket write timeout, in ms */
+#define AST_DEFAULT_WEBSOCKET_WRITE_TIMEOUT 100
+
 /*!
  * \file http_websocket.h
  * \brief Support for WebSocket connections within the Asterisk HTTP server.
@@ -184,4 +187,14 @@ AST_OPTIONAL_API(int, ast_websocket_is_secure, (struct ast_websocket *session),
  */
 AST_OPTIONAL_API(int, ast_websocket_set_nonblock, (struct ast_websocket *session), {return -1;});
 
+/*!
+ * \brief Set the timeout on a non-blocking WebSocket session.
+ *
+ * \since 11.11.0
+ *
+ * \retval 0 on success
+ * \retval -1 on failure
+ */
+AST_OPTIONAL_API(int, ast_websocket_set_timeout, (struct ast_websocket *session, int timeout), {return -1;});
+
 #endif
index 41939ccf210b6e4ceb8cec5ce1306c81ab155050..c72c8da3270608fb081597ba54f6d6a41b5e1b37 100644 (file)
@@ -77,6 +77,7 @@ struct ast_websocket {
        size_t payload_len;               /*!< Length of the payload */
        char *payload;                    /*!< Pointer to the payload */
        size_t reconstruct;               /*!< Number of bytes before a reconstructed payload will be returned and a new one started */
+       int timeout;                      /*!< The timeout for operations on the socket */
        unsigned int secure:1;            /*!< Bit to indicate that the transport is secure */
        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 */
@@ -207,8 +208,9 @@ int AST_OPTIONAL_API_NAME(ast_websocket_close)(struct ast_websocket *session, ui
        session->close_sent = 1;
 
        ao2_lock(session);
-       res = (fwrite(frame, 1, 4, session->f) == 4) ? 0 : -1;
+       res = ast_careful_fwrite(session->f, session->fd, frame, 4, session->timeout);
        ao2_unlock(session);
+
        return res;
 }
 
@@ -251,12 +253,12 @@ int AST_OPTIONAL_API_NAME(ast_websocket_write)(struct ast_websocket *session, en
                return -1;
        }
 
-       if (fwrite(frame, 1, header_size, session->f) != header_size) {
+       if (ast_careful_fwrite(session->f, session->fd, frame, header_size, session->timeout)) {
                ao2_unlock(session);
                return -1;
        }
 
-       if (fwrite(payload, 1, actual_length, session->f) != actual_length) {
+       if (ast_careful_fwrite(session->f, session->fd, payload, actual_length, session->timeout)) {
                ao2_unlock(session);
                return -1;
        }
@@ -318,6 +320,13 @@ int AST_OPTIONAL_API_NAME(ast_websocket_set_nonblock)(struct ast_websocket *sess
        return 0;
 }
 
+int AST_OPTIONAL_API_NAME(ast_websocket_set_timeout)(struct ast_websocket *session, int timeout)
+{
+       session->timeout = timeout;
+
+       return 0;
+}
+
 /* MAINTENANCE WARNING on ast_websocket_read()!
  *
  * We have to keep in mind during this function that the fact that session->fd seems ready
@@ -462,8 +471,10 @@ int AST_OPTIONAL_API_NAME(ast_websocket_read)(struct ast_websocket *session, cha
                }
 
                /* Per the RFC for PING we need to send back an opcode with the application data as received */
-               if (*opcode == AST_WEBSOCKET_OPCODE_PING) {
-                       ast_websocket_write(session, AST_WEBSOCKET_OPCODE_PONG, *payload, *payload_len);
+               if ((*opcode == AST_WEBSOCKET_OPCODE_PING) && (ast_websocket_write(session, AST_WEBSOCKET_OPCODE_PONG, *payload, *payload_len))) {
+                       *payload_len = 0;
+                       ast_websocket_close(session, 1009);
+                       return 0;
                }
 
                session->payload = new_payload;
@@ -613,6 +624,7 @@ static int websocket_callback(struct ast_tcptls_session_instance *ser, const str
                        ao2_ref(protocol_handler, -1);
                        return 0;
                }
+               session->timeout = AST_DEFAULT_WEBSOCKET_WRITE_TIMEOUT;
 
                combined = ast_alloca(combined_length);
                snprintf(combined, combined_length, "%s%s", key, WEBSOCKET_GUID);