]> 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:10:11 +0000 (12:10 +0000)
committerMatthew Jordan <mjordan@digium.com>
Thu, 26 Jun 2014 12:10:11 +0000 (12:10 +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.

Note that this version of the patch, unlike r417310 in Asterisk 11, exposes
configuration options beyond just chan_sip's sip.conf. Configuration options
to configure the write timeout have also been added to pjsip.conf and ari.conf.

#ASTERISK-23917 #close
Reported by: Matt Jordan

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

Merged revisions 417310 from http://svn.asterisk.org/svn/asterisk/branches/11

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

16 files changed:
UPGRADE.txt
channels/chan_sip.c
channels/sip/include/sip.h
configs/ari.conf.sample
configs/pjsip.conf.sample
configs/sip.conf.sample
include/asterisk/http_websocket.h
include/asterisk/res_pjsip.h
res/ari/ari_websockets.c
res/ari/config.c
res/ari/internal.h
res/res_ari.c
res/res_http_websocket.c
res/res_pjsip.c
res/res_pjsip/config_transport.c
res/res_pjsip_transport_websocket.c

index 25336d2cb21fab3d68ec5ba95348c30d79bb85cc..1120364b79d933ac20abbf8bea3a4837065e08ee 100644 (file)
@@ -35,6 +35,12 @@ From 12.3.0 to 12.4.0:
    - "Exited on signal $EXITSIGNAL" => "Asterisk exited on signal $EXITSIGNAL."
    - "Asterisk Died" => "Asterisk on $MACHINE died (sig $EXITSIGNAL)"
 
+ - Added a compatibility option for ari, chan_sip, and chan_pjsip
+   '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 12.3.0 to 12.3.1:
 
  - MixMonitor AMI actions now require users to have authorization classes.
index 64f9f027add871bc3c8b5dc6737ae772b443db06..a1c0d788ce2f5ffa1d8ce1179c56502bfce04c93 100644 (file)
@@ -2668,6 +2668,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;
@@ -32217,6 +32221,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 b13e2bafdc4775e8786de637daf8fcade7e0c45b..2a6557d8d4dc2f0b1d5aa139b0fde60351154e84 100644 (file)
@@ -771,6 +771,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 */
 };
 
 struct ast_websocket;
index decdddc5827b4d0c5e6d9972d14366644555c1aa..59f9a44e5c3f3990cd948ab544e8c492d3c06823 100644 (file)
@@ -1,19 +1,25 @@
 [general]
-enabled = yes          ; When set to no, ARI support is disabled.
-;pretty = no           ; When set to yes, responses from ARI are
-;                      ; formatted to be human readable.
-;allowed_origins =     ; Comma separated list of allowed origins, for
-;                      ; Cross-Origin Resource Sharing. May be set to * to
-;                      ; allow all origins.
-;auth_realm =          ; Realm to use for authentication. Defaults to Asterisk
-;                      ; REST Interface.
+enabled = yes       ; When set to no, ARI support is disabled.
+;pretty = no        ; When set to yes, responses from ARI are
+;                   ; formatted to be human readable.
+;allowed_origins =  ; Comma separated list of allowed origins, for
+;                   ; Cross-Origin Resource Sharing. May be set to * to
+;                   ; allow all origins.
+;auth_realm =       ; Realm to use for authentication. Defaults to Asterisk
+;                   ; REST Interface.
+;
+; Default write timeout to set on websockets. 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.
+;websocket_write_timeout = 100
 
 ;[username]
-;type = user           ; Specifies user configuration
-;read_only = no                ; When set to yes, user is only authorized for
-;                      ; read-only requests.
+;type = user        ; Specifies user configuration
+;read_only = no     ; When set to yes, user is only authorized for
+;                   ; read-only requests.
 ;
-;password =            ; Crypted or plaintext password (see password_format).
+;password =         ; Crypted or plaintext password (see password_format).
 ;
 ; password_format may be set to plain (the default) or crypt. When set to crypt,
 ; crypt(3) is used to validate the password. A crypted password can be generated
@@ -22,3 +28,4 @@ enabled = yes         ; When set to no, ARI support is disabled.
 ; When set to plain, the password is in plaintext.
 ;
 ;password_format = plain
+
index 1bcfcb96b14ecd822414e5e26c33035e81699234..3aa05a96b3f84aac448fe265a03e5d32450f9fab 100644 (file)
                 ; "")
 ;tos=0  ; Enable TOS for the signalling sent over this transport (default: "0")
 ;cos=0  ; Enable COS for the signalling sent over this transport (default: "0")
-
+;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.
 
 ;==========================CONTACT SECTION OPTIONS=========================
 ;[contact]
index dde7093362e09599f9c08e60e5e3dcf5d3811ee2..0c47424fdfdd9ba750f58d64e381c1c2ebb008f7 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 10cb9a023230ebad8e5d8893af700f1b64282719..7920e0d027ec26e84732a39aa45e3ea353f25568 100644 (file)
 
 #include <errno.h>
 
