Page MenuHomePhabricator
Create Task
Maniphest T275394

Normalise IP addresses in HTMLUsersMultiselectField, to avoid adding multiple equivalent IP addresses
Closed, ResolvedPublic
Actions

Description

It is not possible to add multiple equivalent usernames twice using the JS UsersMultiselectWidget, e.g. "Admin 1" and "Admin_1". Since T274568, this is also the case for the PHP widget.

However, in both modes, the widget treats equivalent IP addresses as separate entries:

image.png (100×720 px, 10 KB)

This is problematic because:

  • Anything that handles the widget's values needs to ensure that the IP addresses are unique
  • If the widget has a limit set, it uses up extra entries for equivalent IPs

An example of where equivalents are (unintentionally) treated as two separate IP addresses downstream can be found in CheckUser:

image.png (146×381 px, 9 KB)

Details

SubjectRepoBranchLines +/-
Normalise IP addresses in HTMLUsersMultiselectFieldmediawiki/coremaster+6 -2
Customize query in gerrit

Related Objects

Event Timeline

Tchanders created this task.Feb 22 2021, 2:32 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 22 2021, 2:32 PM
Tchanders mentioned this in T274568: [Bug] Normalise usernames in no-JS.Feb 22 2021, 2:32 PM
• Niharika triaged this task as Medium priority.Feb 23 2021, 3:21 PM
• Niharika moved this task from Untriaged to Triage/To be Estimated on the Anti-Harassment board.
Tchanders renamed this task from Normalise IP addresses in HTMLSUsersMultiselectField, to avoid adding multiple equivalent IP addresses to Normalise IP addresses in HTMLUsersMultiselectField, to avoid adding multiple equivalent IP addresses.Feb 23 2021, 4:58 PM
• TThoabala claimed this task.Mar 8 2021, 6:16 PM
• TThoabala moved this task from Triage/To be Estimated to The Letter Song on the Anti-Harassment board.Mar 8 2021, 6:20 PM
• TThoabala edited projects, added Anti-Harassment (The Letter Song); removed Anti-Harassment.
• TThoabala moved this task from Ready 🎬 (ONLY IF YOU HAVE NO MORE CODE TO REVIEW) to In Progress 💪 on the Anti-Harassment (The Letter Song) board.Mar 8 2021, 7:00 PM
gerritbot added a comment.Mar 16 2021, 8:18 PM

Change 672798 had a related patch set uploaded (by TsepoThoabala; owner: TsepoThoabala):
[mediawiki/core@master] Normalise IP addresses in HTMLUsersMultiselectField

https://gerrit.wikimedia.org/r/672798

gerritbot added a project: Patch-For-Review.Mar 16 2021, 8:18 PM
• TThoabala moved this task from In Progress 💪 to Code Review 🔍 on the Anti-Harassment (The Letter Song) board.Mar 17 2021, 1:42 PM
Tchanders mentioned this in T278153: De-duplicate IP addresses in JS UsersMultiselectWidget.Mar 22 2021, 4:31 PM
Tchanders moved this task from Code Review 🔍 to QA/Testing 🐞 on the Anti-Harassment (The Letter Song) board.Mar 22 2021, 5:18 PM
Tchanders added a subscriber: dom_walden.Mar 22 2021, 5:23 PM

@dom_walden There's a known issue with the solution we've merged, i.e. that it doesn't solve this point for the JS widget:

  • If the widget has a limit set, it uses up extra entries for equivalent IPs

Filed separately as T278153

gerritbot added a comment.Mar 22 2021, 5:50 PM

Change 672798 merged by jenkins-bot:
[mediawiki/core@master] Normalise IP addresses in HTMLUsersMultiselectField

https://gerrit.wikimedia.org/r/672798

ReleaseTaggerBot added a project: MW-1.36-notes (1.36.0-wmf.36; 2021-03-23).Mar 22 2021, 6:00 PM
Maintenance_bot removed a project: Patch-For-Review.Mar 22 2021, 6:10 PM
dom_walden moved this task from QA/Testing 🐞 to Done: Q3 (2020-2021) on the Anti-Harassment (The Letter Song) board.Mar 23 2021, 2:12 PM

We are now removing duplicate IPs of the form ::1 and 0:0:0:0:0:0:0:1, or 3e2b:a4:9ed7:bf36:4aca:5189:c496:5a05 and 3e2b:00a4:9ed7:bf36:4aca:5189:c496:5a05 when submitting Special:Investigate.

I checked that, after removing duplicates, we are still making the correct CheckUser database queries in the backend (for IPv4 and IPv6 addresses and ranges).

I briefly tested that we can still submit Usernames via Special:Investigate, and the bug in T274568 is still fixed.

Duplicates we are still not removing:

  • 192.168.1.2 and 192.168.1.2/32 are treated as different, even though technically they are the same (same goes for IPv6).
  • 192.168.123.0/24 and 192.168.123.123/24 are treated as different, even though they are the same (same goes for IPv6).

Test environment: local docker MediaWiki 1.36.0-alpha (0012150) 07:23, 23 March 2021; CheckUser 2.5 (60d4d16) 06:21, 13 March 2021.

Tchanders mentioned this in T278338: De-duplicate equivalent IP ranges in HTMLUsersMultiselectField.Mar 24 2021, 2:56 PM
• Niharika closed this task as Resolved.Mar 30 2021, 2:30 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL