fix: validate client before adding to url parameters (#4613)

* fix: validate client before adding to url parameters

validate client before adding to url parameters

* fix: correction

correction

* test: additional test

additional test

* fix: review comment

review comment

* test: adjustment

adjustment

Author: 815are

.changeset/sparkly-clocks-laugh.md

@@ -0,0 +1,5 @@
+---
+'@sap-ux-private/preview-middleware-client': patch
+---
+
+Fix sonar issue 'Ensure that tainted data is validated before being used to construct a client-side request URL.' for method 'registerComponentDependencyPaths'

packages/preview-middleware-client/src/flp/init.ts

@@ -185,9 +185,9 @@ export async function resetAppState(container: typeof sap.ushell.Container): Pro
 export async function registerComponentDependencyPaths(appUrls: string[], urlParams: URLSearchParams): Promise<void> {
     const libs = await getManifestLibs(appUrls);
     if (libs && libs.length > 0) {
-        let url = '/sap/bc/ui2/app_index/ui5_app_info?id=' + libs;
+        let url = '/sap/bc/ui2/app_index/ui5_app_info?id=' + encodeURIComponent(libs);
         const sapClient = urlParams.get('sap-client');
-        if (sapClient?.length === 3) {
+        if (sapClient?.length === 3 && /^\d+$/.test(sapClient)) {
             url = url + '&sap-client=' + sapClient;
         }
         const response = await fetch(url);

packages/preview-middleware-client/test/unit/flp/init.test.ts

@@ -97,6 +97,7 @@ describe('flp/init', () => {
 
         beforeEach(() => {
             loaderMock.mockReset();
+            fetchMock.mockReset();
         });
 
         test('single app, no reuse libs', async () => {
@@ -161,6 +162,48 @@ describe('flp/init', () => {
                 expect(error).toEqual('Error');
             }
         });
+
+        describe('test "sap-client" param validation', () => {
+            const sapClientParamTests = [
+                {
+                    name: 'Valid client',
+                    value: '001',
+                    expected: '/sap/bc/ui2/app_index/ui5_app_info?id=test.lib%2C&sap-client=001'
+                },
+                {
+                    name: 'Client contains non number',
+                    value: 'T12',
+                    expected: '/sap/bc/ui2/app_index/ui5_app_info?id=test.lib%2C'
+                },
+                {
+                    name: 'Client more than 3 symbols',
+                    value: '4444',
+                    expected: '/sap/bc/ui2/app_index/ui5_app_info?id=test.lib%2C'
+                },
+                {
+                    name: 'Client less than 3 symbols',
+                    value: '44',
+                    expected: '/sap/bc/ui2/app_index/ui5_app_info?id=test.lib%2C'
+                }
+            ];
+            test.each(sapClientParamTests)('$name', async ({ value, expected }) => {
+                const manifest = JSON.parse(JSON.stringify(testManifest)) as typeof testManifest;
+                manifest['sap.ui5'].dependencies.libs['test.lib'] = {};
+                fetchMock.mockResolvedValueOnce({ json: () => manifest });
+                fetchMock.mockResolvedValueOnce({
+                    json: () => ({
+                        'test.lib': {
+                            dependencies: [{ url: '~url', type: 'UI5LIB', componentId: 'test.lib.component' }]
+                        }
+                    })
+                });
+                const params = new URLSearchParams();
+                params.set('sap-client', value);
+                await registerComponentDependencyPaths(['/'], params);
+                expect(loaderMock).toHaveBeenCalledWith({ paths: { 'test/lib/component': '~url' } });
+                expect(fetchMock).toHaveBeenCalledWith(expected);
+            });
+        });
     });
 
     describe('resetAppState', () => {
@@ -388,7 +431,7 @@ describe('flp/init', () => {
                         callback({}); // WorkspaceConnector
                         return;
                     }
-                     
+
                     await callback(() => Promise.reject('Reload triggered'));
                     resolve(undefined);
                 });