[libsoup/mcatanzaro/#134] connection: Fix check for remote disconnection when in idle state

[Michael Catanzaro <mcatanzaro src gnome org>]

commit b609d44ec39d39a8d4012c65db68a622c3605adc
Author: Michael Catanzaro <mcatanzaro igalia com>
Date:   Fri Feb 22 14:10:12 2019 -0600

    connection: Fix check for remote disconnection when in idle state
    
    A change in glib-networking has surfaced a tricky mistake in
    soup_connection_get_state(). The goal of this code is to detect early if
    the server has closed our connection, but the check didn't account for
    the possibility that TLS-level data could be readable even if no
    HTTP-level data is readable. We need to actually try reading from the
    stream, rather than just checking if the socket is readable, since that
    way any TLS-level data will be handled by GTlsConnection. We should
    receive G_IO_ERROR_WOULD_BLOCK only if there is no HTTP-level data and
    the connection is still open; otherwise, treat it as disconnected.
    
    Many thanks to Dan Winship for explaining this confusing code after I
    found it was problematic, and for proposing this clever solution.
    
    Fixes #134

 libsoup/soup-connection.c | 47 +++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 41 insertions(+), 6 deletions(-)
---
diff --git a/libsoup/soup-connection.c b/libsoup/soup-connection.c
index 5fb4d78c..3e05a5ab 100644
--- a/libsoup/soup-connection.c
+++ b/libsoup/soup-connection.c
@@ -631,6 +631,46 @@ soup_connection_is_via_proxy (SoupConnection *conn)
        return priv->proxy_uri != NULL;
 }
 
+static gboolean
+is_idle_connection_disconnected (SoupConnection *conn)
+{
+       SoupConnectionPrivate *priv = soup_connection_get_instance_private (conn);
+       GIOStream *iostream;
+       GInputStream *istream;
+       char buffer[1];
+       GError *error = NULL;
+       gboolean ret = FALSE;
+
+       if (!soup_socket_is_connected (priv->socket))
+               return TRUE;
+
+       if (priv->unused_timeout && priv->unused_timeout < time (NULL))
+               return TRUE;
+
+       iostream = soup_socket_get_iostream (priv->socket);
+       istream = g_io_stream_get_input_stream (iostream);
+
+       /* This is tricky. The goal is to check if the socket is readable. If
+        * so, that means either the server has disconnected or it's broken (it
+        * should not send any data while the connection is in idle state). But
+        * we can't just check the readability of the SoupSocket because there
+        * could be non-application layer TLS data that is readable, but which
+        * we don't want to consider. So instead, just read and see if the read
+        * succeeds. This is OK to do here because if the read does succeed, we
+        * just disconnect and ignore the data anyway.
+        */
+       g_pollable_input_stream_read_nonblocking (G_POLLABLE_INPUT_STREAM (istream),
+                                                 &buffer, sizeof (buffer),
+                                                 NULL, &error);
+       if (error != NULL) {
+               if (!g_error_matches (error, G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK))
+                       ret = TRUE;
+               g_error_free (error);
+       }
+
+       return ret;
+}
+
 SoupConnectionState
 soup_connection_get_state (SoupConnection *conn)
 {
@@ -641,12 +681,7 @@ soup_connection_get_state (SoupConnection *conn)
        priv = soup_connection_get_instance_private (conn);
 
        if (priv->state == SOUP_CONNECTION_IDLE &&
-           (!soup_socket_is_connected (priv->socket) ||
-            soup_socket_is_readable (priv->socket)))
-               soup_connection_set_state (conn, SOUP_CONNECTION_REMOTE_DISCONNECTED);
-
-       if (priv->state == SOUP_CONNECTION_IDLE &&
-           priv->unused_timeout && priv->unused_timeout < time (NULL))
+           is_idle_connection_disconnected (conn))
                soup_connection_set_state (conn, SOUP_CONNECTION_REMOTE_DISCONNECTED);
 
        return priv->state;