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

msg to user from second client appears in tiny as writing to yourself #271

Closed
shumvgolove opened this issue Nov 19, 2020 · 24 comments
Closed
Labels

Comments

@shumvgolove
Copy link

tiny 0.7.0 (d52029f)

Currently I am running soju bouncer and this error appears only with tiny client:

  1. Send private message from second client (irssi or revolution irc, for example) to user.
  2. In second client, message to user appears in user tab.
  3. tiny client shows this message to user in tab with your nickname.
@osa1
Copy link
Owner

osa1 commented Nov 20, 2020

Thanks for reporting.

I'm trying to understand how to reproduce the issue locally. I guess I should run soju and then connect to it using two clients, tiny and another one, right? Then send a message to someone (to our own nick or someone else?) from the other client (not tiny). The tab created in the other client is as expected but in tiny the tab name is wrong. Did I get this right?

@osa1 osa1 added the bug label Nov 20, 2020
@osa1
Copy link
Owner

osa1 commented Nov 20, 2020

Other bugs that soju revealed previously: (both fixed a while ago)

@osa1
Copy link
Owner

osa1 commented Nov 20, 2020

I'm unable to run soju locally (just created this issue) so not sure how to debug currently. I'll get back to this after I'm able to run soju locally.

@shumvgolove
Copy link
Author

@osa1 Yes, your understanding is correct.

@osa1
Copy link
Owner

osa1 commented Nov 20, 2020

Thanks for confirming. I will fix this once I'm able to run soju locally and reproduce the issue.

@osa1
Copy link
Owner

osa1 commented Nov 20, 2020

Hmm, I'm a bit confused. I couldn't see any bugs involving two clients, but here's a weird behavior or potentially a bug. If I send a message to myself using osa1/oftc instead of osa1 in tiny, using

/msg osa1/oftc test

Two tabs are crated: osa1 and osa1/oftc. The first tab shows the incoming message, the second one shows the message I sent.

I think this is not a bug. To show outgoing messages we use the name user specified, in this case osa1/oftc, but the message coming from the server is to osa1 so the incoming message is shown in the tab for osa1.

I guess the problem is soju expects you to add suffix to nicks when sending privmsgs (e.g. PRIVMSG osa1 :... is not accepted, I need PRIVMSG osa1/oftc :...), but doing that creates a tab for osa1/oftc instead of osa1.

@osa1
Copy link
Owner

osa1 commented Nov 20, 2020

Interestingly, you can't send a message to yourself via soju on konversation without using /msg syntax. If you create a tab by clicking on your nick that creates a tab for "osa1" not for "osa1/oftc", and in soju you need the server suffix.

@shumvgolove
Copy link
Author

shumvgolove commented Nov 20, 2020

I've recorder the behavior: gif.

@osa1
Copy link
Owner

osa1 commented Nov 20, 2020

If you send a message to testestest (as in /msg testestest test2 that you do in the recording) the message will be shown in the tab for testestest, there is no other way to implement this. I think most other clients don't create a tab when you do /msg ..., so this behavior doesn't exist in other clients.

Can you show me a client that does what you expect?

@shumvgolove
Copy link
Author

shumvgolove commented Nov 20, 2020

No, I meant a different thing.

In the recorded behaviour, running /msg testestest test in irssi creates the testestest tab and shows my message there (correct behaviour). On the other hand, tiny shows my message to testestest in tab with my nickname (incorrect behaviour), as if I was writing that message to myself.

@osa1
Copy link
Owner

osa1 commented Nov 20, 2020

If you connect with the nick you used in irssi in the recording with tiny, and connect with the nick you used in tiny with irssi (in other words just swap the clients) and run the same test, do you get a different output?

Maybe I'm an idiot but I can't tell what your nick is in irssi. It shows "testestest" at the bottom (in the input line), but when you send "hello" it sends as "shhhum", which should be the nick used by tiny not irssi. I'm confused...

@shumvgolove
Copy link
Author

shumvgolove commented Nov 20, 2020

I've recorder another one, we can figure this out 🙃 : gif

I have setup three clients with 2 accounts: irssi and tiny for shhhum account and Revoultion IRC for testestest.

  1. I am sending my message from tiny (shhhum) to testestest and it correctly appears in testestest tab.
  2. I am switching to irssi client ([testestest] in the bottom left corner indicates currently opened tab) and sending my message from Revolution IRC (testestest) to shhhum. tiny (and irssi) once again correctly shows this message in testestest tab.
  3. But then, I'm sending my message from irssi (shhhum) to testestest. Both Revolution IRC and irssi shows messages in testestest tab, but tiny shows the last message in shhhum tab (incorrect, the last message should have been appeared in testestest tab).

@osa1
Copy link
Owner

osa1 commented Nov 21, 2020

In the recording I don't understand the first command you're running in tiny. You're connected to soju and you're running /msg testestest ..., and in soju that nick is not valid, I think. You need to add server suffix, e.g. /msg testestest/oftc blah blah instead of /msg testestest blah. Are you sure that you can receive the message you send in tiny from "testestest"?

@osa1
Copy link
Owner

osa1 commented Nov 21, 2020

OK here's the problem explained in my own words. soju allows multiple clients to connect to the same account and it relays messages sent by one to the others. So for example if client 1 and 2 connect to soju, and client 1 sends a message to some nick (say osa1/oftc), then the message sent by 1 is relayed to 2.

The problem is this relayed message causes creating a tab with the user's own nick, as if they're sending a message to themselves.

This problam happens in other clients as well. So far I tried this with tiny, konversation, and hexchat. They all behaved the same: when I send a message from one of these clients with /msg some_nick/oftc blah blah, the other two clients created a tab with name "osa1-soju".

Here's a demonstration. Below osa1/oftc is my main client connected directly (no soju). osa1-soju is soju. All three clients in the screenshot are connected to soju (osa1/oftc is not shown).

Before I send a message to osa1/oftc:

Screenshot_2020-11-21_14-13-12

After I send it the other two clients (hexchat and tiny) creates a tab as if I'm messaging to myself:

Screenshot_2020-11-21_14-13-39

I can do this in any direction. For example if I send the message from tiny connected to soju then the other two clients (hexchat and konversation) create a tab looking as if I'm messaging to myself.

I will check the what the exact PRIVMSG command is relayed by soju, but given that konversation and hexchat also have the same "problem" I doubt that this is really a bug in tiny.

@osa1
Copy link
Owner

osa1 commented Nov 21, 2020

OK, I see what's going on here.

Here's the parsed message when soju relays a message:

Msg {
    pfx: Some(
        User {
            nick: "osa1-soju",
            user: "osa1-soju@127.0.0.1"
        }
    ),
    cmd: PRIVMSG {
        target: User("osa1/oftc"),
        msg: "blah blah",
        is_notice: false,
        ctcp: None
    }
}

So the sender is us, and target is osa1/oftc, which makes sense.

The problem is normally (without soju) when we get a PRIVMSG from a nick that means the sender (pfx above) is that nick and the message is sent to us, because normally a server wouldn't send a PRIVMSG sent to someone else to you. However with soju this is not the case.

This is the case I commented as "not sure if this case can happen" in the source code:

tiny/tiny/src/conn.rs

Lines 211 to 212 in d52029f

// Not sure if this case can happen
ui.set_tab_style(TabStyle::NewMsg, &msg_target);

We need to update that code to handle the case where target of a message is not us.

@osa1
Copy link
Owner

osa1 commented Nov 21, 2020

I wonder if this is also causing problems when using znc.

@shumvgolove
Copy link
Author

shumvgolove commented Nov 21, 2020

Hm. soju requires putting network after channel name if you're using it in the multi upstream mode. In the single upstream mode you can connect to any channel (including private) like you would normally do.

Are you sure that you can receive the message you send in tiny from "testestest"?

So the answer is yes, I can see the message in Revolution IRC (testestest) client.

@shumvgolove
Copy link
Author

And sorry for the gif, but the original recording weighed almost 1.5GB :^(

@osa1 osa1 closed this as completed in 62a4deb Nov 21, 2020
@osa1
Copy link
Owner

osa1 commented Nov 21, 2020

This should be fixed now. Thanks @shumvgolove for reporting this and being patient with me :-)

@osa1 osa1 reopened this Nov 21, 2020
@osa1
Copy link
Owner

osa1 commented Nov 21, 2020

Previous commit was not quite ready. Will push the correct version in a minute.

@osa1 osa1 closed this as completed in db38123 Nov 21, 2020
@osa1
Copy link
Owner

osa1 commented Nov 21, 2020

Should be fixed for real now.

@osa1 osa1 mentioned this issue Nov 21, 2020
4 tasks
@osa1
Copy link
Owner

osa1 commented Nov 22, 2020

This was a bit difficult to test, @shumvgolove it'd be helpful if you could confirm that this is really fixed.

@shumvgolove
Copy link
Author

shumvgolove commented Nov 22, 2020

Sorry, I've been busy this two days. Just compiled new version and it indeed fixes the issue! Thank you @osa1 for working on this great IRC client!

@osa1
Copy link
Owner

osa1 commented Nov 22, 2020

Sorry, I've been busy this two days. Just compiled new version and it indeed fixes the issue!

No problem, thanks for confirming.

Thank you @osa1 for working on this great IRC client!

Glad you liked it :-)

osa1 added a commit that referenced this issue Dec 5, 2020
osa1 added a commit that referenced this issue Dec 8, 2020
Some refactoring done to allow more testing.

Fixes #278. Adds a test for #271.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants