Fix/#4577 (#4583)

* WiP * For adding new query parameters to a URL remove the dependency to the native URL() class * Add additional unit test for checking capitalization * Fix typo * Fix typo * Fix unit tests * Add missing JSDoc
Dash-Industry-Forum · Oct 2, 2024 · c070f5b · c070f5b
1 parent 968bca5
commit c070f5b
Show file tree

Hide file tree

Showing 6 changed files with 56 additions and 19 deletions.
diff --git a/src/core/Settings.js b/src/core/Settings.js
@@ -302,6 +302,7 @@ import Events from './events/Events.js';
  *                rtpSafetyFactor: 5,
  *                mode: Constants.CMCD_MODE_QUERY,
  *                enabledKeys: ['br', 'd', 'ot', 'tb' , 'bl', 'dl', 'mtp', 'nor', 'nrr', 'su' , 'bs', 'rtp' , 'cid', 'pr', 'sf', 'sid', 'st', 'v']
+ *                includeInRequests: ['segment', 'mpd']
  *            },
  *            cmsd: {
  *                enabled: false,
@@ -1299,7 +1300,7 @@ function Settings() {
                 rtpSafetyFactor: 5,
                 mode: Constants.CMCD_MODE_QUERY,
                 enabledKeys: Constants.CMCD_AVAILABLE_KEYS,
-                includeInRequests: ['segment']
+                includeInRequests: ['segment', 'mpd']
             },
             cmsd: {
                 enabled: false,

diff --git a/src/core/Utils.js b/src/core/Utils.js
@@ -81,23 +81,18 @@ class Utils {
         return Utils.mixin(r, src, Utils.clone);
     }
 
-    static addAditionalQueryParameterToUrl(url, params) {
+    static addAdditionalQueryParameterToUrl(url, params) {
         try {
             if (!params || params.length === 0) {
                 return url;
             }
 
-            let modifiedUrl = new URL(url);
-
-            params.forEach((param) => {
-                if (param.key && param.value) {
-                    modifiedUrl.searchParams.set(param.key, param.value);
-                }
+            let updatedUrl = url;
+            params.forEach(({ key, value }) => {
+                const separator = updatedUrl.includes('?') ? '&' : '?';
+                updatedUrl += `${separator}${(encodeURIComponent(key))}=${(encodeURIComponent(value))}`;
             });
-
-            return modifiedUrl.href;
-
-
+            return updatedUrl;
         } catch (e) {
             return url;
         }

diff --git a/src/dash/controllers/ContentSteeringController.js b/src/dash/controllers/ContentSteeringController.js
@@ -326,7 +326,7 @@ function ContentSteeringController() {
             });
         }
 
-        url = Utils.addAditionalQueryParameterToUrl(url, additionalQueryParameter);
+        url = Utils.addAdditionalQueryParameterToUrl(url, additionalQueryParameter);
         return url;
     }
 

diff --git a/src/streaming/net/HTTPLoader.js b/src/streaming/net/HTTPLoader.js
@@ -573,8 +573,12 @@ function HTTPLoader(cfg) {
      * @private
      */
     function _updateRequestUrlAndHeaders(request) {
-        _updateRequestUrlAndHeadersWithCMCD(request);
+        _updateRequestUrlAndHeadersWithCmcd(request);
+        _addPathwayCloningParameters(request);
+        _addCommonAccessToken(request);
+    }
 
+    function _addPathwayCloningParameters(request) {
         // Add queryParams that came from pathway cloning
         if (request.queryParams) {
             const queryParams = Object.keys(request.queryParams).map((key) => {
@@ -583,10 +587,11 @@ function HTTPLoader(cfg) {
                     value: request.queryParams[key]
                 }
             })
-            request.url = Utils.addAditionalQueryParameterToUrl(request.url, queryParams);
+            request.url = Utils.addAdditionalQueryParameterToUrl(request.url, queryParams);
         }
+    }
 
-        // Add headers from CommonAccessToken
+    function _addCommonAccessToken(request) {
         const commonAccessToken = commonAccessTokenController.getCommonAccessTokenForUrl(request.url)
         if (commonAccessToken) {
             request.headers[Constants.COMMON_ACCESS_TOKEN_HEADER] = commonAccessToken
@@ -598,7 +603,7 @@ function HTTPLoader(cfg) {
      * @param request
      * @private
      */
-    function _updateRequestUrlAndHeadersWithCMCD(request) {
+    function _updateRequestUrlAndHeadersWithCmcd(request) {
         const currentServiceLocation = request?.serviceLocation;
         const currentAdaptationSetId = request?.mediaInfo?.id?.toString();
         const isIncludedFilters = clientDataReportingController.isServiceLocationIncluded(request.type, currentServiceLocation) &&
@@ -608,7 +613,7 @@ function HTTPLoader(cfg) {
             const cmcdMode = cmcdParameters.mode ? cmcdParameters.mode : settings.get().streaming.cmcd.mode;
             if (cmcdMode === Constants.CMCD_MODE_QUERY) {
                 const additionalQueryParameter = _getAdditionalQueryParameter(request);
-                request.url = Utils.addAditionalQueryParameterToUrl(request.url, additionalQueryParameter);
+                request.url = Utils.addAdditionalQueryParameterToUrl(request.url, additionalQueryParameter);
             } else if (cmcdMode === Constants.CMCD_MODE_HEADER) {
                 request.headers = Object.assign(request.headers, cmcdModel.getHeaderParameters(request));
             }

diff --git a/src/streaming/protection/controllers/ProtectionController.js b/src/streaming/protection/controllers/ProtectionController.js
@@ -838,7 +838,7 @@ function ProtectionController(config) {
                 });
 
                 if (cmcdParams) {
-                    request.url = Utils.addAditionalQueryParameterToUrl(request.url, [cmcdParams]);
+                    request.url = Utils.addAdditionalQueryParameterToUrl(request.url, [cmcdParams]);
                 }
             }
         }

diff --git a/test/unit/test/core/core.Utils.js b/test/unit/test/core/core.Utils.js
@@ -111,6 +111,42 @@ describe('Utils', () => {
         })
     })
 
+    describe('addAdditionalQueryParameterToUrl', () => {
+
+        it('Should escape URL with whitespaces correctly', () => {
+            const url = 'https://dash.akamaized.net/akamai/bbb_30fps/bbb_30fps.mpd?a=something with spaces';
+            const modifiedUrl = Utils.addAdditionalQueryParameterToUrl(url, [{ key: 'test', value: 'testvalue' }]);
+            expect(modifiedUrl).to.be.equal('https://dash.akamaized.net/akamai/bbb_30fps/bbb_30fps.mpd?a=something with spaces&test=testvalue');
+        })
+
+        it('Should escape URL with CMCD parameters correctly', () => {
+            const params = [{
+                key: 'CMCD',
+                value: 'bl=4000,br=14932,d=4000,dl=4000,mtp=84100,nor="bbb_30fps_3840x2160_12000k_3.m4v",ot=v,rtp=74700,sf=d,sid="4dba0bf4-e517-4b7c-b34a-d1a75206cd53",st=v,tb=14932'
+            }];
+            const url = 'https://dash.akamaized.net/akamai/bbb_30fps/bbb_30fps.mpd?a=something with spaces';
+            const modifiedUrl = Utils.addAdditionalQueryParameterToUrl(url, params);
+            expect(modifiedUrl).to.be.equal('https://dash.akamaized.net/akamai/bbb_30fps/bbb_30fps.mpd?a=something with spaces&CMCD=bl%3D4000%2Cbr%3D14932%2Cd%3D4000%2Cdl%3D4000%2Cmtp%3D84100%2Cnor%3D%22bbb_30fps_3840x2160_12000k_3.m4v%22%2Cot%3Dv%2Crtp%3D74700%2Csf%3Dd%2Csid%3D%224dba0bf4-e517-4b7c-b34a-d1a75206cd53%22%2Cst%3Dv%2Ctb%3D14932');
+        })
+
+        it('Should return the original URL if no query parameters are provided', () => {
+            const url = 'https://dash.akamaized.net/akamai/bbb_30fps/bbb_30fps.mpd?a=something with spaces';
+            const modifiedUrl = Utils.addAdditionalQueryParameterToUrl(url);
+            expect(modifiedUrl).to.be.equal('https://dash.akamaized.net/akamai/bbb_30fps/bbb_30fps.mpd?a=something with spaces');
+        })
+
+        it('Should not change capitalization of existing query parameters', () => {
+            const url = 'https://dash.akamaized.net/akamai/bbb_30fps/bbb_30fps.mpd?a=%3d';
+            const params = [{
+                key: 'CMCD',
+                value: 'bl=4000,br=14932,d=4000,dl=4000,mtp=84100,nor="bbb_30fps_3840x2160_12000k_3.m4v",ot=v,rtp=74700,sf=d,sid="4dba0bf4-e517-4b7c-b34a-d1a75206cd53",st=v,tb=14932'
+            }];
+            const modifiedUrl = Utils.addAdditionalQueryParameterToUrl(url, params);
+            expect(modifiedUrl).to.be.equal('https://dash.akamaized.net/akamai/bbb_30fps/bbb_30fps.mpd?a=%3d&CMCD=bl%3D4000%2Cbr%3D14932%2Cd%3D4000%2Cdl%3D4000%2Cmtp%3D84100%2Cnor%3D%22bbb_30fps_3840x2160_12000k_3.m4v%22%2Cot%3Dv%2Crtp%3D74700%2Csf%3Dd%2Csid%3D%224dba0bf4-e517-4b7c-b34a-d1a75206cd53%22%2Cst%3Dv%2Ctb%3D14932');
+
+        })
+    })
+
     describe('getHostFromUrl', () => {
 
         it('Should return a valid host for an http URL', () => {