-
Notifications
You must be signed in to change notification settings - Fork 51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(FEC-8998): add url encoded for referrer token #299
Conversation
src/common/utils/kaltura-params.js
Outdated
* @return {string} - The referrer after URIComponent encoded | ||
* @private | ||
*/ | ||
function getEncodedReferrer(): string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if it's not relevant to kaltura-params.js
move it to plugins-config.js
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yairans please check approve
src/common/utils/kaltura-params.js
Outdated
*/ | ||
function getEncodedReferrer(): string { | ||
const referrer = getReferrer(); | ||
return typeof referrer === 'string' ? encodeURIComponent(referrer) : referrer; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we typeof
this because of flow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when can it be undefined or not a string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in reality according to the current implementation of getReferrer it doesn't seem possible. Still i think it is good to be sure and wrap it in case getReferrer returns undefined in some weird scenario (for example an old browser or something)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure I follow - do you know old browsers return undefined? according to MDN it is always a string: https://developer.mozilla.org/en-US/docs/Web/API/Document/referrer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed check
Description of the Changes
add new property "encodedReferrer" to plugins config model which uses a new method getEncodedReferrer in kaltura-params which encodes the getReferrer
solves FEC-8998
CheckLists