Encode URI components in buildURL function (#1824)

* Encode URI components in buildURL function * Fix * Use buildUrl instead of URLParams * Fix tests * Fix new tests
AliceO2Group · Jan 23, 2025 · f18232c · f18232c
1 parent d342491
commit f18232c
Show file tree

Hide file tree

Showing 10 changed files with 59 additions and 27 deletions.
diff --git a/lib/public/utilities/fetch/buildUrl.js b/lib/public/utilities/fetch/buildUrl.js
@@ -90,6 +90,14 @@ export const buildUrl = (baseURL, parameters) => {
         return url;
     }
 
+    /**
+     * Sanitize a value to be used as URL parameter key or value
+     *
+     * @param {string} value the value to sanitize
+     * @return {string} the sanitized value
+     */
+    const sanitize = (value) => encodeURIComponent(decodeURIComponent(value));
+
     /**
      * Stringify a query parameter to be used in a URL and push it in the serialized query parameters list
      *
@@ -111,16 +119,16 @@ export const buildUrl = (baseURL, parameters) => {
 
         if (typeof value === 'object' && value !== null) {
             for (const [subKey, subValue] of Object.entries(value)) {
-                formatAndPushQueryParameter(`${key}[${subKey}]`, subValue);
+                formatAndPushQueryParameter(`${key}[${sanitize(subKey)}]`, subValue);
             }
             return;
         }
 
-        serializedQueryParameters.push(`${key}=${value}`);
+        serializedQueryParameters.push(`${key}=${sanitize(value)}`);
     };
 
     for (const [key, parameter] of Object.entries(parameters)) {
-        formatAndPushQueryParameter(key, parameter);
+        formatAndPushQueryParameter(sanitize(key), parameter);
     }
 
     return `${url}?${serializedQueryParameters.join('&')}`;

diff --git a/lib/public/views/Environments/Overview/EnvironmentOverviewModel.js b/lib/public/views/Environments/Overview/EnvironmentOverviewModel.js
@@ -14,6 +14,7 @@
 import { PaginationModel } from '../../../components/Pagination/PaginationModel.js';
 import { Observable, RemoteData } from '/js/src/index.js';
 import { getRemoteDataSlice } from '../../../utilities/fetch/getRemoteDataSlice.js';
+import { buildUrl } from '../../../utilities/fetch/buildUrl.js';
 
 /**
  * Environment overview page model
@@ -52,7 +53,7 @@ export class EnvironmentOverviewModel extends Observable {
             'page[limit]': this._pagination.itemsPerPage,
         };
 
-        const endpoint = `/api/environments?${new URLSearchParams(params).toString()}`;
+        const endpoint = buildUrl('/api/environments', params);
 
         try {
             const { items, totalCount } = await getRemoteDataSlice(endpoint);

diff --git a/lib/public/views/Flps/Flps.js b/lib/public/views/Flps/Flps.js
@@ -13,6 +13,7 @@
 
 import { fetchClient, Observable, RemoteData } from '/js/src/index.js';
 import { PaginationModel } from '../../components/Pagination/PaginationModel.js';
+import { buildUrl } from '../../utilities/fetch/buildUrl.js';
 
 /**
  * Model representing handlers for homePage.js
@@ -53,7 +54,7 @@ export default class Overview extends Observable {
             'page[limit]': this._pagination.itemsPerPage,
         };
 
-        const endpoint = `/api/flps?${new URLSearchParams(params).toString()}`;
+        const endpoint = buildUrl('/api/flps', params);
         const response = await fetchClient(endpoint, { method: 'GET' });
         const result = await response.json();
 

diff --git a/lib/public/views/Logs/Overview/LogsOverviewModel.js b/lib/public/views/Logs/Overview/LogsOverviewModel.js
@@ -19,6 +19,7 @@ import { FilterInputModel } from '../../../components/Filters/common/filters/Fil
 import { AuthorFilterModel } from '../../../components/Filters/LogsFilter/author/AuthorFilterModel.js';
 import { PaginationModel } from '../../../components/Pagination/PaginationModel.js';
 import { getRemoteDataSlice } from '../../../utilities/fetch/getRemoteDataSlice.js';
+import { buildUrl } from '../../../utilities/fetch/buildUrl.js';
 
 /**
  * Model representing handlers for log entries page
@@ -90,7 +91,7 @@ export class LogsOverviewModel extends Observable {
             'page[limit]': this._pagination.itemsPerPage,
         };
 
-        const endpoint = `/api/logs?${new URLSearchParams(params).toString()}`;
+        const endpoint = buildUrl('/api/logs', params);
 
         try {
             const { items, totalCount } = await getRemoteDataSlice(endpoint);

diff --git a/lib/public/views/Runs/Overview/RunsOverviewModel.js b/lib/public/views/Runs/Overview/RunsOverviewModel.js
@@ -26,6 +26,7 @@ import { TimeRangeInputModel } from '../../../components/Filters/common/filters/
 import { FloatComparisonFilterModel } from '../../../components/Filters/common/filters/FloatComparisonFilterModel.js';
 import { detectorsProvider } from '../../../services/detectors/detectorsProvider.js';
 import { AliceL3AndDipoleFilteringModel } from '../../../components/Filters/RunsFilter/AliceL3AndDipoleFilteringModel.js';
+import { buildUrl } from '../../../utilities/fetch/buildUrl.js';
 
 /**
  * Model representing handlers for runs page
@@ -107,8 +108,7 @@ export class RunsOverviewModel extends OverviewPageModel {
      * @inheritdoc
      */
     getRootEndpoint() {
-        const paramsString = new URLSearchParams(this._getFilterQueryParams()).toString();
-        return `/api/runs?${paramsString}`;
+        return buildUrl('/api/runs', this._getFilterQueryParams());
     }
 
     // eslint-disable-next-line valid-jsdoc

diff --git a/lib/public/views/Tags/Details/TagDetailsModel.js b/lib/public/views/Tags/Details/TagDetailsModel.js
@@ -20,6 +20,7 @@ import { jsonFetch } from '../../../utilities/fetch/jsonFetch.js';
 import { Observable, RemoteData } from '/js/src/index.js';
 import { tagsProvider } from '../../../services/tag/tagsProvider.js';
 import { TabbedPanelModel } from '../../../components/TabbedPanel/TabbedPanelModel.js';
+import { buildUrl } from '../../../utilities/fetch/buildUrl.js';
 
 export const TAG_DETAILS_PANELS_KEYS = {
     LOGS: 'logs',
@@ -272,7 +273,7 @@ class TagDetailsTabbedPanelModel extends TabbedPanelModel {
             };
 
             // API call to fetch logs for current tags, include the pagination params.
-            const { items, totalCount } = await getRemoteDataSlice(`/api/tags/${this._tagId}/logs?${new URLSearchParams(params).toString()}`);
+            const { items, totalCount } = await getRemoteDataSlice(buildUrl(`/api/tags/${this._tagId}/logs`, params));
 
             this._pagination.itemsCount = totalCount; // Set items count
 

diff --git a/lib/public/views/Tags/Overview/TagsOverviewModel.js b/lib/public/views/Tags/Overview/TagsOverviewModel.js
@@ -13,6 +13,7 @@
 import { Observable, RemoteData } from '/js/src/index.js';
 import { debounce } from '../../../utilities/debounce.js';
 import { getRemoteDataSlice } from '../../../utilities/fetch/getRemoteDataSlice.js';
+import { buildUrl } from '../../../utilities/fetch/buildUrl.js';
 
 /**
  * Model for tags overview page
@@ -59,7 +60,7 @@ export class TagsOverviewModel extends Observable {
             },
         };
 
-        const endpoint = `/api/tags?${new URLSearchParams(params).toString()}`;
+        const endpoint = buildUrl('/api/tags', params);
 
         try {
             const { items } = await getRemoteDataSlice(endpoint);

diff --git a/lib/utilities/buildUrl.js b/lib/utilities/buildUrl.js
@@ -90,6 +90,14 @@ exports.buildUrl = (baseURL, parameters) => {
         return url;
     }
 
+    /**
+     * Sanitize a value to be used as URL parameter key or value
+     *
+     * @param {string} value the value to sanitize
+     * @return {string} the sanitized value
+     */
+    const sanitize = (value) => encodeURIComponent(decodeURIComponent(value));
+
     /**
      * Stringify a query parameter to be used in a URL and push it in the serialized query parameters list
      *
@@ -111,16 +119,16 @@ exports.buildUrl = (baseURL, parameters) => {
 
         if (typeof value === 'object' && value !== null) {
             for (const [subKey, subValue] of Object.entries(value)) {
-                formatAndPushQueryParameter(`${key}[${subKey}]`, subValue);
+                formatAndPushQueryParameter(`${key}[${sanitize(subKey)}]`, subValue);
             }
             return;
         }
 
-        serializedQueryParameters.push(`${key}=${value}`);
+        serializedQueryParameters.push(`${key}=${sanitize(value)}`);
     };
 
     for (const [key, parameter] of Object.entries(parameters)) {
-        formatAndPushQueryParameter(key, parameter);
+        formatAndPushQueryParameter(sanitize(key), parameter);
     }
 
     return `${url}?${serializedQueryParameters.join('&')}`;

diff --git a/test/lib/utilities/buildUrl.test.js b/test/lib/utilities/buildUrl.test.js
@@ -20,13 +20,15 @@ module.exports = () => {
             simple: 'hello',
             key: {
                 nested: [1, 2],
+                '=': 12,
             },
         }).split('?');
 
         const parametersExpressions = fullParametersExpressions.split('&');
         expect(parametersExpressions).to.includes('simple=hello');
         expect(parametersExpressions).to.includes('key[nested][]=1');
         expect(parametersExpressions).to.includes('key[nested][]=2');
+        expect(parametersExpressions).to.includes('key[%3D]=12');
     });
     it('should successfully build URL by combining existing parameters', () => {
         const [, fullParametersExpressions] = buildUrl('https://example.com?simple=hello&key1[key2][]=13&key1[key2][]=35', {

diff --git a/test/public/logs/create.test.js b/test/public/logs/create.test.js
@@ -109,7 +109,7 @@ module.exports = () => {
     });
 
     it('Should successfully display the log-reply form', async () => {
-        await goToPage(page, 'log-reply&parentLogId=1');
+        await goToPage(page, 'log-reply', { queryParameters: { parentLogId: '1' } });
 
         await expectInnerText(page, 'h3', 'Reply to: First entry');
 
@@ -122,63 +122,63 @@ module.exports = () => {
     });
 
     it('Should successfully display the autofilled runs, environments and lhcFills when replying', async () => {
-        await goToPage(page, 'log-reply&parentLogId=119');
+        await goToPage(page, 'log-reply', { queryParameters: { parentLogId: '119' } });
 
         await expectInputValue(page, 'input#run-numbers', '2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22');
         await expectInputValue(page, 'input#environments', 'Dxi029djX, eZF99lH6');
         await expectInputValue(page, 'input#lhc-fills', '1, 4, 6');
     });
 
     it('Should verify that form autofill all inputs with provided full parameters.', async () => {
-        await goToPage(page, 'log-create&runNumbers=1,2,3&lhcFillNumbers=1,2,3&environmentIds=1,2,3');
+        await goToPage(page, 'log-create', { queryParameters: { runNumbers: '1,2,3', lhcFillNumbers: '1,2,3', environmentIds: '1,2,3' } });
 
         await expectInputValue(page, 'input#run-numbers', '1,2,3');
         await expectInputValue(page, 'input#environments', '1,2,3');
         await expectInputValue(page, 'input#lhc-fills', '1,2,3');
     });
 
     it('Should verify that form autofill runNumbers only when leaving other parameters empty.', async () => {
-        await goToPage(page, 'log-create&runNumbers=1,2,3');
+        await goToPage(page, 'log-create', { queryParameters: { runNumbers: '1,2,3' } });
 
         await expectInputValue(page, 'input#run-numbers', '1,2,3');
         await expectInputValue(page, 'input#environments', '');
         await expectInputValue(page, 'input#lhc-fills', '');
     });
 
     it('Should verify that form autofill environmentIds only when leaving other parameters empty.', async () => {
-        await goToPage(page, 'log-create&environmentIds=1,2,3');
+        await goToPage(page, 'log-create', { queryParameters: { environmentIds: '1,2,3' } });
 
         await expectInputValue(page, 'input#run-numbers', '');
         await expectInputValue(page, 'input#environments', '1,2,3');
         await expectInputValue(page, 'input#lhc-fills', '');
     });
 
     it('Should verify that form autofill the lhcFillNumbers only when leaving other parameters empty.', async () => {
-        await goToPage(page, 'log-create&lhcFillNumbers=1,2,3');
+        await goToPage(page, 'log-create', { queryParameters: { lhcFillNumbers: '1,2,3' } });
 
         await expectInputValue(page, 'input#run-numbers', '');
         await expectInputValue(page, 'input#environments', '');
         await expectInputValue(page, 'input#lhc-fills', '1,2,3');
     });
 
     it('Should verify that form autofill the runNumbers and environmentIds when leaving lhcFills empty.', async () => {
-        await goToPage(page, 'log-create&runNumbers=1,2,3&environmentIds=1,2,3');
+        await goToPage(page, 'log-create', { queryParameters: { runNumbers: '1,2,3', environmentIds: '1,2,3' } });
 
         await expectInputValue(page, 'input#run-numbers', '1,2,3');
         await expectInputValue(page, 'input#environments', '1,2,3');
         await expectInputValue(page, 'input#lhc-fills', '');
     });
 
     it('Should verify that form autofill the runNumbers and lhcFillNumbers when leaving environmentIds empty.', async () => {
-        await goToPage(page, 'log-create&runNumbers=1,2,3&lhcFillNumbers=1,2,3');
+        await goToPage(page, 'log-create', { queryParameters: { runNumbers: '1,2,3', lhcFillNumbers: '1,2,3' } });
 
         await expectInputValue(page, 'input#run-numbers', '1,2,3');
         await expectInputValue(page, 'input#environments', '');
         await expectInputValue(page, 'input#lhc-fills', '1,2,3');
     });
 
     it('Should verify that form autofill the environmentIds and lhcFillNumbers when leaving runNumbers empty.', async () => {
-        await goToPage(page, 'log-create&environmentIds=1,2,3&lhcFillNumbers=1,2,3');
+        await goToPage(page, 'log-create', { queryParameters: { environmentIds: '1,2,3', lhcFillNumbers: '1,2,3' } });
 
         await expectInputValue(page, 'input#run-numbers', '');
         await expectInputValue(page, 'input#environments', '1,2,3');
@@ -187,7 +187,7 @@ module.exports = () => {
 
     it('Should set the correct template when templateKey is specified.', async () => {
         const templateKey = 'on-call';
-        await goToPage(page, `log-create&templateKey=${templateKey}`);
+        await goToPage(page, 'log-create', { queryParameters: { templateKey } });
 
         await page.waitForSelector('select');
         const selectedOption = await page.evaluate(() => document.querySelector('select').value);
@@ -200,8 +200,8 @@ module.exports = () => {
         const issueDescription = 'This is a sample issue description';
         await goToPage(
             page,
-            `log-create&templateKey=${templateKey}&detectorOrSubsystem=${detectorOrSubsystem}&` +
-            `issueDescription=${issueDescription}`,
+            'log-create',
+            { queryParameters: { templateKey, detectorOrSubsystem, issueDescription } },
         );
 
         await expectInputValue(page, 'input#run-numbers', '');
@@ -214,8 +214,17 @@ module.exports = () => {
     it('Should autofill all inputs with provided full parameters.', async () => {
         await goToPage(
             page,
-            'log-create&runNumbers=1,2,3&lhcFillNumbers=1,2,3&environmentIds=1,2,3&templateKey=on-call&detectorOrSubsystem=ALL&' +
-            'issueDescription=This is a sample issue description',
+            'log-create',
+            {
+                queryParameters: {
+                    runNumbers: '1,2,3',
+                    lhcFillNumbers: '1,2,3',
+                    environmentIds: '1,2,3',
+                    templateKey: 'on-call',
+                    detectorOrSubsystem: 'ALL',
+                    issueDescription: 'This is a sample issue description',
+                },
+            },
         );
 
         await expectInputValue(page, 'input#run-numbers', '1,2,3');