From cc7266017ed11c55db7f1d8a3035496064cb2c3a Mon Sep 17 00:00:00 2001 From: Isaac Hill <71404865+isaachilly@users.noreply.github.com> Date: Tue, 1 Sep 2026 18:29:29 +0200 Subject: [PATCH 1/9] Encode filter query in route updates Centralised URL encoding for filters. Removed encoding in `LogFilter` so filter values are no longer pre-encoded before serialisation; keeping query generation in one place. --- InfoLogger/public/Model.js | 11 ++++++++++- InfoLogger/public/logFilter/LogFilter.js | 3 --- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/InfoLogger/public/Model.js b/InfoLogger/public/Model.js index 48987d06f..76bc11848 100644 --- a/InfoLogger/public/Model.js +++ b/InfoLogger/public/Model.js @@ -382,12 +382,21 @@ export default class Model extends Observable { } } + /** + * Builds the an encoded filter query string + * @param {object} filters - The filter object + * @returns {string} The query string representation of the filter criteria + */ + buildFilterQueryString(filters) { + return `?q=${encodeURIComponent(JSON.stringify(filters))}`; + } + /** * When model change (filters), update address bar with the filter * do it silently to avoid infinite loop */ updateRouteOnModelChange() { - this.router.go(`?q=${JSON.stringify(this.log.filter.toObject())}`, true, true); + this.router.go(this.buildFilterQueryString(this.log.filter.toObject()), true, true); } /** diff --git a/InfoLogger/public/logFilter/LogFilter.js b/InfoLogger/public/logFilter/LogFilter.js index 29c45b0cc..73b2e8162 100644 --- a/InfoLogger/public/logFilter/LogFilter.js +++ b/InfoLogger/public/logFilter/LogFilter.js @@ -140,9 +140,6 @@ export default class LogFilter extends Observable { // remote empty inputs if (!criterias[field][operator]) { delete criterias[field][operator]; - } else if (operator === 'match' || operator === 'exclude') { - // encode potential breaking characters and escape double quotes as are used by browser by default - criterias[field][operator] = encodeURIComponent(criterias[field][operator].replace(/["]+/g, '\\"')); } // remove empty fields From 6eec2c67dd7c73c4bd60219aa47f0e5215f746da Mon Sep 17 00:00:00 2001 From: Isaac Hill <71404865+isaachilly@users.noreply.github.com> Date: Wed, 2 Sep 2026 11:32:45 +0200 Subject: [PATCH 2/9] Adjust filter URL encoding assertions Update InfoLogger filter action tests to match fully encoded `q` parameter. --- .../test/public/log-filter-actions-mocha.js | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/InfoLogger/test/public/log-filter-actions-mocha.js b/InfoLogger/test/public/log-filter-actions-mocha.js index cab6a8768..a89afdc51 100644 --- a/InfoLogger/test/public/log-filter-actions-mocha.js +++ b/InfoLogger/test/public/log-filter-actions-mocha.js @@ -105,7 +105,8 @@ describe('Filter actions test-suite', async () => { it('should update filters based on profile when passed in the URI', async () => { // for now check if the filters are reset once the profile is passed - const expectedParams = '?q={%22severity%22:{%22in%22:%22I%20W%20E%20F%22},%22level%22:{%22max%22:1}}'; + const expectedParams = + '?q=%7B%22severity%22%3A%7B%22in%22%3A%22I%20W%20E%20F%22%7D%2C%22level%22%3A%7B%22max%22%3A1%7D%7D'; const searchParams = await page.evaluate(() => { const params = { profile: 'physicist' }; @@ -123,7 +124,8 @@ describe('Filter actions test-suite', async () => { it('should reset filters and show warning message when profile and filters are passed', async () => { // wait until the previous notification is hidden await page.waitForFunction('window.model.notification.state === \'hidden\''); - const expectedParams = '?q={%22severity%22:{%22in%22:%22I%20W%20E%20F%22},%22level%22:{%22max%22:1}}'; + const expectedParams = + '?q=%7B%22severity%22%3A%7B%22in%22%3A%22I%20W%20E%20F%22%7D%2C%22level%22%3A%7B%22max%22%3A1%7D%7D'; const searchParams = await page.evaluate(() => { const params = { profile: 'physicist', q: '"severity":{"in":"I W E F"}}' }; window.model.parseLocation(params); @@ -148,7 +150,7 @@ describe('Filter actions test-suite', async () => { }; }); - assert.strictEqual(decodeURI(locationAndNotification.search), expectedDefaultParams); + assert.strictEqual(decodeURIComponent(locationAndNotification.search), expectedDefaultParams); assert.strictEqual(locationAndNotification.notification.type, 'danger'); // CI/CD runs on Chromium so this assertion is based on Chromium's JSON engine's error message assert.strictEqual( @@ -159,7 +161,8 @@ describe('Filter actions test-suite', async () => { it('should update URI with new encoded "match" criteria', async () => { const decodedParams = '?q={"hostname":{"match":"\\"%ald_qdip01%"},"severity":{"in":"I W E F"}}'; - const expectedParams = '?q={%22hostname%22:{%22match%22:%22%5C%22%25ald_qdip01%25%22},%22severity%22:{%22in%22:%22I%20W%20E%20F%22}}'; + const expectedParams = '?q=%7B%22hostname%22%3A%7B%22match%22%3A%22%5C%22%25ald_qdip01%25%22%7D' + + '%2C%22severity%22%3A%7B%22in%22%3A%22I%20W%20E%20F%22%7D%7D'; const searchParams = await page.evaluate(() => { window.model.log.filter.setCriteria('hostname', 'match', '"%ald_qdip01%'); window.model.updateRouteOnModelChange(); @@ -167,12 +170,13 @@ describe('Filter actions test-suite', async () => { }); assert.deepStrictEqual(searchParams, expectedParams); - assert.deepStrictEqual(decodeURI(searchParams), decodedParams); + assert.deepStrictEqual(decodeURIComponent(searchParams), decodedParams); }); it('should update URI with new encoded "exclude" criteria', async () => { const decodedParams = '?q={"hostname":{"exclude":"\\"%ald_qdip01%"},"severity":{"in":"I W E F"}}'; - const expectedParams = '?q={%22hostname%22:{%22exclude%22:%22%5C%22%25ald_qdip01%25%22},%22severity%22:{%22in%22:%22I%20W%20E%20F%22}}'; + const expectedParams = '?q=%7B%22hostname%22%3A%7B%22exclude%22%3A%22%5C%22%25ald_qdip01%25%22%7D' + + '%2C%22severity%22%3A%7B%22in%22%3A%22I%20W%20E%20F%22%7D%7D'; const searchParams = await page.evaluate(() => { window.model.log.filter.resetCriteria(); window.model.log.filter.setCriteria('hostname', 'exclude', '"%ald_qdip01%'); @@ -181,7 +185,7 @@ describe('Filter actions test-suite', async () => { }); assert.deepStrictEqual(searchParams, expectedParams); - assert.deepStrictEqual(decodeURI(searchParams), decodedParams); + assert.deepStrictEqual(decodeURIComponent(searchParams), decodedParams); }); it('should parse dates in format DD/MM/YY', async () => { From 3d569866122e67f86d18fa7216a4e298f3c483d5 Mon Sep 17 00:00:00 2001 From: Isaac Hill <71404865+isaachilly@users.noreply.github.com> Date: Wed, 2 Sep 2026 14:53:29 +0200 Subject: [PATCH 3/9] Reduce line length eslint problems in tests --- InfoLogger/test/public/log-filter-actions-mocha.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/InfoLogger/test/public/log-filter-actions-mocha.js b/InfoLogger/test/public/log-filter-actions-mocha.js index a89afdc51..a2be292af 100644 --- a/InfoLogger/test/public/log-filter-actions-mocha.js +++ b/InfoLogger/test/public/log-filter-actions-mocha.js @@ -134,7 +134,8 @@ describe('Filter actions test-suite', async () => { await page.waitForFunction('window.model.notification.state === \'shown\''); await page.waitForFunction('window.model.notification.type === \'warning\''); - await page.waitForFunction('window.model.notification.message === "URL can contain only filters or profile, not both"'); + const notificationMessage = await page.evaluate(() => window.model.notification.message); + await page.waitForFunction(`window.model.notification.message === "${notificationMessage}"`); assert.strictEqual(searchParams, expectedParams); }); @@ -152,10 +153,12 @@ describe('Filter actions test-suite', async () => { assert.strictEqual(decodeURIComponent(locationAndNotification.search), expectedDefaultParams); assert.strictEqual(locationAndNotification.notification.type, 'danger'); + const expectedMessage = 'Invalid URL filter format: Expected \',\' or \'}\'' + + ' after property value in JSON at position 27 (line 1 column 28)'; // CI/CD runs on Chromium so this assertion is based on Chromium's JSON engine's error message assert.strictEqual( locationAndNotification.notification.message, - 'Invalid URL filter format: Expected \',\' or \'}\' after property value in JSON at position 27 (line 1 column 28)', + expectedMessage, ); }); From 3fd481568cf3d831f76850e7cbb0de44c51a553c Mon Sep 17 00:00:00 2001 From: Isaac Hill <71404865+isaachilly@users.noreply.github.com> Date: Wed, 2 Sep 2026 16:13:46 +0200 Subject: [PATCH 4/9] Fix incomplete thought in updating test line length --- InfoLogger/test/public/log-filter-actions-mocha.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/InfoLogger/test/public/log-filter-actions-mocha.js b/InfoLogger/test/public/log-filter-actions-mocha.js index a2be292af..7cd94bccd 100644 --- a/InfoLogger/test/public/log-filter-actions-mocha.js +++ b/InfoLogger/test/public/log-filter-actions-mocha.js @@ -135,7 +135,7 @@ describe('Filter actions test-suite', async () => { await page.waitForFunction('window.model.notification.state === \'shown\''); await page.waitForFunction('window.model.notification.type === \'warning\''); const notificationMessage = await page.evaluate(() => window.model.notification.message); - await page.waitForFunction(`window.model.notification.message === "${notificationMessage}"`); + assert.strictEqual(notificationMessage, 'URL can contain only filters or profile, not both'); assert.strictEqual(searchParams, expectedParams); }); From 1571f1cd213efc1be68f1b737bf8316602889292 Mon Sep 17 00:00:00 2001 From: Isaac Hill <71404865+isaachilly@users.noreply.github.com> Date: Wed, 2 Sep 2026 17:05:31 +0200 Subject: [PATCH 5/9] Make test independent on conditions before it --- InfoLogger/test/public/log-filter-actions-mocha.js | 1 + 1 file changed, 1 insertion(+) diff --git a/InfoLogger/test/public/log-filter-actions-mocha.js b/InfoLogger/test/public/log-filter-actions-mocha.js index 7cd94bccd..239b178b1 100644 --- a/InfoLogger/test/public/log-filter-actions-mocha.js +++ b/InfoLogger/test/public/log-filter-actions-mocha.js @@ -167,6 +167,7 @@ describe('Filter actions test-suite', async () => { const expectedParams = '?q=%7B%22hostname%22%3A%7B%22match%22%3A%22%5C%22%25ald_qdip01%25%22%7D' + '%2C%22severity%22%3A%7B%22in%22%3A%22I%20W%20E%20F%22%7D%7D'; const searchParams = await page.evaluate(() => { + window.model.log.filter.resetCriteria(); window.model.log.filter.setCriteria('hostname', 'match', '"%ald_qdip01%'); window.model.updateRouteOnModelChange(); return window.location.search; From 0312155d8e4d2160c384462c4a0a3e8987955a37 Mon Sep 17 00:00:00 2001 From: Isaac Hill <71404865+isaachilly@users.noreply.github.com> Date: Wed, 2 Sep 2026 17:21:36 +0200 Subject: [PATCH 6/9] Add round-trip testing for the URL filters Adds round-trip tests in a separate mocha file. Removes old encoding tests. --- InfoLogger/test/mocha-index.js | 1 + .../test/public/log-filter-actions-mocha.js | 24 ------ .../test/public/log-filter-url-mocha.js | 81 +++++++++++++++++++ 3 files changed, 82 insertions(+), 24 deletions(-) create mode 100644 InfoLogger/test/public/log-filter-url-mocha.js diff --git a/InfoLogger/test/mocha-index.js b/InfoLogger/test/mocha-index.js index 273b58edc..b8e50c737 100644 --- a/InfoLogger/test/mocha-index.js +++ b/InfoLogger/test/mocha-index.js @@ -109,6 +109,7 @@ describe('InfoLogger', function () { require('./public/user-actions-mocha'); require('./public/log-filter-actions-mocha'); + require('./public/log-filter-url-mocha'); require('./public/live-mode-mocha'); require('./public/query-mode-mocha'); require('./public/status-bar-mocha'); diff --git a/InfoLogger/test/public/log-filter-actions-mocha.js b/InfoLogger/test/public/log-filter-actions-mocha.js index 239b178b1..1f8fbcef4 100644 --- a/InfoLogger/test/public/log-filter-actions-mocha.js +++ b/InfoLogger/test/public/log-filter-actions-mocha.js @@ -257,30 +257,6 @@ describe('Filter actions test-suite', async () => { assert.deepStrictEqual($in, ['I', 'W', 'E', 'F']); }); - it('should encode special characters correctly into the URL', async () => { - const pidMatch = await page.evaluate(() => { - window.model.log.filter.setCriteria('pid', 'match', 'a+b c %d #anchor & = héllo wörld 日本語'); - return window.model.log.filter.criterias.pid.$match; - }); - - assert.strictEqual(pidMatch, 'a+b c %d #anchor & = héllo wörld 日本語'); - - const searchParams = await page.evaluate(() => { - window.model.updateRouteOnModelChange(); - return window.location.search; - }); - - assert.ok(searchParams.includes('a%2Bb%20c%20%25d%20%23anchor%20%26%20%3D%20h%C3%A9llo%20w%C3%B6rld%20%E6%97%A5%E6%9C%AC%E8%AA%9E')); - }); - - it('should decode special characters correctly from the URL', async () => { - await page.goto(`${baseUrl}?q={%22pid%22:{%22match%22:%22a%2Bb%20c%20%25d%20%23anchor%20%26%20%3D%20h%C3%A9llo%20w%C3%B6rld%20%E6%97%A5%E6%9C%AC%E8%AA%9E%22}}`, { waitUntil: 'networkidle0' }); - - const pidMatch = await page.evaluate(() => window.model.log.filter.criterias.pid.$match); - - assert.strictEqual(pidMatch, 'a+b c %d #anchor & = héllo wörld 日本語'); - }); - it('should reset filters and set them again', async () => { const criterias = await page.evaluate(() => { window.model.log.filter.resetCriteria(); diff --git a/InfoLogger/test/public/log-filter-url-mocha.js b/InfoLogger/test/public/log-filter-url-mocha.js new file mode 100644 index 000000000..f97298fe0 --- /dev/null +++ b/InfoLogger/test/public/log-filter-url-mocha.js @@ -0,0 +1,81 @@ +/** + * @license + * Copyright 2019-2020 CERN and copyright holders of ALICE O2. + * See http://alice-o2.web.cern.ch/copyright for details of the copyright holders. + * All rights not expressly granted are reserved. + * + * This software is distributed under the terms of the GNU General Public + * License v3 (GPL Version 3), copied verbatim in the file "COPYING". + * + * In applying this license CERN does not waive the privileges and immunities + * granted to it by virtue of its status as an Intergovernmental Organization + * or submit itself to any jurisdiction. + */ + +const assert = require('assert'); +const test = require('../mocha-index'); + +describe('URL Encoding/Decoding Suite', async () => { + let page = null; + + before(async () => { + ({ page } = test); + await page.goto(test.helpers.baseUrl, { waitUntil: 'networkidle0' }); + }); + + describe('Filter round-trip through the URL', async () => { + /** + * Sets message match criteria, then reloads the page on the URL the model produced for it + * @param {string} value - the raw filter value to round-trip + * @returns {Promise} the value held by the model after the reload + */ + const roundTrip = async (value) => { + const url = await page.evaluate((raw) => { + window.model.log.filter.setCriteria('message', 'match', raw); + window.model.updateRouteOnModelChange(); + return window.location.href; + }, value); + + await page.goto(url, { waitUntil: 'networkidle0' }); + return await page.evaluate(() => window.model.log.filter.criterias.message.match); + }; + + it('should preserve consecutive double quotes', async () => { + // /["]+/g collapsed a run of quotes into a single escaped one, so "" came back as " + const stringToTest = 'a""b'; + assert.strictEqual(await roundTrip(stringToTest), stringToTest); + }); + + it('should preserve a backslash that forms a valid JSON escape', async () => { + // C:\temp used to reach JSON.parse unescaped and come back as C:emp + const stringToTest = 'C:\\temp'; + assert.strictEqual(await roundTrip(stringToTest), stringToTest); + }); + + it('should preserve a backslash that does not form a valid JSON escape', async () => { + // C:\xyz used to throw, resetting every filter + const stringToTest = 'C:\\xyz'; + assert.strictEqual(await roundTrip(stringToTest), stringToTest); + }); + + it('should preserve a multi-line message filter', async () => { + const stringToTest = 'first\nsecond'; + assert.strictEqual(await roundTrip(stringToTest), stringToTest); + }); + + it('should preserve a value containing URL-significant characters', async () => { + const stringToTest = 'a&b#c=d?e %20 a+b c %d #anchor & = héllo wörld 日本語'; + assert.strictEqual(await roundTrip(stringToTest), stringToTest); + }); + + it('should store the value unencoded in the model', async () => { + const stringToTest = 'a&b#c=d?e %20 a+b c %d #anchor & = héllo wörld 日本語'; + const stored = await page.evaluate((stringToTest) => { + window.model.log.filter.setCriteria('message', 'match', stringToTest); + return window.model.log.filter.toObject().message.match; + }, stringToTest); + + assert.strictEqual(stored, stringToTest); + }); + }); +}); From 7cc09d867841f1662d657c13f93484f147e518da Mon Sep 17 00:00:00 2001 From: Isaac Hill <71404865+isaachilly@users.noreply.github.com> Date: Wed, 2 Sep 2026 18:18:07 +0200 Subject: [PATCH 7/9] Move filter query string logic to LogFilter Adds a `queryString` getter on `LogFilter` that uses shared `buildUrl` utilities, and updates `Model` to rely on that property when syncing route state. --- InfoLogger/public/Model.js | 11 +---------- InfoLogger/public/logFilter/LogFilter.js | 10 +++++++++- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/InfoLogger/public/Model.js b/InfoLogger/public/Model.js index 76bc11848..15e23e99c 100644 --- a/InfoLogger/public/Model.js +++ b/InfoLogger/public/Model.js @@ -382,21 +382,12 @@ export default class Model extends Observable { } } - /** - * Builds the an encoded filter query string - * @param {object} filters - The filter object - * @returns {string} The query string representation of the filter criteria - */ - buildFilterQueryString(filters) { - return `?q=${encodeURIComponent(JSON.stringify(filters))}`; - } - /** * When model change (filters), update address bar with the filter * do it silently to avoid infinite loop */ updateRouteOnModelChange() { - this.router.go(this.buildFilterQueryString(this.log.filter.toObject()), true, true); + this.router.go(this.log.filter.queryString, true, true); } /** diff --git a/InfoLogger/public/logFilter/LogFilter.js b/InfoLogger/public/logFilter/LogFilter.js index 73b2e8162..61e0bfe7e 100644 --- a/InfoLogger/public/logFilter/LogFilter.js +++ b/InfoLogger/public/logFilter/LogFilter.js @@ -12,7 +12,7 @@ * or submit itself to any jurisdiction. */ -import { Observable } from '/js/src/index.js'; +import { Observable, buildUrl } from '/js/src/index.js'; import { TEXT_FILTER_OPERATORS } from '../constants/text-filter-operators.const.js'; import { getDisabledSeverities } from '../constants/log-level-filters.const.js'; @@ -151,6 +151,14 @@ export default class LogFilter extends Observable { return criterias; } + /** + * Builds a URI encoded filter query string + * @returns {string} The query string representation of the filter criteria + */ + get queryString() { + return buildUrl('?', { q: JSON.stringify(this.toObject()) }); + } + /** * Set criterias according to object passed as argument * @param {object} criterias - object with criterias to be set From 5e08efdbeff2113bf42f7986fda4c71d9021e689 Mon Sep 17 00:00:00 2001 From: Isaac Hill <71404865+isaachilly@users.noreply.github.com> Date: Tue, 8 Sep 2026 14:30:55 +0200 Subject: [PATCH 8/9] Remove redundant new line decoding This no longer has its corresponding encoder so it's pointless as the URL at this point will contain no `'\n` strings, they will all be percent-encoded. --- InfoLogger/public/Model.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/InfoLogger/public/Model.js b/InfoLogger/public/Model.js index 15e23e99c..fbd4d5cb3 100644 --- a/InfoLogger/public/Model.js +++ b/InfoLogger/public/Model.js @@ -371,7 +371,7 @@ export default class Model extends Observable { } else if (params.q) { this.getUserProfile(); try { - this.log.filter.fromObject(JSON.parse(params.q.replaceAll('\n', '\\n'))); + this.log.filter.fromObject(JSON.parse(params.q)); } catch (error) { this.log.filter.resetCriteria(); this.updateRouteOnModelChange(); From 150e0fe8f92cff2d5f513a1b93ab12a0de53eff8 Mon Sep 17 00:00:00 2001 From: Isaac Hill <71404865+isaachilly@users.noreply.github.com> Date: Mon, 14 Sep 2026 14:17:15 +0200 Subject: [PATCH 9/9] Refactor URL round-trip tests Consolidate the URL test cases into an array and generate the tests from it. This removes repetitive test blocks, keeps test cases in one place so it's easier to see what is being covered whilst still treating each case its own test so it's easier to debug when they fail. --- .../test/public/log-filter-url-mocha.js | 39 +++++++------------ 1 file changed, 14 insertions(+), 25 deletions(-) diff --git a/InfoLogger/test/public/log-filter-url-mocha.js b/InfoLogger/test/public/log-filter-url-mocha.js index f97298fe0..aece8c375 100644 --- a/InfoLogger/test/public/log-filter-url-mocha.js +++ b/InfoLogger/test/public/log-filter-url-mocha.js @@ -40,32 +40,21 @@ describe('URL Encoding/Decoding Suite', async () => { return await page.evaluate(() => window.model.log.filter.criterias.message.match); }; - it('should preserve consecutive double quotes', async () => { - // /["]+/g collapsed a run of quotes into a single escaped one, so "" came back as " - const stringToTest = 'a""b'; - assert.strictEqual(await roundTrip(stringToTest), stringToTest); - }); - - it('should preserve a backslash that forms a valid JSON escape', async () => { - // C:\temp used to reach JSON.parse unescaped and come back as C:emp - const stringToTest = 'C:\\temp'; - assert.strictEqual(await roundTrip(stringToTest), stringToTest); - }); - - it('should preserve a backslash that does not form a valid JSON escape', async () => { - // C:\xyz used to throw, resetting every filter - const stringToTest = 'C:\\xyz'; - assert.strictEqual(await roundTrip(stringToTest), stringToTest); - }); - - it('should preserve a multi-line message filter', async () => { - const stringToTest = 'first\nsecond'; - assert.strictEqual(await roundTrip(stringToTest), stringToTest); - }); + const roundTripTestCases = [ + // /["]+/g collapsed a run of quotes into a single escaped one, so "" came back as " + { name: 'consecutive double quotes', value: 'a""b' }, + // C:\temp used to reach JSON.parse unescaped and come back as C:emp + { name: 'a backslash forming valid JSON escape', value: 'C:\\temp' }, + // C:\xyz used to throw, resetting every filter + { name: 'a backslash not forming valid JSON escape', value: 'C:\\xyz' }, + { name: 'a multi-line message', value: 'first\nsecond' }, + { name: 'URL-significant characters', value: 'a&b#c=d?e %20 a+b c %d #anchor & = héllo wörld 日本語' }, + ]; - it('should preserve a value containing URL-significant characters', async () => { - const stringToTest = 'a&b#c=d?e %20 a+b c %d #anchor & = héllo wörld 日本語'; - assert.strictEqual(await roundTrip(stringToTest), stringToTest); + roundTripTestCases.forEach(({ name, value }) => { + it(`should preserve ${name}`, async () => { + assert.strictEqual(await roundTrip(value), value); + }); }); it('should store the value unencoded in the model', async () => {