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

Allow capabilities to be handled after upstream IRC registration #1735

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

kylef
Copy link
Member

@kylef kylef commented Jul 5, 2020

This is perhaps the first part in being able to support cap-notify and capability 3.2 by extending the capability handling to occur after registration. I would like to add some automated testing, having some problems getting those passing (debugging setup with qt/python etc) so some delays with that.

The first change removes the m_bAuthed check, and then the second change makes it so that subsequent CAP LS won't trigger re-requesting already negotiated capabilities, otherwise the following would occur:


ZNC *raw CAP LS
:*raw!znc@znc.in PRIVMSG kyle :YOU -> [ZNC *raw CAP LS]
:*raw!znc@znc.in PRIVMSG kyle :IRC -> [@time=2020-07-05T11:39:07.812Z :irc-eu-2.darkscience.net CAP kyle LS :account-notify account-tag away-notify batch cap-notify chghost echo-message extended-join inspircd.org/poison inspircd.org/standard-replies invite-notify labeled-response multi-prefix sasl server-time setname userhost-in-names ]
:*raw!znc@znc.in PRIVMSG kyle :IRC -> [@time=2020-07-05T11:39:07.822Z :irc-eu-2.darkscience.net CAP kyle ACK :account-notify]
:*raw!znc@znc.in PRIVMSG kyle :IRC -> [@time=2020-07-05T11:39:07.831Z :irc-eu-2.darkscience.net CAP kyle ACK :away-notify]
:*raw!znc@znc.in PRIVMSG kyle :IRC -> [@time=2020-07-05T11:39:07.840Z :irc-eu-2.darkscience.net CAP kyle ACK :extended-join]
:*raw!znc@znc.in PRIVMSG kyle :IRC -> [@time=2020-07-05T11:39:07.848Z :irc-eu-2.darkscience.net CAP kyle ACK :multi-prefix]
:*raw!znc@znc.in PRIVMSG kyle :IRC -> [@time=2020-07-05T11:39:07.857Z :irc-eu-2.darkscience.net CAP kyle ACK :server-time]
:*raw!znc@znc.in PRIVMSG kyle :IRC -> [@time=2020-07-05T11:39:07.865Z :irc-eu-2.darkscience.net CAP kyle ACK :userhost-in-names]

ZNC *raw CAP LS
:*raw!znc@znc.in PRIVMSG kyle :YOU -> [ZNC *raw CAP LS]
:*raw!znc@znc.in PRIVMSG kyle :IRC -> [@time=2020-07-05T11:39:18.827Z :irc-eu-2.darkscience.net CAP kyle LS :account-notify account-tag away-notify batch cap-notify chghost echo-message extended-join inspircd.org/poison inspircd.org/standard-replies invite-notify labeled-response multi-prefix sasl server-time setname userhost-in-names ]
:*raw!znc@znc.in PRIVMSG kyle :IRC -> [@time=2020-07-05T11:39:18.838Z :irc-eu-2.darkscience.net CAP kyle ACK :account-notify]
:*raw!znc@znc.in PRIVMSG kyle :IRC -> [@time=2020-07-05T11:39:18.847Z :irc-eu-2.darkscience.net CAP kyle ACK :away-notify]
:*raw!znc@znc.in PRIVMSG kyle :IRC -> [@time=2020-07-05T11:39:18.855Z :irc-eu-2.darkscience.net CAP kyle ACK :extended-join]
:*raw!znc@znc.in PRIVMSG kyle :IRC -> [@time=2020-07-05T11:39:18.864Z :irc-eu-2.darkscience.net CAP kyle ACK :multi-prefix]
:*raw!znc@znc.in PRIVMSG kyle :IRC -> [@time=2020-07-05T11:39:18.872Z :irc-eu-2.darkscience.net CAP kyle ACK :server-time]
:*raw!znc@znc.in PRIVMSG kyle :IRC -> [@time=2020-07-05T11:39:19.087Z :irc-eu-2.darkscience.net CAP kyle ACK :userhost-in-names]

Are there any other cases I am not thinking of? I am wondering about the interplay with some modules pausing capabilities after registration (perhaps PauseCap shouldn't do anything after registration?).

@codecov
Copy link

codecov bot commented Jul 5, 2020

Codecov Report

Merging #1735 into master will decrease coverage by 0.00%.
The diff coverage is 90.90%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1735      +/-   ##
==========================================
- Coverage   39.36%   39.35%   -0.01%     
==========================================
  Files         123      123              
  Lines       28198    28199       +1     
  Branches       99       99              
==========================================
- Hits        11100    11099       -1     
- Misses      17049    17051       +2     
  Partials       49       49              
Impacted Files Coverage Δ
src/IRCSock.cpp 65.99% <90.90%> (+0.03%) ⬆️
src/HTTPSock.cpp 42.45% <0.00%> (-0.81%) ⬇️
include/znc/IRCSock.h 68.96% <0.00%> (+6.89%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 276ff8e...a89c0e2. Read the comment docs.

@DarthGandalf
Copy link
Member

perhaps PauseCap shouldn't do anything after registration?

I agree

}

SendNextCap();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will send CAP END as the response to CAP NEW

@DarthGandalf
Copy link
Member

One more case: 2 CAP NEW in a row, so that second one was received before CAP ACK from our REQ from the first one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants