diff --git a/client/src/__tests__/helpers.test.ts b/client/src/__tests__/helpers.test.ts index d49c07d0298..3dea94cef8e 100644 --- a/client/src/__tests__/helpers.test.ts +++ b/client/src/__tests__/helpers.test.ts @@ -1,9 +1,9 @@ import { describe, expect, test, afterEach, vi, beforeEach, it } from 'vitest'; -import { sortIp, countClientsStatistics, findAddressType, subnetMaskToBitMask } from '../helpers/helpers'; +import { sortIp, countClientsStatistics, findAddressType, subnetMaskToBitMask, sortAddress } from '../helpers/helpers'; import { ADDRESS_TYPES } from '../helpers/constants'; -describe('sortIp', () => { +function testBaseSortFunction(sortIp: (a: string, b: string) => number) { describe('ipv4', () => { test('one octet differ', () => { const arr = ['127.0.2.0', '127.0.3.0', '127.0.1.0']; @@ -257,6 +257,63 @@ describe('sortIp', () => { }); }); + describe('mixed', () => { + test('ipv4, ipv6 in short and long forms and cidr', () => { + const arr = [ + '2001:db8:11a3:9d7:0:0:0:1/32', + '192.168.1.2', + '127.0.0.2', + '2001:db8:11a3:9d7::1/128', + '2001:db8:11a3:9d7:0:0:0:1', + '127.0.0.1/12', + '192.168.1.1', + '2001:db8::/32', + '2001:db8:11a3:9d7::1/24', + '192.168.1.2/12', + '2001:db7::/32', + '127.0.0.1', + '2001:db8:11a3:9d7:0:0:0:2', + '192.168.1.1/24', + '2001:db7::/64', + '2001:db7::', + '2001:db8::', + '2001:db8:11a3:9d7:0:0:0:1/128', + '192.168.1.1/12', + '127.0.0.1/32', + '::1', + ]; + const sortedArr = [ + '127.0.0.1/12', + '127.0.0.1/32', + '127.0.0.1', + '127.0.0.2', + '192.168.1.1/12', + '192.168.1.1/24', + '192.168.1.1', + '192.168.1.2/12', + '192.168.1.2', + '::1', + '2001:db7::/32', + '2001:db7::/64', + '2001:db7::', + '2001:db8::/32', + '2001:db8::', + '2001:db8:11a3:9d7::1/24', + '2001:db8:11a3:9d7:0:0:0:1/32', + '2001:db8:11a3:9d7::1/128', + '2001:db8:11a3:9d7:0:0:0:1/128', + '2001:db8:11a3:9d7:0:0:0:1', + '2001:db8:11a3:9d7:0:0:0:2', + ]; + + expect(arr.sort(sortIp)).toStrictEqual(sortedArr); + }); + }); +} + +describe('sortIp', () => { + testBaseSortFunction(sortIp); + describe('invalid input', () => { const originalWarn = console.warn; @@ -293,16 +350,75 @@ describe('sortIp', () => { expect(arr.sort(sortIp)).toStrictEqual(arr); }); }); +}) - describe('mixed', () => { - test('ipv4, ipv6 in short and long forms and cidr', () => { +describe('sortAddress', () => { + testBaseSortFunction(sortAddress); + + describe('domain_names_sorting', () => { + test('widcard before other names', () => { + const arr = [ + 'home.fritz.box', + '*.home.fritz.box', + 'adguard-home.fritz.box' + ] + + const sortedArr = [ + '*.home.fritz.box', + 'adguard-home.fritz.box', + 'home.fritz.box' + ]; + + expect(arr.sort(sortAddress)).toStrictEqual(sortedArr); + }) + + test('only non-ip strings', () => { + const arr = [ + 'fritz.box', + 'adguard-home.fritz.box', + 'foo.bar', + 'example.com', + 'my.router.local', + 'office.lan', + 'server.example.org', + 'mail.google.com', + 'desktop.home', + 'printer.office.lan', + 'web.internal', + 'host123.domain.net', + 'api.service.company', + ]; + + const sortedArr = [ + 'adguard-home.fritz.box', + 'api.service.company', + 'desktop.home', + 'example.com', + 'foo.bar', + 'fritz.box', + 'host123.domain.net', + 'mail.google.com', + 'my.router.local', + 'office.lan', + 'printer.office.lan', + 'server.example.org', + 'web.internal', + ]; + + expect(arr.sort(sortAddress)).toStrictEqual(sortedArr); + }); + + test('ipv4, ipv6 in short and long forms and cidr, and a few domain names', () => { const arr = [ '2001:db8:11a3:9d7:0:0:0:1/32', '192.168.1.2', '127.0.0.2', + 'foo.bar', '2001:db8:11a3:9d7::1/128', '2001:db8:11a3:9d7:0:0:0:1', '127.0.0.1/12', + 'fritz.box', + 'adguard-home.fritz.box', '192.168.1.1', '2001:db8::/32', '2001:db8:11a3:9d7::1/24', @@ -317,6 +433,7 @@ describe('sortIp', () => { '2001:db8:11a3:9d7:0:0:0:1/128', '192.168.1.1/12', '127.0.0.1/32', + '*.home.fritz.net', '::1', ]; const sortedArr = [ @@ -341,12 +458,16 @@ describe('sortIp', () => { '2001:db8:11a3:9d7:0:0:0:1/128', '2001:db8:11a3:9d7:0:0:0:1', '2001:db8:11a3:9d7:0:0:0:2', + '*.home.fritz.net', + 'adguard-home.fritz.box', + 'foo.bar', + 'fritz.box', ]; - expect(arr.sort(sortIp)).toStrictEqual(sortedArr); + expect(arr.sort(sortAddress)).toStrictEqual(sortedArr); }); }); -}); +}) describe('findAddressType', () => { it('should return IP type for IP addresses', () => { diff --git a/client/src/components/Filters/Rewrites/Table.tsx b/client/src/components/Filters/Rewrites/Table.tsx index 4f314337ac9..f1a2be5b5f4 100644 --- a/client/src/components/Filters/Rewrites/Table.tsx +++ b/client/src/components/Filters/Rewrites/Table.tsx @@ -4,7 +4,7 @@ import React, { Component } from 'react'; import ReactTable from 'react-table'; import { withTranslation } from 'react-i18next'; -import { sortIp } from '../../../helpers/helpers'; +import { sortAddress } from '../../../helpers/helpers'; import { MODAL_TYPE, TABLES_MIN_ROWS } from '../../../helpers/constants'; import { LocalStorageHelper, LOCAL_STORAGE_KEYS } from '../../../helpers/localStorageHelper'; @@ -37,7 +37,7 @@ class Table extends Component { { Header: this.props.t('answer'), accessor: 'answer', - sortMethod: sortIp, + sortMethod: sortAddress, Cell: this.cellWrap, }, { diff --git a/client/src/helpers/helpers.tsx b/client/src/helpers/helpers.tsx index 3ae453b9b0a..2fc321becd8 100644 --- a/client/src/helpers/helpers.tsx +++ b/client/src/helpers/helpers.tsx @@ -771,69 +771,94 @@ export const getObjectKeysSorted = , K ex }; /** - * @param ip - * @returns {[IPv4|IPv6, 33|129]} + * Helper function for IP and CIDR comparison (supports both v4 and v6). Creates an array of chunks that can be used for comparison. + * @param address - ip, cidr or domain name to create comparison chunks from + * @returns { (string | number)[] } The array of chunks to compare. Those are either bytes for IPs, or domain name parts. */ -const getParsedIpWithPrefixLength = (ip: any) => { - const MAX_PREFIX_LENGTH_V4 = 32; - const MAX_PREFIX_LENGTH_V6 = 128; - - const parsedIp = ipaddr.parse(ip); - const prefixLength = parsedIp.kind() === 'ipv4' ? MAX_PREFIX_LENGTH_V4 : MAX_PREFIX_LENGTH_V6; - - // Increment prefix length to always put IP after CIDR, e.g. 127.0.0.1/32, 127.0.0.1 - return [parsedIp, prefixLength + 1]; -}; - -/** - * Helper function for IP and CIDR comparison (supports both v4 and v6) - * @param item - ip or cidr - * @returns {number[]} - */ -const getAddressesComparisonBytes = (item: any) => { - // Sort ipv4 before ipv6 +const getAddressesComparisonChunks = (address: string, allowOnlyIpAddresses: boolean) => { + // Sort ipv4 before ipv6. Strings that are not IP addresses will be sorted after both. const IP_V4_COMPARISON_CODE = 0; const IP_V6_COMPARISON_CODE = 1; + const NOT_IP_ADDRESS = 2; - const [parsedIp, cidr] = ipaddr.isValid(item) ? getParsedIpWithPrefixLength(item) : ipaddr.parseCIDR(item); + try { + const [parsedIp, cidr] = ipaddr.isValid(address) ? [ ipaddr.parse(address), Number.MAX_SAFE_INTEGER ] : ipaddr.parseCIDR(address); - const [normalizedBytes, ipVersionComparisonCode] = - (parsedIp as IPv4 | IPv6).kind() === 'ipv4' - ? [(parsedIp as IPv4).toIPv4MappedAddress().parts, IP_V4_COMPARISON_CODE] - : [(parsedIp as IPv6).parts, IP_V6_COMPARISON_CODE]; + const [normalizedBytes, ipVersionComparisonCode] = + parsedIp.kind() === 'ipv4' + ? [(parsedIp as IPv4).toIPv4MappedAddress().parts, IP_V4_COMPARISON_CODE] + : [(parsedIp as IPv6).parts, IP_V6_COMPARISON_CODE]; - return [ipVersionComparisonCode, ...normalizedBytes, cidr]; + return [ipVersionComparisonCode, ...normalizedBytes, cidr]; + } + catch (e) { + if (allowOnlyIpAddresses) { + throw new Error(`Invalid address: ${address}. Only IP addresses and CIDRs are allowed.`, { cause: e }); + } + + return [NOT_IP_ADDRESS, address]; + } }; +const getAsElementForSorting = (item: any) => { + // If item is an array, take the first element for sorting + if (Array.isArray(item)) { + item = item[0]; + } + + // String is expected downstream by ipaddr.isValid and .parseCIDR, we can validate it here + if (typeof item !== 'string') { + console.warn('Expected a string. Got', item, `of type ${typeof item} instead.`); + } + + // Make sure we always return a string + return "" + item; +} + /** - * Compare function for IP and CIDR sort in ascending order (supports both v4 and v6) - * @param a - * @param b - * @returns {number} -1 | 0 | 1 + * Creates a sort function for IP addresses, CIDRs and domain names, with given restrictions. + * Don't use this function directly - use sortIp or sortAddress. react-table occasionally passes `true` as third parameter when it is doing reverse sorting. + * @param {boolean} allowOnlyIpAddresses - if true, only IP addresses and CIDRs will be compared. Invalid IP address would throw an exception, leaving the items in their original order. + * @returns A compare function that can be used in Array.prototype.sort() to sort IP addresses, CIDRs and domain names. */ -export const sortIp = (a: any, b: any) => { +const sortFunction = (a: any, b: any, allowOnlyIpAddresses: boolean) => { try { - const comparisonBytesA = Array.isArray(a) ? getAddressesComparisonBytes(a[0]) : getAddressesComparisonBytes(a); - const comparisonBytesB = Array.isArray(b) ? getAddressesComparisonBytes(b[0]) : getAddressesComparisonBytes(b); + const comparisonChunksA = getAddressesComparisonChunks(getAsElementForSorting(a), allowOnlyIpAddresses); + const comparisonChunksB = getAddressesComparisonChunks(getAsElementForSorting(b), allowOnlyIpAddresses); - for (let i = 0; i < comparisonBytesA.length; i += 1) { - const byteA = comparisonBytesA[i]; - const byteB = comparisonBytesB[i]; + for (let i = 0; i < Math.min(comparisonChunksA.length, comparisonChunksB.length); i++) { + const byteA = comparisonChunksA[i]; + const byteB = comparisonChunksB[i]; - if (byteA === byteB) { - // eslint-disable-next-line no-continue - continue; + if (byteA !== byteB) { + return byteA > byteB ? 1 : -1; } - return byteA > byteB ? 1 : -1; } - return 0; + // If all compared chunks are equal, compare by length. Shorter array should come first. + return comparisonChunksA.length - comparisonChunksB.length; } catch (e) { console.warn(e); return 0; } }; +/** + * Compare function for IP, CIDR name sort in ascending order (supports both v4 and v6). + * @param a + * @param b + * @returns {number} -1 | 0 | 1 + */ +export const sortIp = (a: any, b: any) => sortFunction(a, b, true); + +/** + * Compare function for IP, CIDR (supports both v4 and v6) and domain name sort in ascending order. + * @param a + * @param b + * @returns {number} -1 | 0 | 1 + */ +export const sortAddress = (a: any, b: any) => sortFunction(a, b, false); + /** * @param {number} filterId * @returns {string}