Fix catching invalid nick change, closes #628

Changes

@@ -1,5 +1,8 @@
 Revision history for perl distribution Convos
 
+6.48 Not Released
+ - Fix catching invalid nick change #628
+
 6.47 2021-12-28T12:32:00+0900
  - Add more rules for potentially evil svg files
    Contributor: KhanhCM

lib/Convos/Core/Connection/Irc.pm

@@ -171,15 +171,18 @@ sub _irc_event_err_cannotsendtochan {
 
 sub _irc_event_err_erroneusnickname {
   my ($self, $msg) = @_;
+  return if $msg->{handled};
+
   my $nick = $msg->{params}[1] || 'unknown';
   $self->_message("Invalid nickname $nick.", type => 'error');
 }
 
 sub _irc_event_err_nicknameinuse {
   my ($self, $msg) = @_;
-  my $nick = $msg->{params}[1];
+  return if $msg->{handled};
 
   # do not want to flod frontend with these messages
+  my $nick = $msg->{params}[1];
   $self->_message("Nickname $nick is already in use.", type => 'error')
     unless $self->{err_nicknameinuse}{$nick}++;
 
@@ -299,7 +302,8 @@ sub _irc_event_nick {
     delete $self->{err_nicknameinuse};    # allow warning on next nick change
   }
 
-  if ($self->info->{nick} eq $old_nick) {
+  if ($self->nick eq $old_nick) {
+    $self->url->query->param(nick => $new_nick);
     $self->info->{nick} = $new_nick;
     $self->emit(state => info => $self->info);
   }
@@ -637,6 +641,13 @@ sub _make_names_response {
     if $msg->{command} eq 'rpl_namreply';
 }
 
+sub _make_nick_response {
+  my ($self, $msg, $res, $p) = @_;
+  return $p->reject(sprintf '%s: %s', $msg->{params}[-1], $res->{nick})
+    if $msg->{command} =~ m!^err_!;
+  return $p->resolve($res);
+}
+
 sub _make_oper_response {
   my ($self, $msg, $res, $p) = @_;
   return $p->reject($msg->{params}[-1]) if $msg->{command} =~ m!^err_!;
@@ -1024,12 +1035,15 @@ sub _send_names_p {
 sub _send_nick_p {
   my ($self, $nick) = @_;
   return Mojo::Promise->reject('Missing or invalid nick.') unless $nick;
-
-  $self->info->{nick} = $nick;
-  $self->url->query->param(nick => $nick);
-  $self->emit(state => info => $self->info);
-  return $self->_write_p("NICK $nick\r\n") if $self->{stream};
-  return Mojo::Promise->resolve({});
+  return Mojo::Promise->resolve({nick => $nick}) if $self->nick eq $nick;
+  return $self->_write_and_wait_p(
+    "NICK $nick", {nick => $nick},
+    err_erroneusnickname => {1 => $nick},
+    err_nickcollision    => {1 => $nick},
+    err_nicknameinuse    => {1 => $nick},
+    nick                 => {0 => $nick},
+    '_make_nick_response',
+  );
 }
 
 sub _send_oper_p {
@@ -1183,7 +1197,6 @@ CHUNK:
 
     # @wait_for is to avoid "Use of freed value in iteration"
     my @wait_for = values %{$self->{wait_for}{$msg->{command}} || {}};
-    my $handled  = 0;
 
   WAIT_FOR:
     for (@wait_for) {
@@ -1196,14 +1209,14 @@ CHUNK:
 
       $self->_debug('->%s(...)', $make_response_method) if DEBUG;
       $self->$make_response_method($msg, $res, $p);
-      $handled++;
+      $msg->{handled}++;
     }
 
     if (my $cb = $self->can($method)) {
       $self->_debug('->%s(...)', $method) if DEBUG;
       $self->$cb($msg);
     }
-    elsif (!$handled) {
+    elsif (!$msg->{handled}) {
       $self->_debug('->%s(...) (fallback)', $method) if DEBUG;
       $self->_irc_event_fallback($msg);
     }

t/irc-commands.t

@@ -52,6 +52,7 @@ $server->subtest(
       ['#convos',   '/close'],                          # CLOSE
       ['#convos',   '/close superwoman'],               # CLOSE
       ['#convos',   '/names'],                          # NAMES
+      ['#convos',   '/nick foo'],                       # NICK
       ['#convos',   '/topic'],                          # TOPIC get
       ['#convos',   '/topic New topic'],                # TOPIC set
       ['whatever',  '/ison'],                           # ISON
@@ -67,9 +68,6 @@ $server->subtest(
 
     is $connection->get_conversation('#convos')->password, 's3cret', 'password is set';
 
-    $connection->send_p('', '/nick superduper')->$wait_success('nick');
-    is $connection->url->query->param('nick'), 'superduper', 'change nick offline';
-
     ok !$connection->get_conversation('superwoman'), 'superwoman does not exist';
     $connection->send_p('', '/query superwoman ')->$wait_success('query');
     ok $connection->get_conversation('superwoman'), 'superwoman exist';
@@ -206,6 +204,29 @@ $server->subtest(
   }
 );
 
+$server->subtest(
+  'nick' => sub {
+    $connection->send_p('#convos', '/nick')->$wait_reject('Missing or invalid nick.');
+
+    $res = $connection->send_p('#convos', '/nick superman')->$wait_success;
+    is $res->{nick}, 'superman', 'no change';
+
+    $server->server_event_ok('_irc_event_nick')->server_write_ok(['nick-erroneusnickname.irc']);
+    $connection->send_p('#convos', '/nick foo!')->$wait_reject('Erroneous nickname: foo!');
+    $server->processed_ok;
+
+    $server->server_event_ok('_irc_event_nick')->server_write_ok(['nick-superduper.irc']);
+    $res = $connection->send_p('#convos', '/nick superduper')->$wait_success;
+    $server->processed_ok;
+    is_deeply $res, {nick => 'superduper'}, 'nick response';
+
+    note 'change back';
+    $server->server_event_ok('_irc_event_nick')->server_write_ok(['nick-superman.irc']);
+    $res = $connection->send_p('#convos', '/nick superman')->$wait_success;
+    $server->processed_ok;
+  },
+);
+
 $server->subtest(
   'topic' => sub {
     $server->server_event_ok('_irc_event_topic')
@@ -467,6 +488,12 @@ __DATA__
 :localhost 353 superman = #convos :superwoman ~superman &robin
 :localhost 353 superman = #convos :%batboy @superboy +robyn
 :localhost 366 superman #convos :End of /NAMES list.
+@@ nick-superduper.irc
+:superman!whatever NICK superduper
+@@ nick-superman.irc
+:superman!whatever NICK superman
+@@ nick-erroneusnickname.irc
+:lcoalhost 432 superman foo! :Erroneous nickname
 @@ oper.irc
 :localhost 381 superman :You are now an IRC operator
 @@ whois-superwoman.irc