Page MenuHomePhabricator

Incorrect sorting columns in IP/CIDR format
Open, MediumPublic

Description

Author: jebnor

Description:
Colums in the IP/CIDR format to not sort properly in class="wikitable sortable" tables.

IPs sort correctly, (apparently using numeric sort, which is nice.)

{| class="wikitable sortable"
! A !! IP 
|-
| errata-prod || 25.71.30.0 
|-
| errata-test || 25.71.31.0 
|-
| Satellite || 25.71.32.0 
|-
| foobar || 25.23.33.0
|-
| AD3 || 25.71.33.0
|-
| AD3 FOO || 25.71.34.0
|-
| zfs || 192.168.151.0
|}

Upon sorting by IP, we get a table like this:

AIP
foobar25.23.33.0
errata-prod25.71.30.0
errata-test25.71.31.0
Satellite25.71.32.0
AD325.71.33.0
AD3 FOO25.71.34.0
zfs192.168.151.0

However when we add the CIDR mask to the IPs, the table does not sort correctly.

{| class="wikitable sortable"
! A !! IP 
|-
| errata-prod || 25.71.30.0/24
|-
| errata-test || 25.71.31.0/24
|-
| Satellite || 25.71.32.0/24
|-
| foobar || 25.23.33.0/24
|-
| AD3 || 25.71.33.0/24
|-
| AD3 FOO || 25.71.34.0/24
|-
| zfs || 192.168.151.0/24
|}

Sorting by IP gives

AIP
errata-prod25.71.30.0/24
errata-test25.71.31.0/24
Satellite25.71.32.0/24
foobar25.23.33.0/24
AD325.71.33.0/24
AD3 FOO25.71.34.0/24
zfs192.168.151.0/24

Which is incorrect, 'foobar' is not where is should be. It appears to be sorted by the third decimal octet which is confirmed by changing the foobar value to '25.23.47.0/24'
Which gives the 'sorted' table

AIP
errata-prod25.71.30.0/24
errata-test25.71.31.0/24
Satellite25.71.32.0/24
AD325.71.33.0/24
AD3 FOO25.71.34.0/24
foobar25.23.47.0/24
zfs192.168.151.0/24

Changing the CIDR mask doesn't change the outcome

AIP
errata-prod25.71.30.0/16
errata-test25.71.31.0/16
Satellite25.71.32.0/16
AD325.71.33.0/16
AD3 FOO25.71.34.0/24
foobar25.23.47.0/16
zfs192.168.151.0/24

EXPECTED OUTCOME:
Table Sorted by IP/CIDR should output the rows in the same order as IP alone.

AIP
foobar25.23.47.0/16
errata-prod25.71.30.0/16
errata-test25.71.31.0/16
Satellite25.71.32.0/16
AD325.71.33.0/16
AD3 FOO25.71.34.0/24
zfs192.168.151.0/24

Details

Reference
bz34475

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 12:09 AM
bzimport set Reference to bz34475.
bzimport added a subscriber: Unknown Object (MLST).

The autodetection filter will detect this as IPAddresses and sort it like these, but IP/CIDR is not supported by the auto detection, causing unexpected behavior.

The proper solution here is to add CIDR support, or to enforce IP sorting by using "data-sort-type='IPAddress'" on the column. Problem is though that WMF wiki's still haven't enable HTML5 mode, making the latter option impossible.

it matches one of the 3 'date' regexes in autodetect mode to be exact...

Added support for CIDR notation in r111829

jebnor wrote:

Thanks so much for the fix. Minor issue with it. The /xx value in the test cases are greater than the max number of bits in the IP address, which is 32. Also, the regexp looks for 1-3 digits in the /xx portion. Valid values would be 1-32.

These observations do not affect the correctness of the solution, but it may be prudent to minimize overlay with other formats.

I won't be reapplying this change btw, the requested complexity is too much. It has turned from 'accept CIDR notation" into "handle CIDR notation in sort algorithm". Turning it from a 1 hour patch into a multi hour exercise in math and efficiency, for which I do not have the time.

Adding myself in CC as a reminder. Will probably finish the implementation post git migration.

This will fix itself if bug 45161 is solved.

matmarex set Security to None.
matmarex removed a subscriber: Unknown Object (MLST).