[libsoup/carlosgc/websockets-empty-body-close] WebSockets: allow to send close frames with no body

[Carlos Garcia Campos <carlosgc src gnome org>]

commit 73496ae9fd9fce132bd1a964595d83f7d9fa8d5f
Author: Carlos Garcia Campos <cgarcia igalia com>
Date:   Fri Jul 19 16:23:03 2019 +0200

    WebSockets: allow to send close frames with no body
    
    Using the code SOUP_WEBSOCKET_CLOSE_NO_STATUS. The spec says that code
    should not be included in a frame, but that doesn't mean we can't use it
    on the API level to mean no status. We were using 0 internally which is
    not a valid code either. When an empty close frame is received we still
    reply with a SOUP_WEBSOCKET_CLOSE_NORMAL code frame, but we return
    SOUP_WEBSOCKET_CLOSE_NO_STATUS from
    soup_websocket_connection_get_close_code() because that's actually what
    we received.

 libsoup/soup-websocket-connection.c |   9 +-
 tests/websocket-test.c              | 164 +++++++++++++++++++++++++++++-------
 2 files changed, 142 insertions(+), 31 deletions(-)
---
diff --git a/libsoup/soup-websocket-connection.c b/libsoup/soup-websocket-connection.c
index 94ef52a4..8e35e9f0 100644
--- a/libsoup/soup-websocket-connection.c
+++ b/libsoup/soup-websocket-connection.c
@@ -660,6 +660,10 @@ close_connection (SoupWebsocketConnection *self,
                                 code);
                }
                break;
