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.

Sign up for GitHub

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

kick: Exhaustive implementation of the Modern spec + honor TARGMAX in testDoubleKickMessages #100

Merged
merged 2 commits into from
Aug 26, 2021
Merged

kick: Exhaustive implementation of the Modern spec + honor TARGMAX in testDoubleKickMessages #100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
42c3de0
kick: Exhaustive implementation of the Modern spec + honor TARGMAX in…
progval Aug 13, 2021
a5642ff
Update spec reference to @tommyrot's PR, it's better
progval Aug 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,13 @@ BAHAMUT_SELECTORS := \

# testQuitErrors is very flaky
# AccountTagTestCase.testInvite fails because https://github.com/solanum-ircd/solanum/issues/166
# testKickDefaultComment fails because it uses the nick of the kickee rather than the kicker.
CHARYBDIS_SELECTORS := \
not Ergo \
and not deprecated \
and not strict \
and not testDoubleKickMessages \
and not testQuitErrors \
and not testKickDefaultComment \
and not (AccountTagTestCase and testInvite) \
$(EXTRA_SELECTORS)

Expand Down Expand Up @@ -57,6 +58,7 @@ INSPIRCD_SELECTORS := \
# lusers tests fail because they depend on Modern behavior, not just RFC2812 (TODO: update lusers tests to accept RFC2812-compliant implementations)
# statusmsg tests fail because STATUSMSG is present in ISUPPORT, but it not actually supported as PRIVMSG target
# testKeyValidation[empty] fails because ircu2 returns ERR_NEEDMOREPARAMS on empty keys: https://github.com/UndernetIRC/ircu2/issues/13
# testKickDefaultComment fails because it uses the nick of the kickee rather than the kicker.
IRCU2_SELECTORS := \
not Ergo \
and not deprecated \
Expand All @@ -66,6 +68,7 @@ IRCU2_SELECTORS := \
and not lusers \
and not statusmsg \
and not (testKeyValidation and empty) \
and not testKickDefaultComment \
$(EXTRA_SELECTORS)

# same justification as ircu2
Expand All @@ -80,11 +83,13 @@ SNIRCD_SELECTORS := \
$(EXTRA_SELECTORS)

# testListEmpty and testListOne fails because irc2 deprecated LIST
# testKickDefaultComment fails because it uses the nick of the kickee rather than the kicker.
IRC2_SELECTORS := \
not Ergo \
and not deprecated \
and not strict \
and not testListEmpty and not testListOne \
and not testKickDefaultComment \
$(EXTRA_SELECTORS)

MAMMON_SELECTORS := \
Expand All @@ -101,12 +106,13 @@ LIMNORIA_SELECTORS := \
$(EXTRA_SELECTORS)

# testQuitErrors is too flaky for CI
# testKickDefaultComment fails because solanum uses the nick of the kickee rather than the kicker.
SOLANUM_SELECTORS := \
not Ergo \
and not deprecated \
and not strict \
and not testDoubleKickMessages \
and not testQuitErrors \
and not testKickDefaultComment \
$(EXTRA_SELECTORS)

SOPEL_SELECTORS := \
Expand Down
98 changes: 93 additions & 5 deletions irctest/server_tests/kick.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,17 @@


class KickTestCase(cases.BaseServerTestCase):
@cases.mark_specifications("RFC1459", "RFC2812")
@cases.mark_specifications("RFC1459", "RFC2812", "Modern")
def testKickSendsMessages(self):
"""“Once a user has joined a channel, he receives information about
all commands his server receives affecting the channel. This
includes […] KICK”
-- <https://tools.ietf.org/html/rfc1459#section-4.2.1>
and <https://tools.ietf.org/html/rfc2812#section-3.2.1>

" If a comment is given, this will be sent instead of the default message,
the nickname of the user targeted by the KICK."
-- https://github.com/ircdocs/modern-irc/pull/101
"""
self.connectClient("foo")
self.joinChannel(1, "#chan")
Expand Down Expand Up @@ -49,6 +53,59 @@ def testKickSendsMessages(self):
m = self.getMessage(3)
self.assertMessageMatch(m, command="KICK", params=["#chan", "bar", "bye"])

def _testKickNoComment(self, check_default):
self.connectClient("foo")
self.joinChannel(1, "#chan")

self.connectClient("bar")
self.joinChannel(2, "#chan")

self.connectClient("baz")
self.joinChannel(3, "#chan")

# TODO: check foo is an operator

