WebSocket handshake response is deterministic on failure:

Add test for websocket handshake failure HTTP response

fix #2364
close #2365

CHANGELOG.md

@@ -1,3 +1,7 @@
+* WebSocket handshake response is deterministic on failure.
+
+--------------------------------------------------------------------------------
+
 Version 324:
 
 * Fix open append mode for file_posix.

include/boost/beast/websocket/impl/handshake.hpp

@@ -225,6 +225,9 @@ do_handshake(
     RequestDecorator const& decorator,
     error_code& ec)
 {
+    if(res_p)
+        res_p->result(http::status::internal_server_error);
+
     auto& impl = *impl_;
     impl.change_status(status::handshake);
     impl.reset();
@@ -275,12 +278,18 @@ do_handshake(
     if(impl.check_stop_now(ec))
         return;
 
-    impl.on_response(p.get(), key, ec);
-    if(impl.check_stop_now(ec))
-        return;
-
-    if(res_p)
+    if (res_p)
+    {
+        // If res_p is not null, move parser's response into it.
         *res_p = p.release();
+    }
+    else
+    {
+        // Otherwise point res_p at the response in the parser.
+        res_p = &p.get();
+    }
+
+    impl.on_response(*res_p, key, ec);
 }
 
 //------------------------------------------------------------------------------
@@ -325,6 +334,7 @@ async_handshake(
     detail::sec_ws_key_type key;
     auto req = impl_->build_request(
         key, host, target, &default_decorate_req);
+    res.result(http::status::internal_server_error);
     return net::async_initiate<
         HandshakeHandler,
         void(error_code)>(

include/boost/beast/websocket/stream.hpp

@@ -675,7 +675,11 @@ public:
 
         @param res The HTTP Upgrade response returned by the remote
         endpoint. The caller may use the response to access any
-        additional information sent by the server.
+        additional information sent by the server. Note that the response object
+        referenced by this parameter will be updated as long as the stream has
+        received a valid HTTP response. If not (for example because of a communications
+        error), the response contents will be undefined except for the result() which
+        will bet set to 500, Internal Server Error.
 
         @param host The name of the remote host. This is required by
         the HTTP protocol to set the "Host" header field.

test/beast/websocket/handshake.cpp

@@ -724,6 +724,97 @@ public:
     }
 #endif
 
+    void
+    testIssue2364()
+    {
+        { // sync unauthorized
+            net::io_context ioc;
+            stream<test::stream> ws{ioc};
+            auto tr = connect(ws.next_layer());
+            std::string const reply =
+                "HTTP/1.1 401 Unauthorized\r\n"
+                "\r\n";
+            ws.next_layer().append(reply);
+            tr.close();
+            try
+            {
+                websocket::response_type response;
+                error_code ec;
+                ws.handshake(response, "localhost:80", "/", ec);
+                BEAST_EXPECT(ec);
+                BEAST_EXPECTS(response.result() == http::status::unauthorized,
+                                 std::to_string(response.result_int()));
+            }
+            catch(system_error const&)
+            {
+                fail();
+            }
+        }
+
+        { // sync invalid response
+            net::io_context ioc;
+            stream<test::stream> ws{ioc};
+            auto tr = connect(ws.next_layer());
+            std::string const reply =
+                    "zzzzzzzzzzzzzzzzzzzzz";
+            ws.next_layer().append(reply);
+            tr.close();
+            try
+            {
+                websocket::response_type response;
+                error_code ec;
+                ws.handshake(response, "localhost:80", "/", ec);
+                BEAST_EXPECT(ec);
+                BEAST_EXPECTS(response.result() == http::status::internal_server_error,
+                              std::to_string(response.result_int()));
+            }
+            catch(system_error const&)
+            {
+                fail();
+            }
+        }
+
+        { // async unauthorized
+            net::io_context ioc;
+            stream<test::stream> ws{ioc};
+            auto tr = connect(ws.next_layer());
+            std::string const reply =
+                "HTTP/1.1 401 Unauthorized\r\n"
+                "\r\n";
+            ws.next_layer().append(reply);
+            tr.close();
+            websocket::response_type response;
+            auto handler = [&](error_code ec)
+            {
+                BEAST_EXPECT(ec);
+                BEAST_EXPECTS(response.result() == http::status::unauthorized,
+                                  std::to_string(response.result_int()));
+            };
+            ws.async_handshake(response, "localhost:80", "/", handler);
+            ioc.run();
+        }
+
+        { // async invalid response
+            net::io_context ioc;
+            stream<test::stream> ws{ioc};
+            auto tr = connect(ws.next_layer());
+            std::string const reply =
+                    "zzzzzzzzzzzzzzzz";
+            ws.next_layer().append(reply);
+            tr.close();
+            websocket::response_type response;
+            auto handler = [&](error_code ec)
+            {
+                BEAST_EXPECT(ec);
+                BEAST_EXPECTS(response.result() == http::status::internal_server_error,
+                              std::to_string(response.result_int()));
+            };
+            ws.async_handshake(response, "localhost:80", "/", handler);
+            ioc.run();
+        }
+    }
+
+
     void
     run() override
     {
@@ -737,6 +828,7 @@ public:
 #if BOOST_ASIO_HAS_CO_AWAIT
         boost::ignore_unused(&handshake_test::testAwaitableCompiles);
 #endif
+        testIssue2364();
     }
 };