test: Don't accidentally accept that a chat protection is broken #4550
Conversation
| @@ -398,7 +398,10 @@ impl ChatId { | |||
|
|
|||
| match chat.typ { | |||
| Chattype::Undefined => bail!("Can't accept chat of undefined chattype"), | |||
| Chattype::Single if chat.protected == ProtectionStatus::ProtectionBroken => { | |||
| Chattype::Single | |||
| if chat.blocked == Blocked::Not | |||
There was a problem hiding this comment.
Why this check was added?
There was a problem hiding this comment.
The idea of this PR is:
send_recv_accept() asserts that the chat is a request, and if a chat is not a request, accept() doesn't accept the protection-brokenness.
I.e. this check makes sure that send_recv_accept() never accidentally accepts the protection-brokenness.
Also, it just seemed the correct behavior to me that if a chat is both a request and protection-broken, the request is accepted. Though this shouldn't actually happen, anyway, so, this doesn't really matter.
...That said, this line isn't a very important part of the PR, I can also revert it.
There was a problem hiding this comment.
But why then Blocked::Not is here, not Blocked::Request?
There was a problem hiding this comment.
@iequidoo if chat.blocked == Blocked::Not, i.e. it's NOT a Request, then this accepts the broken protection.
Otherwise (Blocked::Request or Blocked::Yes), this goes on to the next match arm, and accepts the request / unblocks the chat.
There was a problem hiding this comment.
Ok, but is the comment below correct then?
The chat was in the 'Request' state because the protection was broken.
Because the chat is not a request, but has a broken protection.
There was a problem hiding this comment.
Ah, yes, I'll change the comment. It's from an early prototype when chats with a broken protection were also marked as Requests.
There was a problem hiding this comment.
But as I said, it'd be no problem to revert this change, the important part of the change is in test_utils.rs.
|
@Hocuri In the discussion you said "Ah, yes, I'll change the comment." and the comment is not changed so I assumed it is not ready for review yet. Is there some change not pushed? |
|
Oops yes, forgot to commit&push this, thanks for reminding! |
Hocuri
force-pushed
the
hoc/dont-accidentally-accept
branch
from
1f64227
to
40c2d9c
Compare
In one of the tests I wrote, I called `send_recv_accept()`, which accepted the protection-brokenness, and I spent a long time debugging this. So, `send_recv_accept()` asserts that the chat is a request, and if a chat is not a request, accept() doesn't accept the protection-brokenness.
Hocuri
force-pushed
the
hoc/dont-accidentally-accept
branch
from
40c2d9c
to
e297c70
Compare
In one of the tests I wrote, I called
send_recv_accept(), which accepted the protection-brokenness, and I spent a long time debugging this.So,
send_recv_accept()asserts that the chat is a request, and if a chat is not a request, accept() doesn't accept the protection-brokenness.