Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wamp_session::process_challenge protocol_error throws will crash the application #232

Open
pratman opened this issue Feb 7, 2022 · 6 comments

Comments

@pratman
Copy link

pratman commented Feb 7, 2022

m_session_join has to take the exception instead of throwing so connecting(and challenged) client can react on .get() instead of the whole application crashing from the unhandled exception (which is thrown inside of the asio_context loop).

one of the places with a possible fix:
image

@oberstet
Copy link
Contributor

oberstet commented Feb 7, 2022

why is that required? calling code can catch exceptions thrown in async code like this https://github.com/crossbario/autobahn-cpp/blob/master/examples/caller.cpp

@oberstet
Copy link
Contributor

oberstet commented Feb 7, 2022

mmh ... wait .. maybe I'm confused .. other places in the session code do similar as you suggest:

@oberstet
Copy link
Contributor

oberstet commented Feb 7, 2022

aaah, ok: so the session code throws protocol errors, but uses setting the exception on a future in cases the application can / want to react to in a regular fashion.

so the actual Q is: in this specific case, the signature of the challenge sent by the client could not be verified by the router, and hence the router denies the client authentication.

this isn't a protocol error so ..

@pratman
Copy link
Author

pratman commented Feb 7, 2022

router didnt deny anything, send_message threw from any reason such as no transport.
in any case application shouldnt crash.
joining client is waiting on the .get() and should get the exception through the future (like everywhere else).
only in this method is done incorrectly.
i think real question is whether failing to send a message should crash an application

@oberstet
Copy link
Contributor

oberstet commented Feb 7, 2022

well, why would throwing an exception crash a program necessarily? just catch it? not sure, I'm not using c++ often these days, and these arcane styles;)

anyways, this code has the pattern I believe to be correct:

inline void wamp_session::process_abort(wamp_message&& message)

rethinking about the issue you posted: if a signature could not be verified, the WAMP openening handshake fails, and a completely new session and handshake must be performed (at the WAMP protocol level).

so that means, m_session_join should complete with error, but it is not a fatal error (like eg the router talking gibberish bytes ..)

@pratman
Copy link
Author

pratman commented Feb 7, 2022

Thanks for looking into this so quickly.
This challenge handling happens within the asio context and exceptions thrown within these methods such as process_challenge cannot be caught by the application.
The only way to signal such error from the context thread to the application is by using the futures, as done in the other methods.
In process_abort as you mentioned above, the throws will also remain unhandled and lead to a crash.

Example of the stack:

VCRUNTIME140!_CxxThrowException+0x66 [throw.cpp @ 74]
YOURAPP!<lambda_ef10035f957bb69c2f0ada66cf891c1f>::operator()+0x117 [wamp_session.ipp @ 788]
YOURAPP!boost_asio_handler_invoke_helpers::invoke+0xe [handler_invoke_helpers.hpp @ 37]
YOURAPP!boost::asio::detail::handler_work<<lambda_ef10035f957bb69c2f0ada66cf891c1f>,boost::asio::system_executor,boost::asio::system_executor>::complete+0xe [handler_work.hpp @ 100]
YOURAPP!boost::asio::detail::completion_handler<<lambda_ef10035f957bb69c2f0ada66cf891c1f> >::do_complete+0x11d [completion_handler.hpp @ 70]
YOURAPP!boost::asio::detail::win_iocp_operation::complete+0x1d [win_iocp_operation.hpp @ 47]
YOURAPP!boost::asio::detail::win_iocp_io_context::do_one+0x356 [win_iocp_io_context.ipp @ 461]
YOURAPP!boost::asio::detail::win_iocp_io_context::run+0xeb [win_iocp_io_context.ipp @ 203]
YOURAPP!boost::asio::io_context::run+0x24 [io_context.ipp @ 64]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants