laudrup/boost-wintls

"Unknown Socket type in teardown" when using with Boost Beast Websockets

gitgitwhat opened this issue · 6 comments

First, thanks for building this! As great as OpenSSL is, it is not friendly for Windows devs so this is just what I was looking for. I'm having a bit of a problem though. I've taken your HTTP example and combined it with the Beast websocket example (https://github.com/boostorg/beast/blob/master/example/websocket/client/sync-ssl/websocket_client_sync_ssl.cpp) to try to get secure websockets. Sample code is as follows:

namespace beast = boost::beast;
namespace http = beast::http;
namespace websocket = beast::websocket;
namespace net = boost::asio;
namespace ssl = boost::wintls;

using tcp = boost::asio::ip::tcp;

void test() {
    std::string host = "echo.websocket.org";
    auto const port = "443";
    auto const text = "test123";
    net::io_context ioc;

    ssl::context ctx{ boost::wintls::method::system_default };
    tcp::resolver resolver{ ioc };

    websocket::stream<ssl::stream<tcp::socket>> ws{ ioc, ctx };

    auto const results = resolver.resolve(host, port);
    auto ep = net::connect(beast::get_lowest_layer(ws), results);

    host += ':' + std::to_string(ep.port());
    ws.next_layer().handshake(boost::wintls::handshake_type::client);

    ws.set_option(websocket::stream_base::decorator(
        [](websocket::request_type& req)         {
            req.set(http::field::user_agent,
            std::string(BOOST_BEAST_VERSION_STRING) +
            " websocket-client-coro");
        }));
    ws.handshake(host, "/");

    ws.write(net::buffer(std::string(text)));

    beast::flat_buffer buffer;
    ws.read(buffer);

    ws.close(websocket::close_code::normal);
    std::cout << beast::make_printable(buffer.data()) << std::endl;
}

Problem is it doesn't compile the Beast teardown doesn't match

teardown.hpp(54,33): error C2338: Unknown Socket type in teardown.

I'm new to Boost so this is a confusing error. How can wintls work with Beast for websockets?

Hi @gitgitwhat,

I'm really happy to hear that someone is actually using this.

To be honest I haven't spend much time on it lately, mostly due to lack of time but definitely also because of lack of interest from the outside world so getting some kind of feedback means a lot.

I'll try to have a look at it and get back to you, although I cannot really promise when that will be.

If you figure out the reason for the type mismatch, please let me know. Thanks.

Thanks for the response.

I have not figured it out yet. I can get it to compile if I comment out the assert in Boost Beast but that's obviously not a good solution.

I tried following the example Beast provides here (https://www.boost.org/doc/libs/develop/libs/beast/doc/html/beast/using_websocket/teardown.html) but my code just fails compiling with a different error. Maybe @vinniefalco can chime in as the author of Beast.

WebSockets.cpp(27,1): error C2955: 'boost::wintls::stream': use of class template requires template argument list
boost\wintls\stream.hpp(45): message : see declaration of 'boost::wintls::stream'
WebSockets.cpp(46): message : see reference to class template instantiation 'wintls_stream<NextLayer>' being compiled

template<class NextLayer>
class wintls_stream : public boost::wintls::stream {
    template<class SyncStream>
    friend
        void
        teardown(
            boost::beast::role_type role,
            wintls_stream<SyncStream>& stream,
            boost::system::error_code& ec);

    template<class AsyncStream, class TeardownHandler>
    friend
        void
        async_teardown(
            boost::beast::role_type role,
            wintls_stream<AsyncStream>& stream,
            TeardownHandler&& handler);
};

template<class SyncStream>
static inline void teardown(boost::beast::role_type role, wintls_stream<SyncStream>& stream, boost::system::error_code& ec) {
    // Just forward it to the underlying ssl::stream
    using boost::beast::websocket::teardown;
    teardown(role, *stream.p_, ec);
}

template<class AsyncStream, class TeardownHandler>
static inline void async_teardown(boost::beast::role_type role, wintls_stream<AsyncStream>& stream, TeardownHandler&& handler) {
    // Just forward it to the underlying ssl::stream
    using boost::beast::websocket::async_teardown;
    async_teardown(role, *stream.p_, std::forward<TeardownHandler>(handler));
}

Thanks for taking your time to look into this in detail.

This is most certainly a bug in my library, so removing the assertion from boost::beast is not only a bad solution, it is most likely just hiding a real issue that will just show up in other strange ways as I'm certain the assertion is there for a reason.

I assume wintls::stream doesn't fulfill all the requirements its should to be used this way, which is very likely, but I have to look into to it more in detail. Since this is supposed to be used the exact same way as asio::ssl I would of course like this to be possible.

You are more than welcome if you can figure out what is missing/wrong with my code and submit a PR as I'm not sure when I'll have the time to look into this. For personal reasons I'm a bit busy at the moment.

I've had a quick look at this and I think it might be possible to get this to compile and work by more or less reusing the code from beast/websocket/impl/ssl.hpp.

As far as I can tell, it just calls shutdown on the stream which should work the same by calling shutdown on a boost::wintls::stream.

I don't understand the reasoning behind requiring this in the first place to be honest, but the beast definitely know what they're doing, so I'm sure there's a good reason.

Since this is something that is required specifically to use beast::websocket and beast::websocket specifically provides an implementation for asio::ssl itself, I consider this more of a missing feature/header in boost::wintls than a bug though.

But let's leave this issue open as it would be nice to have, although I must admit I don't consider it to be critical.

Adding this before your test function ought to do it:

namespace boost {
namespace beast {
template <class SyncStream>
void teardown(boost::beast::role_type, boost::wintls::stream<SyncStream>& stream, boost::system::error_code& ec) {
  stream.shutdown(ec);
}
}
}

I guess I could add this as well as the async version to a header in the boost::wintls library. I'll look into that.

Awesome!

So I actually took a cue from the ssl.hpp code and refactored it for wintls but was still having an issue. However, I did not wrap it in in the boost and beast namespace. Once I did that as you've shown, it now compiles fine. Full code is below. I just have it at the top of my code file so I'm not sure where it would need to be integrated into the wintls code (if at all).

I still need to test to make sure this actually works but at least it gets past the compile assert.

namespace boost {
    namespace beast {
        template<class SyncStream>
        void teardown(boost::beast::role_type role, boost::wintls::stream<SyncStream>& stream, boost::system::error_code& ec);

        template<class AsyncStream>
        void teardown(boost::beast::role_type role, boost::wintls::stream<AsyncStream>& stream, boost::system::error_code& ec) {
            stream.shutdown(ec);
            using boost::beast::websocket::teardown;
            boost::system::error_code ec2;
            teardown(role, stream.next_layer(), ec ? ec2 : ec);
        }

        template<class AsyncStream, class TeardownHandler>
        void async_teardown(boost::beast::role_type role, boost::wintls::stream<AsyncStream>& stream, TeardownHandler&& handler);

        namespace wintls_async_td_detail {
            template<class AsyncStream>
            struct wintls_shutdown_op : boost::asio::coroutine {
                wintls_shutdown_op(boost::wintls::stream<AsyncStream>& s, boost::beast::role_type role) : s_(s), role_(role) {
                }

                template<class Self>
                void operator()(Self& self, boost::system::error_code ec = {}, std::size_t = 0) {
                    BOOST_ASIO_CORO_REENTER(*this) {
                        BOOST_ASIO_CORO_YIELD
                            s_.async_shutdown(std::move(self));
                        ec_ = ec;

                        using boost::beast::websocket::async_teardown;
                        BOOST_ASIO_CORO_YIELD
                            async_teardown(role_, s_.next_layer(), std::move(self));
                        if (!ec_) {
                            ec_ = ec;
                        }

                        self.complete(ec_);
                    }
                }

            private:
                boost::wintls::stream::stream<AsyncStream>& s_;
                boost::beast::role_type role_;
                boost::system::error_code ec_;
            };
        }

        template<class AsyncStream, class TeardownHandler>
        void async_teardown(boost::beast::role_type role, boost::wintls::stream<AsyncStream>& stream, TeardownHandler&& handler) {
            return boost::asio::async_compose<TeardownHandler, void(boost::system::error_code)>(
                wintls_async_td_detail::wintls_shutdown_op<AsyncStream>(stream, role), handler, stream);
        }
    }
}