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

perBuyerSignals is being clobbered if it's a string #12079

Closed
maco opened this issue Aug 1, 2024 · 4 comments
Closed

perBuyerSignals is being clobbered if it's a string #12079

maco opened this issue Aug 1, 2024 · 4 comments

Comments

@maco
Copy link

maco commented Aug 1, 2024

Type of issue

Bug 🐞

Description

According to the PAAPI spec (see also the commit history for clarification), regarding perBuyerSignals:

All fields that accept arbitrary metadata (auctionSignals, sellerSignals, and perBuyerSignals dictionary values) must be JSON-serializable values (i.e. supported by JSON.stringify())

We're using a string, and Prebid.js doesn't check for this case, instead assuming it will be an Object in every case. This string format is intentional, as it makes it easy to use a checksum to confirm that no one has fraudulently modified our bidding data.

Steps to reproduce

Drop this test in test/spec/modules/paapi_spec.js at line 159, just after the one it's obviously based on ("should not override existing perBuyerSignals")

            it('should not override existing perBuyerSignals, even when PBS is a string', () => {
              const original = 'original';
              igb1.pbs = {
                prebid: deepClone(original)
              };
              sinon.assert.match(getBuyerAuctionConfig().perBuyerSignals[igb1.origin], {
                prebid: original
              });
            });

I believe the code that replaces the string with an object is setFPD(), when called from addPaapiConfigHook() 's if (igb && checkOrigin(igb)) block

Expected results

The string value we pass as perBuyerSignals will be returned to us as an identical string.

Actual results

Our perBuyerSignals value is lost.

@dgirardi
Copy link
Collaborator

dgirardi commented Aug 1, 2024

That test case asserts that when perBuyerSignals is an object with a property prebid that is a string, it should not be overridden by prebid's enrichments. And indeed it fails, but it's different from how I interpret your description ("perBuyerSignals is being clobbered if it's a string"). From my testing a string perBuyerSignals is left intact.

Are you losing a string in perBuyerSignals.prebid, or should I be seeing an issue when the entire signal is a string?

@patmmccann
Copy link
Collaborator

patmmccann commented Aug 2, 2024

🔨 🧶

@patmmccann
Copy link
Collaborator

patmmccann commented Aug 2, 2024

The string value we pass as perBuyerSignals will be returned to us as an identical string.

This is not a reasonable expectation, and goes against both where the google ads and openrtb teams are headed. See WICG/turtledove#1211 and https://github.com/InteractiveAdvertisingBureau/openrtb/pull/175/files#diff-407630e60468e9a631ed6267e70ab5b99b67fdd674651a555bb46225b73ffc74R380 and WICG/turtledove#1232

Publishers (prebid) and ssps may want to inject things into PBS and you'll end up with
pbs = { you: yourthing, ssp: theirthing, publisher: anotherthing, tls: onemorething }

where yourthing is the original string you respond with to the component seller (assuming you are a buyer in a component auction) and the other seven keys or values have evolving form

@maco
Copy link
Author

maco commented Aug 2, 2024

Alright, in that case I'll close this.

@maco maco closed this as completed Aug 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

3 participants