Refactor return values from secure-join message handling

This return value was very complicated to understand.  Some failure
returns were returned as Err and some as Ok with no consistency, but
resulting in the same behaviour.

This refactor makes the handle_securejoin_handshake the sole place
responsible for maintaining the state of the secure join
process (context.bob) and also in charge of terminating the ongoing
process.  This is none of receive_imf's business.

The remaining returns are now cleanly classified in application-errors
and protocol errors:

Applications errors result in an Err and mean there is a bug or
something else serious went wrong, like database access suddenly
failed or so.  In this case receive_imf is still responsible for
clearing the state and resetting ongoing-process.  It may be possible
this should still be moved back to securejoin.rs so that recieve_imf
doesn't need to know anything about this either.

Protocol errors are not failures for receive_imf, it just means the
received message didn't follow the protocol.  Receive_imf in this case
is told to ignore the message: that is hide it but not delete it.

Other Ok returns also only say what needs to happen to the message:

- It's fully processed and needs no further processing, instead should
  be removed

- It should still be processed as a normal received message.

This changes some behaviour: if the chat creation/lookup for the
contact fails this is treated as an application error.  Previously
this was silently ignored and send_msg() would be called with a 0
chat_id without checking the response.  This resulted in the protocol
quitely being blocked.

This all shhould now make it easier to resultify more of the functions
called by this function, instead of having to deal with very
complicated application logic hidden in the return code.
This commit is contained in:
Floris Bruynooghe
2020-01-12 22:27:40 +01:00
committed by Floris Bruynooghe
parent 454c52f4ab
commit c82fdb9fa1
2 changed files with 132 additions and 108 deletions

View File

@@ -17,7 +17,7 @@ use crate::message::{self, MessageState, MessengerMessage, MsgId};
use crate::mimeparser::*;
use crate::param::*;
use crate::peerstate::*;
use crate::securejoin::handle_securejoin_handshake;
use crate::securejoin::{self, handle_securejoin_handshake};
use crate::sql;
use crate::stock::StockMessage;
use crate::{contact, location};
@@ -328,7 +328,6 @@ fn add_parts(
MessageState::InFresh
};
to_id = DC_CONTACT_ID_SELF;
let mut needs_stop_ongoing_process = false;
// handshake messages must be processed _before_ chats are created
// (eg. contacs may be marked as verified)
@@ -338,26 +337,24 @@ fn add_parts(
*chat_id = 0;
allow_creation = true;
match handle_securejoin_handshake(context, mime_parser, from_id) {
Ok(ret) => {
if ret.hide_this_msg {
*hidden = true;
*needs_delete_job = ret.delete_this_msg;
state = MessageState::InSeen;
}
if let Some(status) = ret.bob_securejoin_success {
context.bob.write().unwrap().status = status as i32;
}
needs_stop_ongoing_process = ret.stop_ongoing_process;
Ok(securejoin::HandshakeMessage::Done) => {
*hidden = true;
*needs_delete_job = true;
state = MessageState::InSeen;
}
Ok(securejoin::HandshakeMessage::Ignore) => {
*hidden = true;
state = MessageState::InSeen;
}
Ok(securejoin::HandshakeMessage::Propagate) => {
// Message will still be processed as "member
// added" or similar system message.
}
Err(err) => {
// maybe this message belongs to an aborted scan,
// however, by the explicit header check we really know
// that it is a Secure-Join message that should be hidden in the chat view.
*hidden = true;
warn!(
context,
"Unexpected messaged passed to Secure-Join handshake protocol: {}", err
);
context.bob.write().unwrap().status = 0; // secure-join failed
context.stop_ongoing();
error!(context, "Error in Secure-Join message handling: {}", err);
}
}
}
@@ -454,13 +451,6 @@ fn add_parts(
{
state = MessageState::InNoticed;
}
if needs_stop_ongoing_process {
// The Secure-Join protocol finished and the group
// creation handling is done. Stopping the ongoing
// process will let dc_join_securejoin() return.
context.stop_ongoing();
}
} else {
// Outgoing