Formatting Query String doesn't properly encode

[JoshuaRogan]

Type of issue

Bug

Description

One of the two functions that can format a query string doesn't correctly encode values.

Two format Functions:
https://github.com/prebid/Prebid.js/blob/master/src/utils.js#L135
https://github.com/prebid/Prebid.js/blob/master/src/utils.js#L1165 --> Doesn't properly encode

Steps to reproduce

Input any URL with query params into this function as an input.

(See test page for more reproduction steps)

Test page

CodePen Extracted Example

https://codepen.io/Josh-Rogan/pen/bGOaMWr?editors=0012

In the following code pen, when a query param contains something similar to a URL with query params, it won't properly encode those into the final string. Looking at the console output description_url is not properly parsed.

Intermittent Live Test Page

This only works sometimes so may not be the best way to reproduce

Live Test Page

Use network tab to find gampad/ads? and sometimes you can see where you end up with additional query params like cust_params which are not properly within it's variable

Expected results

All query params are properly encoded.

Actual results

All query params are not encoded. In particular, if any query param value has a & it will pollute the real query params with additional query params.

Platform details

• Chrome but this can reproduced in any environment

Other information

Ideally, we should not be using multiple query string formatting functions. This would be a good opportunity to unify and potentially update to use URLSearchParams(https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams)

Quick Fix

[dgirardi]

I agree that some refactoring is warranted, but unfortunately the "wrong" version is used in many places, with many callers assuming they need to do the encoding themselves - so it cannot be simply "fixed" and I'm not sure we want to try to refactor everything (you cannot always tell from the code whether the inputs are already URL-encoded).

Do you have more details on where specifically this is a problem? Could it be fixed by switching to the alternate version?

[dgirardi]

This might be one:

Prebid.js/modules/dfpAdServerVideo.js

Lines 139 to 143 in 32c1bf6

	return buildUrl(Object.assign({
	protocol: 'https',
	host: 'securepubads.g.doubleclick.net',
	pathname: '/gampad/ads'
	}, urlComponents, { search: queryParams }));

[dgirardi]

I am not able to spot the issue from the demo page - but I'm not sure what I'm looking for. The code appears to know that it should encode cust_params:

Prebid.js/modules/dfpAdServerVideo.js

Line 104 in 6d39d8b

let encodedCustomParams = getCustParams(bid, options, urlSearchComponent && urlSearchComponent.cust_params);

[patmmccann]

Flagging jwplayer#59 as a possible 9.0 change to debate

[patmmccann]

Committee decided not to change the utility without changing all the places it was used to reflect new behavior. We're not actively planning to take on the larger scoped project as the utility is in widespread use.