Merge pull request #2222 from sopel-irc/adminchannel-hostmask-fix

adminchannel: fix, simplify, and comment hostmask utility function

sopel/modules/adminchannel.py

@@ -117,28 +117,37 @@ def kick(bot, trigger):
 
 
 def configureHostMask(mask):
-    if mask == '*!*@*':
-        return mask
-    if re.match('^[^.@!/]+$', mask) is not None:
+    # shortcut for nick!*@*
+    if re.match(r'^[^.@!/\s]+$', mask) is not None:
         return '%s!*@*' % mask
-    if re.match('^[^@!]+$', mask) is not None:
+
+    # shortcut for *!*@host
+    # won't work for local names w/o dot, but does support cloaks/with/slashes
+    if re.match(r'^[^@!\s]+$', mask) is not None:
         return '*!*@%s' % mask
 
-    m = re.match('^([^!@]+)@$', mask)
+    # shortcut for *!user@*
+    # requires trailing @ to be recognized as a username instead of a nick
+    m = re.match(r'^([^!@\s]+)@$', mask)
     if m is not None:
         return '*!%s@*' % m.group(1)
 
-    m = re.match('^([^!@]+)@([^@!]+)$', mask)
+    # shortcut for *!user@host
+    m = re.match(r'^([^!@\s]+)@([^@!\s]+)$', mask)
     if m is not None:
         return '*!%s@%s' % (m.group(1), m.group(2))
 
-    m = re.match('^([^!@]+)!(^[!@]+)@?$', mask)
+    # shortcut for nick!user@*
+    m = re.match(r'^([^!@\s]+)!([^!@\s]+)@?$', mask)
     if m is not None:
         return '%s!%s@*' % (m.group(1), m.group(2))
 
-    if re.match(r'^\S+[!]\S+[@]\S+$', mask) is not None:
+    # not a shortcut; validate full NUH format
+    if re.match(r'^[^!@\s]+![^!@\s]+@[^!@\s]+$', mask) is not None:
         return mask
-    return ''
+
+    # not a shortcut nor a valid hostmask
+    raise ValueError('Invalid hostmask format or unsupported shorthand')
 
 
 @plugin.require_chanmsg
@@ -163,9 +172,12 @@ def ban(bot, trigger):
             return
         channel = opt
         banmask = text[2]
-    banmask = configureHostMask(banmask)
-    if banmask == '':
+
+    try:
+        banmask = configureHostMask(banmask)
+    except ValueError:
         return
+
     bot.write(['MODE', channel, '+b', banmask])
 
 
@@ -190,9 +202,12 @@ def unban(bot, trigger):
             return
         channel = opt
         banmask = text[2]
-    banmask = configureHostMask(banmask)
-    if banmask == '':
+
+    try:
+        banmask = configureHostMask(banmask)
+    except ValueError:
         return
+
     bot.write(['MODE', channel, '-b', banmask])
 
 
@@ -217,9 +232,12 @@ def quiet(bot, trigger):
             return
         quietmask = text[2]
         channel = opt
-    quietmask = configureHostMask(quietmask)
-    if quietmask == '':
+
+    try:
+        quietmask = configureHostMask(quietmask)
+    except ValueError:
         return
+
     bot.write(['MODE', channel, '+q', quietmask])
 
 
@@ -244,9 +262,12 @@ def unquiet(bot, trigger):
             return
         quietmask = text[2]
         channel = opt
-    quietmask = configureHostMask(quietmask)
-    if quietmask == '':
+
+    try:
+        quietmask = configureHostMask(quietmask)
+    except ValueError:
         return
+
     bot.write(['MODE', channel, '-q', quietmask])
 
 
@@ -278,9 +299,12 @@ def kickban(bot, trigger):
         mask = text[3]
         reasonidx = 4
     reason = ' '.join(text[reasonidx:])
-    mask = configureHostMask(mask)
-    if mask == '':
+
+    try:
+        mask = configureHostMask(mask)
+    except ValueError:
         return
+
     bot.write(['MODE', channel, '+b', mask])
     bot.kick(nick, channel, reason)
 

test/modules/test_modules_adminchannel.py

@@ -0,0 +1,43 @@
+"""Tests for Sopel's ``adminchannel`` plugin"""
+from __future__ import generator_stop
+
+import pytest
+
+from sopel.modules import adminchannel
+
+
+VALID_INPUTS = (
+    ('justanick', 'justanick!*@*'),
+    ('just-a.host', '*!*@just-a.host'),
+    ('justauser@', '*!justauser@*'),
+    ('someuser@just-a.host', '*!someuser@just-a.host'),
+    ('someuser@dotlesshost', '*!someuser@dotlesshost'),
+    ('somenick!someuser', 'somenick!someuser@*'),
+    ('somenick!someuser@', 'somenick!someuser@*'),
+    ('somenick!someuser@', 'somenick!someuser@*'),
+    ('full!host@mask', 'full!host@mask'),
+    ('full!mask@host.with.dots', 'full!mask@host.with.dots'),
+    ('libera/style/cloak', '*!*@libera/style/cloak'),
+)
+
+INVALID_INPUTS = (
+    'mask with whitespace',
+    'cloak/with whitespace',
+    'nick!auser@something with whitespace',
+    'nick with spaces!user@host',
+    'nick!user with spaces@host',
+    'two!user!names@host',
+    'two!user@host@names',
+)
+
+
+@pytest.mark.parametrize('raw, checked', VALID_INPUTS)
+def test_configureHostMask(raw, checked):
+    """Test the `configureHostMask` helper for functionality and compatibility."""
+    assert adminchannel.configureHostMask(raw) == checked
+
+
+@pytest.mark.parametrize('raw', INVALID_INPUTS)
+def test_configureHostMask_invalid(raw):
+    with pytest.raises(ValueError):
+        adminchannel.configureHostMask(raw)