From c070f5b60f7ed6233a7e7adfd630d6cceb2fd85c Mon Sep 17 00:00:00 2001 From: Daniel Silhavy Date: Wed, 2 Oct 2024 13:26:03 +0200 Subject: [PATCH] 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 --- src/core/Settings.js | 3 +- src/core/Utils.js | 17 ++++----- .../controllers/ContentSteeringController.js | 2 +- src/streaming/net/HTTPLoader.js | 15 +++++--- .../controllers/ProtectionController.js | 2 +- test/unit/test/core/core.Utils.js | 36 +++++++++++++++++++ 6 files changed, 56 insertions(+), 19 deletions(-) diff --git a/src/core/Settings.js b/src/core/Settings.js index 712773ec1b..f74b2ed8fa 100644 --- 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 index 7f6284fdee..61e75645e4 100644 --- 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 index 2d051b7196..ab3e3f2b3a 100644 --- 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 index 056ce14e53..efe7ec9f71 100644 --- 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 index 669904c810..bc6c22acc0 100644 --- 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 index ee5e20d861..0d942ba020 100644 --- 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', () => {