crossbario / autobahn-cpp

WAMP for C++ in Boost/Asio
https://crossbar.io/autobahn
Boost Software License 1.0
251 stars 104 forks source link

send_message on wamp_session::process_goodbye throws always #224

Open pratman opened 3 years ago

pratman commented 3 years ago

with:

  1. process_goodybe setting m_session_id = 0;
  2. calling send_message with parameter session_established defaulted to true
  3. condition if (session_established && !m_session_id) will always be true

this results into throwing no_session_error() that will be uncaught and crash the application

KnoerleMan commented 3 years ago

Does setting the session id to zero has to be done in process_goodbye (line 834) ? This is done when leaving the session inside the dispatched handler (line 225).

For me it looks like setting the id to zero at that point should not be done, since we only want to reply to a goodbye message from the peer, or do nothing in case we called leave.

pratman commented 3 years ago

I think at least is clear that it should not be set to 0 before send_message as it will just always throw.

oberstet commented 3 years ago

that sounds like a bug: obviously, to send a goodbye or reply to one, the session ID is still needed. it should be zero'ed only after that. it should be zero'ed, since that signals "no session currently attached". after replying to a goodbye with a goodbye, no further message must be sent. when sending a goodbye in the first place (active leave), a response is expected but no additional message must be sent. IOW: goodbye is always the last message sent on a session.

pratman commented 3 years ago

Additionally, all other send_message calls are wrapped on try-catch, but the ones in process_goodbye and process_invocation.

Also setting session id to 0, signals "internally" that no more messages should be sent. But for the user of the session, there is no inmediate notification about it. Caller of session would also need to get such information ASAP.

oberstet commented 3 years ago

Also setting session id to 0, signals "internally" that no more messages should be sent. But for the user of the session, there is no inmediate notification about it. Caller of session would also need to get such information ASAP.

yes, absolutely: the "on-leave" should fire, and allow user code to react