IX adapter: not encoding bid request url

[jas-singh]

Type of issue

Bug

Description

The IX adapter (and potentially other adapters) are no longer working from release 3.16 onwards due to a Prebid framework update. We've tracked down the issue to an update made here #5092 cause of which the query string parameters in the GET request are no longer being encoded which results in malformed requests being made.

Steps to reproduce

• Configure the IX bidder with a valid configuration https://github.com/prebid/Prebid.js/blob/master/modules/ixBidAdapter.md on a test page
• Check the bid request call

Expected bid request made

https://as-sec.casalemedia.com/cygnus?s=430348&v=7.2&r=%7B%22id%22%3A%2252a75425cf918b%22%2C%22imp%22%3A%5B%7B%22id%22%3A%2265b3578b6556a5%22%2C%22banner%22%3A%7B%22w%22%3A300%2C%22h%22%3A250%2C%22topframe%22%3A1%7D%2C%22ext%22%3A%7B%22siteID%22%3A%22430348%22%2C%22sid%22%3A%22300x250%22%7D%7D%5D%2C%22site%22%3A%7B%22ref%22%3A%22https%3A%2F%2Fhideout.co%2F%22%2C%22page%22%3A%22https%3A%2F%2Fhideout.co%2Fwatch.php%3Fv%3D543952%26p%3D5553%22%7D%2C%22ext%22%3A%7B%22source%22%3A%22prebid%22%7D%2C%22regs%22%3A%7B%22ext%22%3A%7B%22gdpr%22%3A0%7D%7D%2C%22user%22%3A%7B%22ext%22%3A%7B%22consent%22%3A%22%22%7D%7D%7D&ac=j&sd=1&

Actual bid request made

https://as-sec.casalemedia.com/cygnus?t=40000&s=430347&v=7.2&r={%22id%22:%22131810d23efe1f2%22,%22imp%22:[{%22id%22:%2214a0fea136ed4e4%22,%22ext%22:{%22siteID%22:%22430347%22,%22sid%22:%22728x90%22},%22banner%22:{%22w%22:728,%22h%22:90,%22topframe%22:1}},{%22id%22:%221534bd54c909136%22,%22ext%22:{%22siteID%22:%22430348%22,%22sid%22:%22300x250%22},%22banner%22:{%22w%22:300,%22h%22:250,%22topframe%22:1}}],%22site%22:{%22page%22:%22https://hideout.co/watchPrebid3.php?v=556143&p=5553&pbjs_debug=true%22},%22ext%22:{%22source%22:%22prebid%22}}&ac=j&sd=1

Test page

https://hideout.co/watchPrebid3.php?v=556143&p=5553&pbjs_debug=true

Other information

• It was traced backed to refactoring of the function parseQueryStringParameters in https://github.com/prebid/Prebid.js/pull/5092/files#diff-2b4ca49d4bb0a774c4d4c1672d7aa781L133 where this functionality was lost

[fehza123]

Can confirm the issue as a publisher, def. will appreciate a fix on this as IX is an important partner of ours!

[bretg]

This is an item for @ix-prebid-support

[ix-prebid-support]

Thanks @bretg , we have scheduled work to address this issue within our adapter asap. Hopefully any other adapter who was relying on the Utils.js method up until 3.16 to URI encode query parameters has already updated their code as well.

[kizzard]

I added a PR to suggest rolling back this particular change as well as making the unit test better. The new function bound to parseQueryStringParameters no longer uses encodeURIComponent and could have silently broken other adapters as well as IX. If a more substantial test had been in place, it would have failed during the utils update.

What is possibly concerning is that during the utils consolidation commit, many adapter unit tests were updated to remove assertions that required trailing ampersands (which were caused by the original behavior of parseQueryStringParameters) - without thought for if adapters or endpoints might actually expect those trailing ampersands.

It might be worth thinking about whether changing unit tests for adapters is acceptable without input from the adapter maintainers.

[mkendall07]

@snapwich I didn't review the linked PR, but if this changed the behavior of the utils function I agree it should be rolled back/addressed. Thoughts?

[snapwich]

Yes, I think the PR fix is good. It's unfortunate that we have two query string creators, one that encodes values and one that doesn't, but I think they might fulfill different purposes? Seems like some places where formatQS is used encodes the entire result (rather than just the individual values), so perhaps we need both and they just need better names.

Rolling back parseQueryStringParamters is good enough for now though I think.

[snapwich]

@kizzard thanks for the update and additional tests

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.