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

handle an already used nick different from the one we send #219

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@erdnaxeli

erdnaxeli commented Feb 10, 2015

This commit fixes the following bug.

If we set a nick with a space, like "abc d", the IRC server only understands the first part, here "abc". Now imagine the case where this nick ("abc") is already used. The server notifies Irssi, but Irssi think the nick is "abc d" so it tries "abc d_". The server only understands the "abc" part, and so on, so the connection stays stuck.

This commit fixes that by checking when receiving a "nick already used" message if the nick sent by the server is the same as the one we have registered. If not, we use the nick from the server, and the classic process (with underscores and numbers) continues.

@dequis

This comment has been minimized.

Member

dequis commented Feb 10, 2015

If we set a nick with a space, like "abc d"

I think you're fixing the wrong issue here...

@erdnaxeli

This comment has been minimized.

erdnaxeli commented Feb 10, 2015

Can you be more specific?

On irc someone says to me that some protocols may accept a nick with a space, so we cannot reject it when it is set.

@dequis

This comment has been minimized.

Member

dequis commented Feb 11, 2015

IRC certainly doesn't, and this sounds like an IRC specific problem to me. Sure, it might not be correct to reject it in the UI side, but the irc protocol side shouldn't be attempting to send an invalid nick

@erdnaxeli

This comment has been minimized.

erdnaxeli commented Feb 11, 2015

Ok. So maybe we can handle this in server_init() in irc/core/irc-servers.c?

@ailin-nemui

This comment has been minimized.

Contributor

ailin-nemui commented Sep 22, 2015

might be viable, what do you think @dequis

@ailin-nemui ailin-nemui added the bug label Nov 1, 2015

@ailin-nemui ailin-nemui modified the milestone: 0.8.19 Nov 1, 2015

@ailin-nemui

This comment has been minimized.

Contributor

ailin-nemui commented Nov 9, 2015

I kinda like this idea, could we have some comment @ahf

@ailin-nemui

This comment has been minimized.

Contributor

ailin-nemui commented May 18, 2016

@erdnaxeli I would like to fix this issue, I see that there is the following issue: if you set the alternate nick to an invalid (space-containing) nick, then the code could -- based on the server reply -- re-set the nick to the alternate nick and again be trapped in a circle. do you agree?

@erdnaxeli

This comment has been minimized.

erdnaxeli commented May 26, 2016

If the alternate nick contains a space too, irssi (with my commit) will still figure out that the nick send back by the server is different, and then the process with underscore will start.

Tested and validated.

@erdnaxeli

This comment has been minimized.

erdnaxeli commented May 26, 2016

I've not spend any time on this issue till our discussion in September. Is the idea of handling this case in server_init() in irc/core/irc-servers.c still the right solution? We could check both nick and alternate_nick here for spaces.

I can take time this week-end to work on it.

@erdnaxeli

This comment has been minimized.

erdnaxeli commented Jun 5, 2016

Ok, I tried to set conn->nick to a correct nick without space (I just takes the substring before the space), and a valid nick is sent. But then, if this nick is already taken, the alternate nick process tries with the wrong nick (containing space). It doesn't use the version set on server_init().

@ailin-nemui ailin-nemui modified the milestones: 0.8.21, 1.1.1 Jan 3, 2017

@ailin-nemui

This comment has been minimized.

Contributor

ailin-nemui commented Sep 11, 2017

hi @erdnaxeli , my original analysis was flawed:

if the nick (as per /set nick) contains spaces or otherwise the server uses a different nick, then the alt_nick is not tried by your code:

the comparison whether nick EQ server->nick AND nick NOT EQ alt_nick fails (because the server->nick was corrected)

we just need to move the comparison before the correction

@irssi irssi deleted a comment Nov 25, 2017

@irssi irssi deleted a comment from dequis Nov 25, 2017

@ailin-nemui ailin-nemui added WIP and removed bug needs changes labels Jan 6, 2018

@ailin-nemui ailin-nemui removed this from the 1.1.1 milestone Jan 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment