Repository: osa1/tiny (language: rust)

Issue:
Prefix without '!' assumed to be a server
RFC2812 defines prefix as follows:
```
prefix     =  servername / ( nickname [ [ "!" user ] "@" host ] )
```

By this syntax, a prefix can be proven to be a nick if it contains either '!' or '@'. Otherwise, it is ambigious.

However, libtiny_wire treats any prefix not containing a '!' as a server name, and tiny replaces server names with the connection connection server. This leads to situations where all history-playback from a bouncer like soju ends up replacing all nicks with the server name.

For non-zero prefixes, a simple `struct { name, user, host }` would probably make the most sense, removing the distinction. Empty prefixes could then be communicated separately, either through an option or an empty Server enum value, for when tiny needs to add the server name itself in the UI.


Discussion / hints:
Thanks for reporting. You're right that we don't handle the ambiguous case correctly, instead we assume that the sender is a server.

I think we could improve the situation by searching for '.' and '-' characters in the prefix when there isn't a '!' or '@', because nicks can't have those characters. I think this would solve the problem when connecting to soju, right?

If the prefix doesn't have those characters and doesn't have '!' or '@' then there will still be ambiguity, which I'm not sure how to handle best. Perhaps in that case assuming server as we do today is fine as I never seen this become a problem in years of use until this bug report.
I tried to run soju locally to see the problem myself and whether my proposed fix above will work, but I can't follow the instructions in the README:

```
$ go run ./cmd/sojuctl create-user soju-osa1 -admin
warning: GOPATH set to GOROOT (/home/omer/go) has no effect
go: downloading golang.org/x/crypto v0.0.0-20200820211705-5c72a883971a
go: downloading github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510
go: downloading gopkg.in/irc.v3 v3.1.3
go: downloading github.com/emersion/go-sasl v0.0.0-20200509203442-7bfe0ed36a21
go: downloading nhooyr.io/websocket v1.8.6
go: downloading github.com/mattn/go-sqlite3 v1.14.1
go: downloading github.com/klauspost/compress v1.10.11
go: downloading golang.org/x/sys v0.0.0-20200824131525-c12d262b63d8
# runtime/internal/atomic
../../go/src/runtime/internal/atomic/atomic_amd64x.go:13:6: Load redeclared in this block
        previous declaration at ../../go/src/runtime/internal/atomic/atomic_amd64.go:16:24
../../go/src/runtime/internal/atomic/atomic_amd64x.go:19:6: Loadp redeclared in this block
        previous declaration at ../../go/src/runtime/internal/atomic/atomic_amd64.go:22:32
../../go/src/runtime/internal/atomic/atomic_amd64x.go:25:6: Load64 redeclared in this block
        previous declaration at ../../go/src/runtime/internal/atomic/atomic_amd64.go:28:26
../../go/src/runtime/internal/atomic/atomic_amd64x.go:30:6: Xadd redeclared in this block
        previous declaration at ../../go/src/runtime/internal/atomic/atomic_amd64.go:39:37
../../go/src/runtime/internal/atomic/atomic_amd64x.go:33:6: Xadd64 redeclared in this block
        previous declaration at ../../go/src/runtime/internal/atomic/atomic_amd64.go:42:39
../../go/src/runtime/internal/atomic/atomic_amd64x.go:36:6: Xadduintptr redeclared in this block
        previous declaration at ../../go/src/runtime/internal/atomic/atomic_amd64.go:45:47
../../go/src/runtime/internal/atomic/atomic_amd64x.go:39:6: Xchg redeclared in this block
        previous declaration at ../../go/src/runtime/internal/atomic/atomic_amd64.go:48:36
../../go/src/runtime/internal/atomic/atomic_amd64x.go:42:6: Xchg64 redeclared in this block
        previous declaration at ../../go/src/runtime/internal/atomic/atomic_amd64.go:51:38
../../go/src/runtime/internal/atomic/atomic_amd64x.go:45:6: Xchguintptr redeclared in this block
        previous declaration at ../../go/src/runtime/internal/atomic/atomic_amd64.go:54:45
../../go/src/runtime/internal/atomic/atomic_amd64x.go:48:6: And8 redeclared in this block
        previous declaration at ../../go/src/runtime/internal/atomic/atomic_amd64.go:63:27
../../go/src/runtime/internal/atomic/atomic_amd64x.go:48:6: too many errors
```

I'll still submit a PR, it'd be helpful if you could let me know if it fixes the issue.

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2d8ce280..ab79fd31 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -29,6 +29,9 @@
   syntax) are now printed to `tiny_debug_logs.txt` file in the log directory. If
   logging is disabled then the file is created at tiny's working directory.
   (#238)
+- Handling of IRC messages with ambiguous prefix (when it's unclear whether the
+  sender is a server or nick) improved. tiny should now work better with some
+  bouncers such as soju. (#249)
 
 # 2020/06/28: 0.6.0
 
diff --git a/libtiny_client/src/state.rs b/libtiny_client/src/state.rs
index 9dcf66d3..808d710f 100644
--- a/libtiny_client/src/state.rs
+++ b/libtiny_client/src/state.rs
@@ -277,35 +277,70 @@ impl StateInner {
 
         use wire::Cmd::*;
         match cmd {
+            // PING: Send PONG
             PING { server } => {
                 snd_irc_msg.try_send(wire::pong(server)).unwrap();
             }
 
-            //
-            // Setting usermask using JOIN, RPL_USERHOST and 396 (?)
-            // Also initialize the channel state on JOIN
-            //
+            // JOIN: If this is us then update usermask if possible, create the channel state. If
+            // someone else add the nick to channel.
             JOIN { chan } => {
-                if let Some(Pfx::User { nick, user }) = pfx {
-                    if nick == &self.current_nick {
+                match pfx {
+                    Some(Pfx::User { nick, user }) if nick == &self.current_nick => {
                         // Set usermask
                         let usermask = format!("{}!{}", nick, user);
                         self.usermask = Some(usermask);
+                    }
+                    _ => {}
+                }
+
+                match pfx {
+                    Some(Pfx::User { nick, .. }) | Some(Pfx::Ambiguous(nick)) => {
+                        if nick == &self.current_nick {
+                            // We joined a channel, initialize channel state
+                            match utils::find_idx(&self.chans, |c| &c.name == chan) {
+                                None => {
+                                    let mut chan = Chan::new(chan.to_owned());
+                                    // Since nick was found in the prefix, we are in the channel
+                                    chan.join_state = JoinState::Joined;
+                                    self.chans.push(chan);
+                                }
+                                Some(chan_idx) => {
+                                    // This happens because we initialize channel states for channels
+                                    // that we will join on connection when the client is first created
+                                    let chan = &mut self.chans[chan_idx];
+                                    chan.join_state = JoinState::Joined;
+                                    chan.nicks.clear();
+                                }
+                            }
+                        } else {
+                            match utils::find_idx(&self.chans, |c| &c.name == chan) {
+                                Some(chan_idx) => {
+                                    self.chans[chan_idx]
+                                        .nicks
+                                        .insert(wire::drop_nick_prefix(nick).to_owned());
+                                }
+                                None => {
+                                    debug!("Can't find channel state for JOIN: {:?}", cmd);
+                                }
+                            }
+                        }
+                    }
+                    Some(Pfx::Server(_)) | None => {}
+                }
+            }
 
-                        // Initialize channel state
+            // PART: If this is us remove the channel state. Otherwise remove the nick from the
+            // channel.
+            PART { chan, .. } => match pfx {
+                Some(Pfx::User { nick, .. }) | Some(Pfx::Ambiguous(nick)) => {
+                    if nick == &self.current_nick {
                         match utils::find_idx(&self.chans, |c| &c.name == chan) {
                             None => {
-                                let mut chan = Chan::new(chan.to_owned());
-                                // Since nick was found in the prefix, we are in the channel
-                                chan.join_state = JoinState::Joined;
-                                self.chans.push(chan);
+                                debug!("Can't find channel state: {}", chan);
                             }
                             Some(chan_idx) => {
-                                // This happens because we initialize channel states for channels
-                                // that we will join on connection when the client is first created
-                                let chan = &mut self.chans[chan_idx];
-                                chan.join_state = JoinState::Joined;
-                                chan.nicks.clear();
+                                self.chans.remove(chan_idx);
                             }
                         }
                     } else {
@@ -313,14 +348,42 @@ impl StateInner {
                             Some(chan_idx) => {
                                 self.chans[chan_idx]
                                     .nicks
-                                    .insert(wire::drop_nick_prefix(nick).to_owned());
+                                    .remove(wire::drop_nick_prefix(nick));
                             }
                             None => {
-                                debug!("Can't find channel state for JOIN: {:?}", cmd);
+                                debug!("Can't find channel state for PART: {:?}", cmd);
                             }
                         }
                     }
                 }
+                Some(Pfx::Server(_)) | None => {}
+            },
+
+            // QUIT: Update the `chans` field for the channels that the user was in
+            QUIT { ref mut chans, .. } => {
+                let nick = match pfx {
+                    Some(Pfx::User { nick, .. }) | Some(Pfx::Ambiguous(nick)) => nick,
+                    Some(Pfx::Server(_)) | None => {
+                        return;
+                    }
+                };
+                for chan in self.chans.iter_mut() {
+                    if chan.nicks.contains(nick) {
+                        chans.push(chan.name.to_owned());
+                        chan.nicks.remove(nick);
+                    }
+                }
+            }
+
+            // 396: Try to set usermask.
+            Reply { num: 396, params } => {
+                // :hobana.freenode.net 396 osa1 haskell/developer/osa1
+                // :is now your hidden host (set by services.)
+                if params.len() == 3 {
+                    let usermask =
+                        format!("{}!~{}@{}", self.current_nick, self.nicks[0], params[1]);
+                    self.usermask = Some(usermask);
+                }
             }
 
             // Reply 477 when user needs to be identified with NickServ to join a channel
@@ -390,16 +453,7 @@ impl StateInner {
                 }
             }
 
-            Reply { num: 396, params } => {
-                // :hobana.freenode.net 396 osa1 haskell/developer/osa1
-                // :is now your hidden host (set by services.)
-                if params.len() == 3 {
-                    let usermask =
-                        format!("{}!~{}@{}", self.current_nick, self.nicks[0], params[1]);
-                    self.usermask = Some(usermask);
-                }
-            }
-
+            // 302: Try to set usermask.
             Reply { num: 302, params } => {
                 // 302 RPL_USERHOST
                 // :ircd.stealth.net 302 yournick :syrk=+syrk@millennium.stealth.net
@@ -426,38 +480,7 @@ impl StateInner {
                 }
             }
 
-            //
-            // Remove channel state on PART
-            //
-            PART { chan, .. } => {
-                if let Some(Pfx::User { nick, .. }) = pfx {
-                    if nick == &self.current_nick {
-                        match utils::find_idx(&self.chans, |c| &c.name == chan) {
-                            None => {
-                                debug!("Can't find channel state: {}", chan);
-                            }
-                            Some(chan_idx) => {
-                                self.chans.remove(chan_idx);
-                            }
-                        }
-                    } else {
-                        match utils::find_idx(&self.chans, |c| &c.name == chan) {
-                            Some(chan_idx) => {
-                                self.chans[chan_idx]
-                                    .nicks
-                                    .remove(wire::drop_nick_prefix(nick));
-                            }
-                            None => {
-                                debug!("Can't find channel state for PART: {:?}", cmd);
-                            }
-                        }
-                    }
-                }
-            }
-
-            //
-            // RPL_WELCOME, start introduction sequence and NickServ authentication
-            //
+            // RPL_WELCOME: Start introduction sequence and NickServ authentication.
             Reply { num: 001, .. } => {
                 snd_ev.try_send(Event::Connected).unwrap();
                 snd_ev
@@ -471,9 +494,7 @@ impl StateInner {
                 }
             }
 
-            //
-            // RPL_YOURHOST, set servername
-            //
+            // RPL_YOURHOST: Set servername
             Reply { num: 002, params } => {
                 // 002    RPL_YOURHOST
                 //        "Your host is <servername>, running version <ver>"
@@ -490,11 +511,8 @@ impl StateInner {
                 }
             }
 
-            //
-            // ERR_NICKNAMEINUSE, try another nick if we don't have a nick yet
-            //
+            // ERR_NICKNAMEINUSE: Try another nick if we don't have a nick yet.
             Reply { num: 433, .. } => {
-                // ERR_NICKNAMEINUSE. If we don't have a nick already try next nick.
                 if !self.nick_accepted {
                     let new_nick = self.get_next_nick();
                     // debug!("new nick: {}", new_nick);
@@ -505,45 +523,44 @@ impl StateInner {
                 }
             }
 
-            //
             // NICK message sent from the server when our nick change request was successful
-            //
             NICK {
                 nick: new_nick,
                 ref mut chans,
             } => {
-                if let Some(Pfx::User { nick: old_nick, .. }) = pfx {
-                    if old_nick == &self.current_nick {
-                        snd_ev
-                            .try_send(Event::NickChange(new_nick.to_owned()))
-                            .unwrap();
-
-                        match utils::find_idx(&self.nicks, |nick| nick == new_nick) {
-                            None => {
-                                self.nicks.push(new_nick.to_owned());
-                                self.current_nick_idx = self.nicks.len() - 1;
-                            }
-                            Some(nick_idx) => {
-                                self.current_nick_idx = nick_idx;
+                match pfx {
+                    Some(Pfx::User { nick: old_nick, .. }) | Some(Pfx::Ambiguous(old_nick)) => {
+                        if old_nick == &self.current_nick {
+                            snd_ev
+                                .try_send(Event::NickChange(new_nick.to_owned()))
+                                .unwrap();
+
+                            match utils::find_idx(&self.nicks, |nick| nick == new_nick) {
+                                None => {
+                                    self.nicks.push(new_nick.to_owned());
+                                    self.current_nick_idx = self.nicks.len() - 1;
+                                }
+                                Some(nick_idx) => {
+                                    self.current_nick_idx = nick_idx;
+                                }
                             }
-                        }
 
-                        self.current_nick = new_nick.to_owned();
-                    }
+                            self.current_nick = new_nick.to_owned();
+                        }
 
-                    // Rename the nick in channel states, also populate the chan list
-                    for chan in &mut self.chans {
-                        if chan.nicks.remove(old_nick) {
-                            chan.nicks.insert(new_nick.to_owned());
-                            chans.push(chan.name.to_owned());
+                        // Rename the nick in channel states, also populate the chan list
+                        for chan in &mut self.chans {
+                            if chan.nicks.remove(old_nick) {
+                                chan.nicks.insert(new_nick.to_owned());
+                                chans.push(chan.name.to_owned());
+                            }
                         }
                     }
+                    Some(Pfx::Server(_)) | None => {}
                 }
             }
 
-            //
-            // RPL_ENDOFMOTD, join channels, set away status
-            //
+            // RPL_ENDOFMOTD: Join channels, set away status
             Reply { num: 376, .. } => {
                 let chans: Vec<&str> = self.chans.iter().map(|c| c.name.as_str()).collect();
                 if !chans.is_empty() {
@@ -556,9 +573,7 @@ impl StateInner {
                 }
             }
 
-            //
-            // RPL_NAMREPLY: users in a channel
-            //
+            // RPL_NAMREPLY: Set users in a channel
             Reply { num: 353, params } => {
                 let chan = &params[2];
                 match utils::find_idx(&self.chans, |c| &c.name == chan) {
@@ -578,28 +593,7 @@ impl StateInner {
                 }
             }
 
-            //
-            // QUIT: Update the `chans` field for the channels that the user was in
-            //
-            QUIT { ref mut chans, .. } => {
-                let nick = match pfx {
-                    Some(Pfx::User { nick, .. }) => nick,
-                    _ => {
-                        warn!("Could not extract nick from QUIT message.");
-                        return;
-                    }
-                };
-                for chan in self.chans.iter_mut() {
-                    if chan.nicks.contains(nick) {
-                        chans.push(chan.name.to_owned());
-                        chan.nicks.remove(nick);
-                    }
-                }
-            }
-
-            //
             // SASL authentication
-            //
             CAP {
                 client: _,
                 subcommand,
@@ -646,9 +640,7 @@ impl StateInner {
                 snd_irc_msg.try_send(wire::cap_end()).unwrap();
             }
 
-            //
             // Ignore the rest
-            //
             _ => {}
         }
     }
@@ -723,6 +715,12 @@ async fn retry_channel_join(
 const SERVERNAME_PREFIX: &str = "Your host is ";
 const SERVERNAME_PREFIX_LEN: usize = SERVERNAME_PREFIX.len();
 
+/// Parse server name from RPL_YOURHOST reply or fallback to using the server name inside
+/// Pfx::Server. See https://www.irc.com/dev/docs/refs/numerics/002.html for more info.
+fn parse_servername(pfx: Option<&Pfx>, params: &[String]) -> Option<String> {
+    parse_yourhost_msg(&params).or_else(|| parse_server_pfx(pfx))
+}
+
 /// Try to parse servername in a 002 RPL_YOURHOST reply params.
 fn parse_yourhost_msg(params: &[String]) -> Option<String> {
     let msg = params.get(1).or_else(|| params.get(0))?;
@@ -737,19 +735,14 @@ fn parse_yourhost_msg(params: &[String]) -> Option<String> {
 
 /// Get the server name from a prefix.
 fn parse_server_pfx(pfx: Option<&Pfx>) -> Option<String> {
-    if let Some(Pfx::Server(server_name)) = pfx {
-        Some(server_name.to_owned())
-    } else {
-        None
+    match pfx {
+        Some(Pfx::Server(server_name)) | Some(Pfx::Ambiguous(server_name)) => {
+            Some(server_name.to_owned())
+        }
+        Some(Pfx::User { .. }) | None => None,
     }
 }
 
-/// Parse server name from RPL_YOURHOST reply or fallback to using the server name inside
-/// Pfx::Server. See https://www.irc.com/dev/docs/refs/numerics/002.html for more info.
-fn parse_servername(pfx: Option<&Pfx>, params: &[String]) -> Option<String> {
-    parse_yourhost_msg(&params).or_else(|| parse_server_pfx(pfx))
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 
 #[cfg(test)]
diff --git a/libtiny_wire/src/lib.rs b/libtiny_wire/src/lib.rs
index 0288ef54..6b527364 100644
--- a/libtiny_wire/src/lib.rs
+++ b/libtiny_wire/src/lib.rs
@@ -77,24 +77,89 @@ pub fn authenticate(msg: &str) -> String {
     format!("AUTHENTICATE {}\r\n", msg)
 }
 
-/// Sender of a message
+/// Sender of a message ("prefix" in the RFC). Instead of returning a `String` we parse prefix part
+/// of the message according to the RFC because users of this library sometimes need to distinguish
+/// a server from a user. For example, in tiny if a PRIVMSG to us is coming from a server then we
+/// show it in the server tab. Otherwise we show it in the sender's (user) tab.
 ///
-/// `<prefix> ::= <servername> | <nick> [ '!' <user> ] [ '@' <host> ]`
+/// (Note that the ambiguity in the RFC makes this a best-effort thing. When we get a PRIVMSG from
+/// e.g. "foo" it's not possible to know whether "foo" is a server or a user.)
 ///
-/// From RFC 2812:
-///
-/// > If the prefix is missing from the message, it is assumed to have originated from the
-/// > connection from which it was received from.
+/// One alternative here would be to defer parsing to the users so that, for example, if in the
+/// context we expect the message to be coming from a user we call `Pfx::parse_user()` which
+/// interprets the ambiguous case as "user" and `Pfx::parse_server()` which interprets it as
+/// "server". The downside is that'd sometimes means parsing the prefix multiple times. For
+/// example, in tiny, a Client would parse the prefix to get the nick to update the channel state,
+/// then we'd parse it again in tiny to update the TUI.
+
+// We could still provide `get_server()` and `get_nick()` that interpret the ambiguous case as
+// server and nick, respectively, but I don't think it'd be much more convenient that pattern
+// matching explicitly. See the commented-out code below.
+
 #[derive(Debug, PartialEq, Eq, Clone)]
 pub enum Pfx {
+    /// Sender is a server.
     Server(String),
 
-    /// <nick>!<user>@<host>
+    /// Sender is a nick.
     User {
+        /// Nick of the sender
         nick: String,
-        /// user@host
+        /// `user@host` part
         user: String,
     },
+
+    /// Server could be a server or nick, it's unclear. According to the RFC if we have something
+    /// like "localhost" which doesn't have '!', '@', or a character that 'servername' can have but
+    /// 'nickname' cannot, we can't tell whether the sender is a server or a nick. In those cases
+    /// we return this variant. See also #247.
+    Ambiguous(String),
+}
+
+/*
+impl Pfx {
+    /// Get the server name if the prefix is for a server. Ambiguous case is interpreted as server.
+    pub fn get_server(&self) -> Option<&str> {
+        match self {
+            Pfx::Server(ref server) | Pfx::Ambiguous(ref server) => Some(server),
+            Pfx::User { .. } => None,
+        }
+    }
+
+    /// Get the nick if the prefix is for a user. Ambiguous case is interpreted as a nick.
+    pub fn get_nick(&self) -> Option<&str> {
+        match self {
+            Pfx::User { ref nick, .. } | Pfx::Ambiguous(ref nick) => Some(nick),
+            Pfx::Server(_) => None,
+        }
+    }
+}
+*/
+
+// RFC 2812 section 2.3.1
+fn parse_pfx(pfx: &str) -> Pfx {
+    match pfx.find(&['!', '@'][..]) {
+        Some(idx) => Pfx::User {
+            nick: (&pfx[0..idx]).to_owned(),
+            user: (&pfx[idx + 1..]).to_owned(),
+        },
+        None => {
+            // Chars that nicks can have but servernames cannot
+            match pfx.find(&['[', ']', '\\', '`', '_', '^', '{', '|', '}'][..]) {
+                Some(_) => Pfx::User {
+                    nick: pfx.to_owned(),
+                    user: "".to_owned(),
+                },
+                None => {
+                    // Nicks can't have '.'
+                    match pfx.find('.') {
+                        Some(_) => Pfx::Server(pfx.to_owned()),
+                        None => Pfx::Ambiguous(pfx.to_owned()),
+                    }
+                }
+            }
+        }
+    }
 }
 
 /// Target of a message
@@ -107,6 +172,10 @@ pub enum MsgTarget {
 /// An IRC message
 #[derive(Debug, PartialEq, Eq)]
 pub struct Msg {
+    /// Sender of a message. According to RFC 2812 it's optional:
+    ///
+    /// > If the prefix is missing from the message, it is assumed to have originated from the
+    /// > connection from which it was received from.
     pub pfx: Option<Pfx>,
     pub cmd: Cmd,
 }
@@ -375,16 +444,6 @@ fn parse_one_message(mut msg: &str) -> Result<Msg, String> {
     Ok(Msg { pfx, cmd })
 }
 
-fn parse_pfx(pfx: &str) -> Pfx {
-    match pfx.find('!') {
-        None => Pfx::Server(pfx.to_owned()),
-        Some(idx) => Pfx::User {
-            nick: (&pfx[0..idx]).to_owned(),
-            user: (&pfx[idx + 1..]).to_owned(),
-        },
-    }
-}
-
 fn parse_params(chrs: &str) -> Vec<&str> {
     debug_assert!(!chrs.starts_with(' '));
 
@@ -514,7 +573,11 @@ mod tests {
         .unwrap();
 
         let mut msgs = vec![];
-        while let Some(msg) = parse_irc_msg(&mut buf) {
+        while let Some(Ok(msg)) = parse_irc_msg(&mut buf) {
+            assert_eq!(
+                msg.pfx,
+                Some(Pfx::Server("barjavel.freenode.net".to_owned()))
+            );
             msgs.push(msg);
         }
 
@@ -711,4 +774,37 @@ mod tests {
             },
         );
     }
+
+    #[test]
+    fn test_parse_pfx() {
+        use Pfx::*;
+        assert_eq!(parse_pfx("xyz"), Ambiguous("xyz".to_string()));
+        assert_eq!(parse_pfx("xy-z"), Ambiguous("xy-z".to_string()),);
+        assert_eq!(parse_pfx("xy.z"), Server("xy.z".to_string()));
+        assert_eq!(
+            parse_pfx("xyz[m]"),
+            User {
+                nick: "xyz[m]".to_string(),
+                user: "".to_string()
+            }
+        );
+        assert_eq!(
+            parse_pfx("fe-00106.xyz.net"),
+            Server("fe-00106.xyz.net".to_string())
+        );
+        assert_eq!(
+            parse_pfx("osa1!osa1@x.y.im"),
+            User {
+                nick: "osa1".to_string(),
+                user: "osa1@x.y.im".to_string(),
+            }
+        );
+        assert_eq!(
+            parse_pfx("IRC!IRC@fe-00106.xyz.net"),
+            User {
+                nick: "IRC".to_string(),
+                user: "IRC@fe-00106.xyz.net".to_string()
+            }
+        );
+    }
 }
diff --git a/tiny/src/conn.rs b/tiny/src/conn.rs
index c9b4caa0..b4d6d3d8 100644
--- a/tiny/src/conn.rs
+++ b/tiny/src/conn.rs
@@ -138,25 +138,25 @@ fn handle_irc_msg(ui: &dyn UI, client: &Client, msg: wire::Msg) {
             let pfx = match pfx {
                 Some(pfx) => pfx,
                 None => {
-                    // TODO: log this?
+                    debug!("PRIVMSG without prefix: {:?}", msg);
                     return;
                 }
             };
 
-            // sender to be shown in the UI
-            let origin = match pfx {
-                Server(_) => serv,
-                User { ref nick, .. } => nick,
+            // Sender to be shown in the UI
+            let sender = match pfx {
+                Server(ref serv) => serv,
+                User { ref nick, .. } | Ambiguous(ref nick) => nick,
             };
 
             if ctcp == Some(wire::CTCP::Version) {
-                let msg_target = if ui.user_tab_exists(serv, origin) {
-                    MsgTarget::User { serv, nick: origin }
+                let msg_target = if ui.user_tab_exists(serv, sender) {
+                    MsgTarget::User { serv, nick: sender }
                 } else {
                     MsgTarget::Server { serv }
                 };
                 ui.add_client_msg(
-                    &format!("Received version request from {}", origin),
+                    &format!("Received version request from {}", sender),
                     &msg_target,
                 );
                 return;
@@ -169,27 +169,31 @@ fn handle_irc_msg(ui: &dyn UI, client: &Client, msg: wire::Msg) {
                     let ui_msg_target = MsgTarget::Chan { serv, chan: &chan };
                     // highlight the message if it mentions us
                     if msg.find(&client.get_nick()).is_some() {
-                        ui.add_privmsg(origin, &msg, ts, &ui_msg_target, true, is_action);
+                        ui.add_privmsg(sender, &msg, ts, &ui_msg_target, true, is_action);
                         ui.set_tab_style(TabStyle::Highlight, &ui_msg_target);
                         let mentions_target = MsgTarget::Server { serv: "mentions" };
                         ui.add_msg(
-                            &format!("{} in {}:{}: {}", origin, serv, chan, msg),
+                            &format!("{} in {}:{}: {}", sender, serv, chan, msg),
                             ts,
                             &mentions_target,
                         );
                         ui.set_tab_style(TabStyle::Highlight, &mentions_target);
                     } else {
-                        ui.add_privmsg(origin, &msg, ts, &ui_msg_target, false, is_action);
+                        ui.add_privmsg(sender, &msg, ts, &ui_msg_target, false, is_action);
                         ui.set_tab_style(TabStyle::NewMsg, &ui_msg_target);
                     }
                 }
                 wire::MsgTarget::User(target) => {
+                    // Message is sent to the user. If the sender is a server we show the message
+                    // in the server tab, otherwise we show it in a private tab. In case of an
+                    // ambiguity we again show in a private tab, as some bouncers send PRIVMSGs
+                    // from users with ambiguous prefix without a `user@host` part. See #247.
                     let msg_target = {
                         match pfx {
                             Server(_) => MsgTarget::Server { serv },
-                            User { ref nick, .. } => {
-                                // show NOTICE messages in server tabs if we don't have a tab
-                                // for the sender already (see #21)
+                            User { ref nick, .. } | Ambiguous(ref nick) => {
+                                // Show NOTICE messages in server tabs if we don't have a tab for
+                                // the sender already (see #21)
                                 if is_notice && !ui.user_tab_exists(serv, nick) {
                                     MsgTarget::Server { serv }
                                 } else {
@@ -198,11 +202,11 @@ fn handle_irc_msg(ui: &dyn UI, client: &Client, msg: wire::Msg) {
                             }
                         }
                     };
-                    ui.add_privmsg(origin, &msg, ts, &msg_target, false, is_action);
+                    ui.add_privmsg(sender, &msg, ts, &msg_target, false, is_action);
                     if target == client.get_nick() {
                         ui.set_tab_style(TabStyle::Highlight, &msg_target);
                     } else {
-                        // not sure if this case can happen
+                        // Not sure if this case can happen
                         ui.set_tab_style(TabStyle::NewMsg, &msg_target);
                     }
                 }
@@ -211,9 +215,13 @@ fn handle_irc_msg(ui: &dyn UI, client: &Client, msg: wire::Msg) {
 
         JOIN { chan } => {
             let nick = match pfx {
-                Some(User { nick, .. }) => nick,
-                _ => {
-                    // TODO: log this?
+                Some(User { nick, .. }) | Some(Ambiguous(nick)) => nick,
+                Some(Server(_)) | None => {
+                    debug!(
+                        "JOIN with weird prefix: pfx={:?}, cmd={:?}",
+                        pfx,
+                        JOIN { chan }
+                    );
                     return;
                 }
             };
@@ -232,11 +240,15 @@ fn handle_irc_msg(ui: &dyn UI, client: &Client, msg: wire::Msg) {
             }
         }
 
-        PART { chan, .. } => {
+        PART { chan, msg } => {
             let nick = match pfx {
-                Some(User { nick, .. }) => nick,
-                _ => {
-                    // TODO: log this?
+                Some(User { nick, .. }) | Some(Ambiguous(nick)) => nick,
+                Some(Server(_)) | None => {
+                    debug!(
+                        "PART with weird prefix: pfx={:?}, cmd={:?}",
+                        pfx,
+                        PART { chan, msg }
+                    );
                     return;
                 }
             };
@@ -249,11 +261,15 @@ fn handle_irc_msg(ui: &dyn UI, client: &Client, msg: wire::Msg) {
             }
         }
 
-        QUIT { chans, .. } => {
+        QUIT { chans, msg } => {
             let nick = match pfx {
-                Some(User { ref nick, .. }) => nick,
-                _ => {
-                    // TODO: log this?
+                Some(User { ref nick, .. }) | Some(Ambiguous(ref nick)) => nick,
+                Some(Server(_)) | None => {
+                    debug!(
+                        "QUIT with weird prefix: pfx={:?}, cmd={:?}",
+                        pfx,
+                        QUIT { chans, msg }
+                    );
                     return;
                 }
             };
@@ -268,9 +284,13 @@ fn handle_irc_msg(ui: &dyn UI, client: &Client, msg: wire::Msg) {
 
         NICK { nick, chans } => {
             let old_nick = match pfx {
-                Some(User { nick, .. }) => nick,
-                _ => {
-                    // TODO: log this?
+                Some(User { nick, .. }) | Some(Ambiguous(nick)) => nick,
+                Some(Server(_)) | None => {
+                    debug!(
+                        "NICK with weird prefix: pfx={:?}, cmd={:?}",
+                        pfx,
+                        NICK { nick, chans }
+                    );
                     return;
                 }
             };
@@ -420,7 +440,7 @@ fn handle_irc_msg(ui: &dyn UI, client: &Client, msg: wire::Msg) {
                 );
             } else {
                 match pfx {
-                    Some(Server(msg_serv)) => {
+                    Some(Server(msg_serv)) | Some(Ambiguous(msg_serv)) => {
                         let msg_target = MsgTarget::Server { serv };
                         ui.add_privmsg(
                             &msg_serv,
@@ -432,7 +452,7 @@ fn handle_irc_msg(ui: &dyn UI, client: &Client, msg: wire::Msg) {
                         );
                         ui.set_tab_style(TabStyle::NewMsg, &msg_target);
                     }
-                    pfx => {
+                    Some(User { .. }) | None => {
                         debug!(
                             "Ignoring numeric reply {}: pfx={:?}, params={:?}",
                             n, pfx, params
@@ -455,9 +475,9 @@ fn handle_irc_msg(ui: &dyn UI, client: &Client, msg: wire::Msg) {
                 );
                 ui.set_tab_style(TabStyle::NewMsg, &msg_target);
             }
-            pfx => {
+            Some(User { .. }) | Some(Ambiguous(_)) | None => {
                 debug!(
-                    "Ignoring server command {}: pfx={:?}, params={:?}",
+                    "Ignoring command {}: pfx={:?}, params={:?}",
                     cmd, pfx, params
                 );
             }


Test patch: