Fix/Validate URLTemplates before tracking #448
Conversation
Lotphy
force-pushed
the
PV-19335
branch
3 times, most recently
from
8037fd8
to
0ecb10b
Compare
Lotphy
changed the title
Lotphy
changed the title
Lotphy
changed the title
src/util/util.js
Outdated
| * or | ||
| * - be protocol-relative urls | ||
| * | ||
| * @param {String|String[]|Object[]} URLTemplates - A string|string[]|object[] of url templates. |
There was a problem hiding this comment.
| * @param {String|String[]|Object[]} URLTemplates - A string|string[]|object[] of url templates. | |
| * @param {Array} URLTemplates - An Array of url templates. |
It should be only this no? As we are expecting an array all the time, and also this is the input value from trackURLs
There was a problem hiding this comment.
Good one. I needed to check after the urls that are not in an array such as survey. As far as I know, this is the only case, and it's not called using trackURLs. Update inc.
src/util/util.js
Outdated
| } | ||
|
|
||
| function isValidUrl(url) { | ||
| return (url?.startsWith('http://') || url?.startsWith('https://') || url?.startsWith('//')); |
There was a problem hiding this comment.
Isn't it more performant and also more elegant with only one regex?
There was a problem hiding this comment.
Not sure, because this can make http url valid for https hosts, and I'm pretty bad at regex xD.
My first version used window.location but the testing can be limited if we mock the location to https://something for example.
return (url?.startsWith(window.location.protocol) || url?.startsWith('//'))
Description
Check and validate tracking urls before requesting them.
Issue
Provided tracking urls are not always valid urls
Type