+/*! \brief Default websocket write timeout, in ms */
+#define AST_DEFAULT_WEBSOCKET_WRITE_TIMEOUT 100
+
+/*! \brief Default websocket write timeout, in ms (as a string) */
+#define AST_DEFAULT_WEBSOCKET_WRITE_TIMEOUT_STR "100"
+
 /*!
  * \file http_websocket.h
  * \brief Support for WebSocket connections within the Asterisk HTTP server.
@@ -234,4 +240,15 @@ AST_OPTIONAL_API(int, ast_websocket_is_secure, (struct ast_websocket *session),
  */
 AST_OPTIONAL_API(int, ast_websocket_set_nonblock, (struct ast_websocket *session), { errno = ENOSYS; return -1;});
 
+/*!
+ * \brief Set the timeout on a non-blocking WebSocket session.
+ *
+ * \since 11.11.0
+ * \since 12.4.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 13859583cc3476892ff7cdf15f5902b376015a8d..b5a00837dc625ed85358a9621e5ddb5b830c4d3c 100644 (file)
@@ -126,6 +126,8 @@ struct ast_sip_transport {
        unsigned int tos;
        /*! QOS COS value */
        unsigned int cos;
+       /*! Write timeout */
+       int write_timeout;
 };
 
 /*!
index 90d6f0fdb3f49483c444a39ae5452187ae758d53..ff0a53c4fc7b11e4354a33aa381b2a7b73ecef27 100644 (file)
@@ -56,11 +56,16 @@ struct ast_ari_websocket_session *ast_ari_websocket_session_create(
        struct ast_websocket *ws_session, int (*validator)(struct ast_json *))
 {
        RAII_VAR(struct ast_ari_websocket_session *, session, NULL, ao2_cleanup);
+       RAII_VAR(struct ast_ari_conf *, config, ast_ari_config_get(), ao2_cleanup);
 
        if (ws_session == NULL) {
                return NULL;
        }
 
+       if (config == NULL || config->general == NULL) {
+               return NULL;
+       }
+
        if (validator == NULL) {
                validator = null_validator;
        }
@@ -72,6 +77,11 @@ struct ast_ari_websocket_session *ast_ari_websocket_session_create(
                return NULL;
        }
 
+       if (ast_websocket_set_timeout(ws_session, config->general->write_timeout)) {
+               ast_log(LOG_WARNING, "Failed to set write timeout %d on ARI web socket\n",
+                       config->general->write_timeout);
+       }
+
        session = ao2_alloc(sizeof(*session), websocket_session_dtor);
        if (!session) {
                return NULL;
index 59c4d7d9499c2a6a7e292a9b9bfad6b47ec6d37a..667d91ac07ae5ade18f55c095d4bc81ab3d97105 100644 (file)
@@ -27,6 +27,7 @@
 ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 
 #include "asterisk/config_options.h"
+#include "asterisk/http_websocket.h"
 #include "internal.h"
 
 /*! \brief Locking container for safe configuration access. */
@@ -320,6 +321,9 @@ int ast_ari_config_init(void)
        aco_option_register(&cfg_info, "allowed_origins", ACO_EXACT, general_options,
                "", OPT_STRINGFIELD_T, 0,
                STRFLDSET(struct ast_ari_conf_general, allowed_origins));
+       aco_option_register(&cfg_info, "websocket_write_timeout", ACO_EXACT, general_options,
+               AST_DEFAULT_WEBSOCKET_WRITE_TIMEOUT_STR, OPT_INT_T, PARSE_IN_RANGE,
+               FLDSET(struct ast_ari_conf_general, write_timeout), 1, INT_MAX);
 
        aco_option_register(&cfg_info, "type", ACO_EXACT, user, NULL,
                OPT_NOOP_T, 0, 0);
index 8453747f19f094d7585f924f27aa55a9934277bb..93ea0b773c12bfc6626777ed6644b7cf6b8e194d 100644 (file)
@@ -65,6 +65,8 @@ struct ast_ari_conf {
 struct ast_ari_conf_general {
        /*! Enabled by default, disabled if false. */
        int enabled;
+       /*! Write timeout for websocket connections */
+       int write_timeout;
        /*! Encoding format used during output (default compact). */
        enum ast_json_encoding_format format;
        /*! Authentication realm */
index ce7027e44974664f2416594e8da47e5198e95911..acdbbfe9aa0fb80acdea95ccdf27f5f62883c4e1 100644 (file)
                                                <ref type="link">https://wiki.asterisk.org/wiki/display/AST/Asterisk+Builtin+mini-HTTP+Server</ref>
                                        </see-also>
                                </configOption>
+                               <configOption name="websocket_write_timeout">
+                                       <synopsis>The timeout (in milliseconds) to set on WebSocket connections.</synopsis>
+                                       <description>
+                                               <para>If a websocket connection accepts input slowly, the timeout
+                                               for writes to it can be increased to keep it from being disconnected.
+                                               Value is in milliseconds; default is 100 ms.</para>
+                                       </description>
+                               </configOption>
                                <configOption name="pretty">
                                        <synopsis>Responses from ARI are formatted to be human readable</synopsis>
                                </configOption>
index 112a35fc257f98436bdf0cb9cd288d3ad8c3ec44..23fe92075b72cdbc715b15d743db7eb5b92770ff 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 */
@@ -255,7 +256,7 @@ 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;
 }
@@ -298,13 +299,12 @@ int AST_OPTIONAL_API_NAME(ast_websocket_write)(struct ast_websocket *session, en
                ao2_unlock(session);
                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;
        }
@@ -366,6 +366,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
@@ -510,8 +517,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;
@@ -685,6 +694,7 @@ int AST_OPTIONAL_API_NAME(ast_websocket_uri_cb)(struct ast_tcptls_session_instan
                        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);
index 08d86537d43fd11aae357a4a091e74d17d5bb2de..3e20b9e7b0473f93d220780c45e21c704365af6a 100644 (file)
                                        or the <replaceable>wss</replaceable> protocols.</para></note>
                                        </description>
                                </configOption>
+                               <configOption name="websocket_write_timeout">
+                                       <synopsis>The timeout (in milliseconds) to set on WebSocket connections.</synopsis>
+                                       <description>
+                                               <para>If a websocket connection accepts input slowly, the timeout
+                                               for writes to it can be increased to keep it from being disconnected.
+                                               Value is in milliseconds; default is 100 ms.</para>
+                                       </description>
+                               </configOption>
                        </configObject>
                        <configObject name="contact">
                                <synopsis>A way of creating an aliased name to a SIP URI</synopsis>
index 22581ca52a275d6b4a7193e82cb5db2193f4373a..785fcc5ac57c7646d78e607c03b96c8e7210c871 100644 (file)
@@ -28,6 +28,7 @@
 #include "asterisk/sorcery.h"
 #include "asterisk/acl.h"
 #include "include/res_pjsip_private.h"
+#include "asterisk/http_websocket.h"
 
 static int sip_transport_to_ami(const struct ast_sip_transport *transport,
                                struct ast_str **buf)
@@ -668,6 +669,7 @@ int ast_sip_initialize_sorcery_transport(void)
        ast_sorcery_object_field_register_custom(sorcery, "transport", "local_net", "", transport_localnet_handler, localnet_to_str, localnet_to_vl, 0, 0);
        ast_sorcery_object_field_register_custom(sorcery, "transport", "tos", "0", transport_tos_handler, tos_to_str, NULL, 0, 0);
        ast_sorcery_object_field_register(sorcery, "transport", "cos", "0", OPT_UINT_T, 0, FLDSET(struct ast_sip_transport, cos));
+       ast_sorcery_object_field_register(sorcery, "transport", "websocket_write_timeout", AST_DEFAULT_WEBSOCKET_WRITE_TIMEOUT_STR, OPT_INT_T, PARSE_IN_RANGE, FLDSET(struct ast_sip_transport, write_timeout), 1, INT_MAX);
 
        ast_sip_register_endpoint_formatter(&endpoint_transport_formatter);
 
index 22962dab0d7957c0a1a96096eeccfeb1a0c3ca40..bae120a19c00f250a3583cfd3a06fb7f25a5c8eb 100644 (file)
@@ -207,6 +207,37 @@ static int transport_read(void *data)
        return (read_data->payload_len == recvd) ? 0 : -1;
 }
 
+static int get_write_timeout(void)
+{
+       int write_timeout = -1;
+       struct ao2_container *transports;
+
+       transports = ast_sorcery_retrieve_by_fields(ast_sip_get_sorcery(), "transport", AST_RETRIEVE_FLAG_ALL, NULL);
+
+       if (transports) {
+               struct ao2_iterator it_transports = ao2_iterator_init(transports, 0);
+               struct ast_sip_transport *transport;
+
+               for (; (transport = ao2_iterator_next(&it_transports)); ao2_cleanup(transport)) {
+                       if (transport->type != AST_TRANSPORT_WS && transport->type != AST_TRANSPORT_WSS) {
+                               continue;
+                       }
+                       ast_debug(5, "Found %s transport with write timeout: %d\n",
+                               transport->type == AST_TRANSPORT_WS ? "WS" : "WSS",
+                               transport->write_timeout);
+                       write_timeout = MAX(write_timeout, transport->write_timeout);
+               }
+               ao2_cleanup(transports);
+       }
+
+       if (write_timeout < 0) {
+               write_timeout = AST_DEFAULT_WEBSOCKET_WRITE_TIMEOUT;
+       }
+
+       ast_debug(1, "Write timeout for WS/WSS transports: %d\n", write_timeout);
+       return write_timeout;
+}
+
 /*!
  \brief WebSocket connection handler.
  */
@@ -222,6 +253,11 @@ static void websocket_cb(struct ast_websocket *session, struct ast_variable *par
                return;
        }
 
+       if (ast_websocket_set_timeout(session, get_write_timeout())) {
+               ast_websocket_unref(session);
+               return;
+       }
+
        if (!(serializer = ast_sip_create_serializer())) {
                ast_websocket_unref(session);
                return;