+       case SOUP_WEBSOCKET_CLOSE_NO_STATUS:
+               /* This is special case to send a close message with no body */
+               code = 0;
+               break;
        default:
                if (code < 3000) {
                        g_debug ("Wrong closing code %d received", code);
@@ -695,6 +699,7 @@ receive_close (SoupWebsocketConnection *self,
        switch (len) {
        case 0:
                /* Send a clean close when having an empty payload */
+               pv->peer_close_code = SOUP_WEBSOCKET_CLOSE_NO_STATUS;
                close_connection (self, 1000, NULL);
                return;
        case 1:
@@ -1910,6 +1915,7 @@ soup_websocket_connection_send_message (SoupWebsocketConnection *self,
  * main loop runs.
  *
  * The @code and @data are sent to the peer along with the close request.
+ * If @code is %SOUP_WEBSOCKET_CLOSE_NO_STATUS a close message with no body is sent.
  * Note that the @data must be UTF-8 valid.
  *
  * Since: 2.50
@@ -1925,8 +1931,7 @@ soup_websocket_connection_close (SoupWebsocketConnection *self,
        pv = self->pv;
        g_return_if_fail (!pv->close_sent);
 
-       g_return_if_fail (code != SOUP_WEBSOCKET_CLOSE_NO_STATUS &&
-                         code != SOUP_WEBSOCKET_CLOSE_ABNORMAL &&
+       g_return_if_fail (code != SOUP_WEBSOCKET_CLOSE_ABNORMAL &&
                          code != SOUP_WEBSOCKET_CLOSE_TLS_HANDSHAKE);
        if (pv->connection_type == SOUP_WEBSOCKET_CONNECTION_SERVER)
                g_return_if_fail (code != SOUP_WEBSOCKET_CLOSE_NO_EXTENSION);
diff --git a/tests/websocket-test.c b/tests/websocket-test.c
index 8de3a1dd..b9a7d658 100644
--- a/tests/websocket-test.c
+++ b/tests/websocket-test.c
@@ -733,17 +733,36 @@ test_protocol_client_any_soup (Test *test,
        g_assert_cmpstr (soup_message_headers_get_one (test->msg->response_headers, 
"Sec-WebSocket-Protocol"), ==, NULL);
 }
 
+static const struct {
+       gushort code;
+       const char *reason;
+       gushort expected_sender_code;
+       const char *expected_sender_reason;
+       gushort expected_receiver_code;
+       const char *expected_receiver_reason;
+} close_clean_tests[] = {
+       { SOUP_WEBSOCKET_CLOSE_NORMAL, "NORMAL", SOUP_WEBSOCKET_CLOSE_NORMAL, "NORMAL", 
SOUP_WEBSOCKET_CLOSE_NORMAL, "NORMAL" },
+       { SOUP_WEBSOCKET_CLOSE_GOING_AWAY, "GOING_AWAY", SOUP_WEBSOCKET_CLOSE_GOING_AWAY, "GOING_AWAY", 
SOUP_WEBSOCKET_CLOSE_GOING_AWAY, "GOING_AWAY" },
+       { SOUP_WEBSOCKET_CLOSE_NORMAL, NULL, SOUP_WEBSOCKET_CLOSE_NORMAL, NULL, SOUP_WEBSOCKET_CLOSE_NORMAL, 
NULL },
+       { SOUP_WEBSOCKET_CLOSE_NO_STATUS, NULL, SOUP_WEBSOCKET_CLOSE_NORMAL, NULL, 
SOUP_WEBSOCKET_CLOSE_NO_STATUS, NULL },
+};
+
 static void
-test_close_clean_client (Test *test,
-                         gconstpointer data)
+do_close_clean_client (Test *test,
+                      gushort code,
+                      const char *reason,
+                      gushort expected_sender_code,
+                      const char *expected_sender_reason,
+                      gushort expected_receiver_code,
+                      const char *expected_receiver_reason)
 {
        gboolean close_event_client = FALSE;
-       gboolean close_event_server = FALSE;
+        gboolean close_event_server = FALSE;
 
        g_signal_connect (test->client, "closed", G_CALLBACK (on_close_set_flag), &close_event_client);
        g_signal_connect (test->server, "closed", G_CALLBACK (on_close_set_flag), &close_event_server);
 
-       soup_websocket_connection_close (test->client, SOUP_WEBSOCKET_CLOSE_GOING_AWAY, "give me a reason");
+       soup_websocket_connection_close (test->client, code, reason);
        g_assert_cmpint (soup_websocket_connection_get_state (test->client), ==, 
SOUP_WEBSOCKET_STATE_CLOSING);
 
        WAIT_UNTIL (soup_websocket_connection_get_state (test->server) == SOUP_WEBSOCKET_STATE_CLOSED);
@@ -752,14 +771,62 @@ test_close_clean_client (Test *test,
        g_assert (close_event_client);
        g_assert (close_event_server);
 
-       g_assert_cmpint (soup_websocket_connection_get_close_code (test->client), ==, 
SOUP_WEBSOCKET_CLOSE_GOING_AWAY);
-       g_assert_cmpint (soup_websocket_connection_get_close_code (test->server), ==, 
SOUP_WEBSOCKET_CLOSE_GOING_AWAY);
-       g_assert_cmpstr (soup_websocket_connection_get_close_data (test->server), ==, "give me a reason");
+       g_assert_cmpint (soup_websocket_connection_get_close_code (test->client), ==, expected_sender_code);
+       g_assert_cmpstr (soup_websocket_connection_get_close_data (test->client), ==, expected_sender_reason);
+       g_assert_cmpint (soup_websocket_connection_get_close_code (test->server), ==, expected_receiver_code);
+       g_assert_cmpstr (soup_websocket_connection_get_close_data (test->server), ==, 
expected_receiver_reason);
+}
+
+static void
+test_close_clean_client_soup (Test *test,
+                             gconstpointer data)
+{
+       guint i;
+
+       for (i = 0; i < G_N_ELEMENTS (close_clean_tests); i++) {
+               setup_soup_connection (test, data);
+
+               do_close_clean_client (test,
+                                      close_clean_tests[i].code,
+                                      close_clean_tests[i].reason,
+                                      close_clean_tests[i].expected_sender_code,
+                                      close_clean_tests[i].expected_sender_reason,
+                                      close_clean_tests[i].expected_receiver_code,
+                                      close_clean_tests[i].expected_receiver_reason);
+
+               teardown_soup_connection (test, data);
+       }
+}
+
+static void
+test_close_clean_client_direct (Test *test,
+                               gconstpointer data)
+{
+       guint i;
+
+       for (i = 0; i < G_N_ELEMENTS (close_clean_tests); i++) {
+               setup_direct_connection (test, data);
+
+               do_close_clean_client (test,
+                                      close_clean_tests[i].code,
+                                      close_clean_tests[i].reason,
+                                      close_clean_tests[i].expected_sender_code,
+                                      close_clean_tests[i].expected_sender_reason,
+                                      close_clean_tests[i].expected_receiver_code,
+                                      close_clean_tests[i].expected_receiver_reason);
+
+               teardown_direct_connection (test, data);
+       }
 }
 
 static void
-test_close_clean_server (Test *test,
-                         gconstpointer data)
+do_close_clean_server (Test *test,
+                      gushort code,
+                      const char *reason,
+                      gushort expected_sender_code,
+                      const char *expected_sender_reason,
+                      gushort expected_receiver_code,
+                      const char *expected_receiver_reason)
 {
        gboolean close_event_client = FALSE;
        gboolean close_event_server = FALSE;
@@ -767,7 +834,7 @@ test_close_clean_server (Test *test,
        g_signal_connect (test->client, "closed", G_CALLBACK (on_close_set_flag), &close_event_client);
        g_signal_connect (test->server, "closed", G_CALLBACK (on_close_set_flag), &close_event_server);
 
-       soup_websocket_connection_close (test->server, SOUP_WEBSOCKET_CLOSE_GOING_AWAY, "another reason");
+       soup_websocket_connection_close (test->server, code, reason);
        g_assert_cmpint (soup_websocket_connection_get_state (test->server), ==, 
SOUP_WEBSOCKET_STATE_CLOSING);
 
        WAIT_UNTIL (soup_websocket_connection_get_state (test->server) == SOUP_WEBSOCKET_STATE_CLOSED);
@@ -776,9 +843,52 @@ test_close_clean_server (Test *test,
        g_assert (close_event_client);
        g_assert (close_event_server);
 
-       g_assert_cmpint (soup_websocket_connection_get_close_code (test->server), ==, 
SOUP_WEBSOCKET_CLOSE_GOING_AWAY);
-       g_assert_cmpint (soup_websocket_connection_get_close_code (test->client), ==, 
SOUP_WEBSOCKET_CLOSE_GOING_AWAY);
-       g_assert_cmpstr (soup_websocket_connection_get_close_data (test->client), ==, "another reason");
+       g_assert_cmpint (soup_websocket_connection_get_close_code (test->server), ==, expected_sender_code);
+       g_assert_cmpstr (soup_websocket_connection_get_close_data (test->server), ==, expected_sender_reason);
+       g_assert_cmpint (soup_websocket_connection_get_close_code (test->client), ==, expected_receiver_code);
+       g_assert_cmpstr (soup_websocket_connection_get_close_data (test->client), ==, 
expected_receiver_reason);
+}
+
+static void
+test_close_clean_server_soup (Test *test,
+                             gconstpointer data)
+{
+       guint i;
+
+       for (i = 0; i < G_N_ELEMENTS (close_clean_tests); i++) {
+               setup_direct_connection (test, data);
+
+               do_close_clean_server (test,
+                                      close_clean_tests[i].code,
+                                      close_clean_tests[i].reason,
+                                      close_clean_tests[i].expected_sender_code,
+                                      close_clean_tests[i].expected_sender_reason,
+                                      close_clean_tests[i].expected_receiver_code,
+                                      close_clean_tests[i].expected_receiver_reason);
+
+               teardown_direct_connection (test, data);
+       }
+}
+
+static void
+test_close_clean_server_direct (Test *test,
+                               gconstpointer data)
+{
+       guint i;
+
+       for (i = 0; i < G_N_ELEMENTS (close_clean_tests); i++) {
+               setup_direct_connection (test, data);
+
+               do_close_clean_server (test,
+                                      close_clean_tests[i].code,
+                                      close_clean_tests[i].reason,
+                                      close_clean_tests[i].expected_sender_code,
+                                      close_clean_tests[i].expected_sender_reason,
+                                      close_clean_tests[i].expected_receiver_code,
+                                      close_clean_tests[i].expected_receiver_reason);
+
+               teardown_direct_connection (test, data);
+       }
 }
 
 static gboolean
@@ -1190,23 +1300,19 @@ main (int argc,
                    test_send_bad_data,
                    teardown_soup_connection);
 
-       g_test_add ("/websocket/direct/close-clean-client", Test, NULL,
-                   setup_direct_connection,
-                   test_close_clean_client,
-                   teardown_direct_connection);
-       g_test_add ("/websocket/soup/close-clean-client", Test, NULL,
-                   setup_soup_connection,
-                   test_close_clean_client,
-                   teardown_soup_connection);
+       g_test_add ("/websocket/direct/close-clean-client", Test, NULL, NULL,
+                   test_close_clean_client_direct,
+                   NULL);
+       g_test_add ("/websocket/soup/close-clean-client", Test, NULL, NULL,
+                   test_close_clean_client_soup,
+                   NULL);
 
-       g_test_add ("/websocket/direct/close-clean-server", Test, NULL,
-                   setup_direct_connection,
-                   test_close_clean_server,
-                   teardown_direct_connection);
-       g_test_add ("/websocket/soup/close-clean-server", Test, NULL,
-                   setup_soup_connection,
-                   test_close_clean_server,
-                   teardown_soup_connection);
+       g_test_add ("/websocket/direct/close-clean-server", Test, NULL, NULL,
+                   test_close_clean_server_direct,
+                   NULL);
+       g_test_add ("/websocket/soup/close-clean-server", Test, NULL, NULL,
+                   test_close_clean_server_soup,
+                   NULL);
 
        g_test_add ("/websocket/direct/message-after-closing", Test, NULL,
                    setup_direct_connection,