cinchrb / cinch

The IRC Bot Building Framework
http://www.rubydoc.info/gems/cinch
MIT License
1k stars 180 forks source link

Cinch doesn't work with server enable Client Capabilities Extension (CAP) #128

Closed chinkung closed 10 years ago

chinkung commented 11 years ago

When I use Cinch to connection to server that enable CAP LS, it will timeout, I have tried to disable it and it's working again but I think this should be fixed, below is the log

[2013/07/18 14:09:07.797] II Connecting to irc.xxx.com:6667 [2013/07/18 14:09:07.807] >> :irc.xxx.com NOTICE Auth :* Looking up your hostname... [2013/07/18 14:09:07.810] << CAP LS [2013/07/18 14:09:07.811] << NICK cinch [2013/07/18 14:09:07.811] >> :irc.xxx.com NOTICE Auth :* Could not resolve your hostname: Domain name not found; using your IP address (xx.xx.xx.xx) instead. [2013/07/18 14:09:07.812] >> :irc.xxx.com CAP 242AAAA80 LS :tls [2013/07/18 14:09:07.813] << USER cinch 0 * :cinch [2013/07/18 14:09:07.813] << CAP REQ : [2013/07/18 14:09:18.892] >> ERROR :Closing link: (cinch@xx.xx.xx.xx) [Registration timeout] [2013/07/18 14:09:18.893] !! Lost connection.

dominikh commented 11 years ago

Cinch works very well with servers that use CAP, such as Freenode. The server you're connecting to is probably not implementing it correctly. If you tell me the server address I'll take a look myself.

alown commented 10 years ago

I am seeing this problem negotiating with ldilley/rubircd@9aade7615bca578c4e56ffceed2574d7c35fa8d6. My understanding of the spec (http://ircv3.atheme.org/specification/capability-negotiation-3.1) suggests that Cinch is at fault here for not sending a CAP END before continuing registration: "If a client issues an LS subcommand, registration must be suspended until an END subcommand is received"

The implementation also seems to strongly suggest this:

if user.nick != "*" && user.ident != nil && user.gecos != nil && user.is_negotiating_cap
   Network.send(user, Numeric.ERR_NOTREGISTERED("CAP")) # user has not closed CAP with END
end
dominikh commented 10 years ago

Can you suggest a scenario in which we do not send a CAP END? In the original example in this issue, the server was at fault for not ACKing the REQ. Unless there's an unhandled path in Cinch, we send CAP END on all occassions that we have to.

To outline the flow here: We send LS, we send a REQ (which might be empty), we wait for an (N)ACK, we check if we need to set up SASL or not. If we do not use SASL, we send an END. If we do use SASL, we authenticate, and on successful authentication, we send END.

Edit: For as long as authentication fails and we have more methods to try, we try them. Otherwise we give up and send an END.

alown commented 10 years ago

Here is a log using connecting to the rubircd server which is showing this problem.

[2013/09/02 21:13:22.854] II Connecting to alipc-server-ex:1997
[2013/09/02 21:13:22.858] << CAP LS 
[2013/09/02 21:13:22.858] >> :alipc-server-ex.internal.eezysys.co.uk NOTICE Auth :*** Looking up your hostname...
[2013/09/02 21:13:22.859] << PASS mypassword 
[2013/09/02 21:13:22.860] >> :alipc-server-ex.internal.eezysys.co.uk NOTICE Auth :*** Found your hostname (alipc-server-ex)
[2013/09/02 21:13:22.860] << NICK mailbot 
[2013/09/02 21:13:22.860] >> :alipc-server-ex.internal.eezysys.co.uk CAP * LS :
[2013/09/02 21:13:22.861] << USER mailbot 0 * :cinch
[2013/09/02 21:13:22.861] << CAP REQ :
[2013/09/02 21:13:22.899] >> :alipc-server-ex.internal.eezysys.co.uk 451 CAP :Register first.
[2013/09/02 21:13:22.939] >> :alipc-server-ex.internal.eezysys.co.uk CAP mailbot NAK: 
[2013/09/02 21:13:22.939] >> :alipc-server-ex.internal.eezysys.co.uk 451 CAP :Register first.

This server eventually time-outs this connection attempt, because it never got an END.

The server does send a NAK (You spelled it NACK <- this isn't the error is it?) but Cinch never sends an END here, so rubircd refuses to acknowledge this as a valid registration.

dominikh commented 10 years ago

Well, there are two issues here, one of which is a bug in Cinch. We do indeed look for NACK instead of NAK. The second issue would be that we only do client capabilities before the registration process finishes. We won't attempt anything after the registration is over.

dominikh commented 10 years ago

As a matter of fact, I'm not even sure that sending a 451 is acceptable here; but that's a different issue.

alown commented 10 years ago

1) Ok. If you fix that bug, that would be great.

2) This doesn't sound like an actual problem. CAP'ing during the registration process is perfectly valid as long as a END is called before the server times-out the registration, since the server is not permitted to continue until it has this.

(At no point did I say that rubircd is fully compliant, I merely raised it as a server you could test against, which showed the issue in this bug report)

dominikh commented 10 years ago

Actually, unless my eyes are failing me, they're sending a "NAK:", note the final colon and lacking space before it. So even if I fix this bug (which I will), there's still a bug in that IRCd you'll want to file.

alown commented 10 years ago

Yes, you are correct that that is a bug (checking the spec). I will report it there.

Network.send(user, ":#{Options.server_name} CAP #{user.nick} NAK: #{args[1..-1].join(" ")}")
dominikh commented 10 years ago

Closing this issue. The original bug report is based on a server that's not behaving correctly, version 2.0.8 fixes the NAK bug. If there are any issues remaining, open new issues.