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

Ignore multiple spaces between user name and expiry in list commands #44

Closed
Krinkle opened this issue Apr 26, 2018 · 0 comments
Closed

Comments

@Krinkle
Copy link
Member

Krinkle commented Apr 26, 2018

# Without expiry, fine.
CVNBot14 bl add 71.237.252.43  r=vandalism on en.wikipedia
<•CVNBot14> Added: 71.237.252.43 is on global blacklist, added by Krinkle until 02:38, 27 May 2018 ("vandalism on en.wikipedia")

# With two spaces before "x=", adds it as user name, bug!
CVNBot14 bl add 71.237.252.43  x=7440  r=vandalism on en.wikipedia
 <•CVNBot14> Added: 71.237.252.43  x=7440 is on global blacklist, added by Krinkle until 02:39, 27 May 2018 ("vandalism on en.wikipedia")

# With one space before "x=", works as expected.
<•Krinkle> CVNBot14 bl add 71.237.252.43 x=5000 r=vandalism on en.wikipedia
<•CVNBot14> Updated: 71.237.252.43 is on global blacklist, added by Krinkle until 10:43, 20 November 2018 ("vandalism on en.wikipedia")

# With two spaces before "x=", while bad, worked for "bl add", but fails for "bl del", double bug!
CVNBot14 bl del 71.237.252.43  x=7440
<•CVNBot14> Deleted 71.237.252.43 from global blacklist

This means they can be added, but not removed. Anyway, we should fix it so that multiple spaces are ignored.

The problem is at https://github.com/countervandalism/CVNBot/blob/6c60cd4bd1d0f6d8a67847b91ae7fa89570ee827/src/CVNBot/ListManager.cs#L30

^(?<cmd>add|del|show|test) (?<item>.+?)(?: p=(?<project>\S+?))?(?: x=(?<len>\d{1,4}))?(?: r=(?<reason>.+?))?$            

It only considers x = as expiry if there is no space after its value or if there is exactly one space between its value and r=. Given there were multiple spaces, it was not matched, and instead consumed by the <item> match.

@Krinkle Krinkle added this to the 3.x milestone Aug 8, 2019
@Krinkle Krinkle removed this from the 3.x milestone Jun 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

1 participant