User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b) Gecko/20050115 Firefox/1.0+ Build Identifier: Chatzilla 0.9.67 [Firefox 1.0+/20050115] With hostnames, the most specific part is usually before the first dot, so ChatZilla puts a * there when banning. With numeric IP addresses, it should do things the other way around. Reproducible: Always Steps to Reproduce: Kick and ban a user without a hostname using the context menu or the kick-ban command. Actual Results: A mode like "+b *!user@*.0.0.1" is set. Expected Results: A mode like "+b *!firstname.lastname@example.org.*" should be set instead.
Created attachment 171573 [details] [diff] [review] draft patch This seems to work, I'm not too sure about coding style though...
ChatZilla coding style would have: if (condition) statement; else statement; As long as there's only one statement with only one line of code in each if-block and else-block. See http://www.hacksrus.com/~ginda/pedant.html I'm not entitled to review this or anything, but I guessed you might appreciate some feedback. If you're done, you can request review from ssieb or rginda.
Created attachment 178369 [details] [diff] [review] patch 2
Attachment #178369 - Flags: review?(jessiebarnes72) → review?(samuel)
Comment on attachment 178369 [details] [diff] [review] patch 2 >+ var hostmask = e.user.host.replace(/[^.]+$/, "*"); >+ else >+ var hostmask = e.user.host.replace(/^[^.]+/, "*"); This will cause a strict warning, you declared the variable twice. Declare it before the test.
Attachment #178369 - Flags: review?(samuel) → review-
Created attachment 178377 [details] [diff] [review] patch 3 - avoid strict warning
Comment on attachment 178377 [details] [diff] [review] patch 3 - avoid strict warning a=asa
Attachment #178377 - Flags: approval1.8b2? → approval1.8b2+
Checked in --> FIXED. An automatic nightly ChatZilla build containing this fix will be available in around 40 minutes from http://twpol.dyndns.org/mozilla/chatzilla/nightly/
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
This bug regressed when ban and unban were implemented, and determining the ban mask was moved to CIRCUser.prototype.getBanMask. 0.9.68.* all have a 'proper' getBanMask, cvs does not. We need to migrate this patch to the appropriate place in irc.js Note that we still no longer put a * on the first bit of the ip - we just fail to replace the last bit with a *. Setting minority to minor. This may also be a wontfix - depends how useful we think this really is. It's 3.30am atm, so don't ask me to make any sensible decisions. James, Robert, Samuel, it's your call :-)
Severity: normal → minor
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Created attachment 200810 [details] [diff] [review] Fix regression by porting older patch As I'm adding this, I wonder if this would work with ipv6? Do we work with ipv6? Do IRC servers work with ipv6 at all?
Attachment #200810 - Flags: review?(silver)
Comment on attachment 200810 [details] [diff] [review] Fix regression by porting older patch >- if (!/^\d+\.\d+\.\d+\.\d+$/.test(hostmask)) >+ if (hostmask.match(/^[\d\.]*$/) != null) >+ hostmask = hostmask.replace(/[^.]+$/, "*"); >+ else That regexp wont work. It requires the IP to be "126.96.36.199." which is wont ever be (no trailing period). The original one used by the code that you are removing will, however, work. I'm not actually sure we want this at all, though.
Attachment #200810 - Flags: review?(silver) → review-
Sorry, I misread the regexp there. I still prefer the one being removed, though.
Created attachment 206769 [details] [diff] [review] Patch with old regexp in place Alright. So then this patch should do, or we should just close the bug as fixed again. That's up to you though :-).
Comment on attachment 206769 [details] [diff] [review] Patch with old regexp in place I think, per a discussion on IRC, we want to review these munges at some point, but for now I'm OK with just banning an entire subnet.
Attachment #206769 - Flags: review?(silver) → review+
Checked in --> (RE)FIXED.
Status: REOPENED → RESOLVED
Last Resolved: 14 years ago → 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.