self.getMessages(1)
self.getMessages(2)
self.getMessages(3)
self.sendLine(1, "KICK #chan bar")
try:
m = self.getMessage(1)
if m.command == "482":
raise runner.ImplementationChoice(
"Channel creators are not opped by default."
)
self.assertMessageMatch(m, command="KICK")
except client_mock.NoMessageException:
# The RFCs do not say KICK must be echoed
pass
m2 = self.getMessage(2)
m3 = self.getMessage(3)
if check_default:
self.assertMessageMatch(m2, command="KICK", params=["#chan", "bar", "foo"])
self.assertMessageMatch(m3, command="KICK", params=["#chan", "bar", "foo"])
else:
self.assertMessageMatch(m2, command="KICK", params=["#chan", "bar", ANYSTR])
self.assertMessageMatch(m3, command="KICK", params=["#chan", "bar", ANYSTR])

@cases.mark_specifications("RFC2812")
def testKickDefaultComment(self):
"""
"If a "comment" is
given, this will be sent instead of the default message, the nickname
of the user issuing the KICK."
-- https://datatracker.ietf.org/doc/html/rfc2812#section-3.2.8
"""
self._testKickNoComment(check_default=True)

@cases.mark_specifications("Modern")
def testKickNoComment(self):
"""
"If no comment is given, the server SHOULD use a default message instead."
-- https://github.com/ircdocs/modern-irc/pull/101
"""
self._testKickNoComment(check_default=False)

@cases.mark_specifications("RFC2812")
def testKickPrivileges(self):
"""Test who has the ability to kick / what error codes are sent
Expand Down Expand Up @@ -116,12 +173,37 @@ def testKickNonexistentChannel(self):
self.assertMessageMatch(m, command="403")

@pytest.mark.parametrize("multiple_targets", [True, False])
@cases.mark_specifications("RFC2812")
@cases.mark_specifications("RFC2812", "Modern", "ircdocs")
def testDoubleKickMessages(self, multiple_targets):
"""“The server MUST NOT send KICK messages with multiple channels or
users to clients. This is necessarily to maintain backward
compatibility with old client software.”
-- https://tools.ietf.org/html/rfc2812#section-3.2.8

"The server MUST NOT send KICK messages with multiple channels or
users to clients.
This is necessary to maintain backward compatibility with existing
client software."
-- https://github.com/ircdocs/modern-irc/pull/101

"Servers MAY limit the number of target users per `KICK` command
via the [`TARGMAX` parameter of `RPL_ISUPPORT`](#targmax-parameter),
and silently drop targets if the number of targets exceeds the limit."
-- https://github.com/ircdocs/modern-irc/pull/101

"If the "TARGMAX" parameter is not advertised or a value is not sent
then a client SHOULD assume that no commands except the "JOIN" and "PART"
commands accept multiple parameters."
-- https://defs.ircdocs.horse/defs/isupport.html#targmax

"If this parameter is not advertised or a value is not sent then a client
SHOULD assume that no commands except the `JOIN` and `PART` commands
accept multiple parameters."
-- https://github.com/ircdocs/modern-irc/pull/113

"If <limit> is not specified, then there is no maximum number of targets
for that command."
-- https://modern.ircdocs.horse/#targmax-parameter
"""
self.connectClient("foo")
self.joinChannel(1, "#chan")
Expand All @@ -135,6 +217,14 @@ def testDoubleKickMessages(self, multiple_targets):
self.connectClient("qux")
self.joinChannel(4, "#chan")

targmax = dict(
item.split(":", 1)
for item in self.server_support.get("TARGMAX", "").split(",")
if item
)
if targmax.get("KICK", "1") == "1":
raise runner.NotImplementedByController("Multi-target KICK")

# TODO: check foo is an operator

# Synchronize
Expand All @@ -153,14 +243,12 @@ def testDoubleKickMessages(self, multiple_targets):
raise runner.OptionalExtensionNotSupported(
"Channel creators are not opped by default."
)
if m.command in {"401", "403"}:
raise runner.NotImplementedByController("Multi-target KICK")
except client_mock.NoMessageException:
# The RFCs do not say KICK must be echoed
pass

mgroup = self.getMessages(4)
self.assertGreaterEqual(len(mgroup), 2)
self.assertGreaterEqual(len(mgroup), 2, mgroup)
m1, m2 = mgroup[:2]

self.assertMessageMatch(m1, command="KICK", params=["#chan", ANYSTR, "bye"])
Expand Down