Skip to content
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

Relaido Bid Adapter: Add params for hashed canonical url. #8743

Merged

Conversation

relaido
Copy link
Contributor

@relaido relaido commented Jul 27, 2022

Type of change

  • Bugfix
  • Feature
  • New bidder adapter
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Does this change affect user-facing APIs or examples documented on http://prebid.org?
  • Other

Description of change

  • Add params for hashed canonical url.
  • I use sha1, but it doesn't matter if the security is not secure.

Comment on the following review

// TODO: is 'page' the right value here?
ref: bidderRequest.refererInfo.page,

@@ -104,9 +105,10 @@ function buildRequests(validBidRequests, bidderRequest) {
uuid: getUuid(),
pv: '$prebid.version$',
imuid: imuid,
canonical_url_hash: getCanonicalUrlHash(bidderRequest.refererInfo),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do you want the url hash? why not just hash it server side?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@patmmccann
The reason for hashing is to reduce the computational load on the server side. It also has the advantage of data reduction since the value is a fixed length.

// TODO: is 'page' the right value here?
ref: bidderRequest.refererInfo.page
})
});
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you remove the todo if you confirmed this is correct

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@patmmccann
todo comment removed. Thanks for the review.

@patmmccann patmmccann self-requested a review July 27, 2022 14:00
@patmmccann patmmccann self-assigned this Jul 27, 2022
Copy link
Collaborator

@patmmccann patmmccann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious what this is for

removed review comment.
@patmmccann patmmccann merged commit 45c3312 into prebid:master Jul 28, 2022
JacobKlein26 pushed a commit to nextmillenniummedia/Prebid.js that referenced this pull request Feb 9, 2023
* add relaido adapter

* remove event listener

* fixed UserSyncs and e.data

* fix conflicts

* Add params for hashed canonical url.

* Add params for hashed canonical url.

removed review comment.

Co-authored-by: ishigami_shingo <[email protected]>
Co-authored-by: cmertv-sishigami <[email protected]>
Co-authored-by: t_bun <[email protected]>
Co-authored-by: n.maeura <[email protected]>
jorgeluisrocha pushed a commit to jwplayer/Prebid.js that referenced this pull request May 23, 2023
* add relaido adapter

* remove event listener

* fixed UserSyncs and e.data

* fix conflicts

* Add params for hashed canonical url.

* Add params for hashed canonical url.

removed review comment.

Co-authored-by: ishigami_shingo <[email protected]>
Co-authored-by: cmertv-sishigami <[email protected]>
Co-authored-by: t_bun <[email protected]>
Co-authored-by: n.maeura <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants