Prebid.js
Prebid.js copied to clipboard
Prebid Server: native ad JS event tracker support
Type of issue
Question/Feature
Description
DV360 requires support for native JS trackers as otherwise the bidding might be limited (more details here. Is there any reason why in this code there's only method 1 (img) but not also method 2? Since Prebid itself is a piece of JavaScript and hence it's safe to assume JS trackers are always supported. Or am I missing something?
I also think that should probably include 2
. There was some discussion in https://github.com/prebid/Prebid.js/pull/8086#discussion_r863957819 but I'm afraid I don't recall the full context. @musikele do you remember why we agreed just image trackers was fine?
I don't know why it's only 1
since ages, the "general" opinion is that pixels cause less harm than javascript trackers (that in theory may inject malicious code). When we discussed this topic we thought about adding 2
but then we decided to keep existing behaviour (and keep only 1
).
@musikele - is the intention of the new PBJS native approach to completely specify imp.native, or just imp.native.assets?
If all of imp.native, then seems like imp.native.eventtrackers would just be specified there.
If just just imp.native.assets, I would propose creating an s2sConfig override for the other fields in ORTB imp.native. e.g.
pbjs.setConfig({
s2sConfig: {
...
ortbNative: {. // replaces the default JSON
context: 1,
plcmttype: 1,
eventtrackers: [
{event: 1, methods: [1]} // so you can put whatever you want
]
}
}
});
@bretg ,
the new native requires users to specify the whole imp.native
request, comprising of eventtrackers
. That piece of code is just to specify defaults if any value is missing.
So, i don't think we need to change that piece of code right now... Also, not do this change in the native PR
Ok cool @musikele. I don't see changes to the pbsBidAdapter in https://github.com/prebid/Prebid.js/pull/8086 -- agree we'd need some then, right?
@jbartek25 - this is the proposed solution to what you're bringing up... the AdUnit would specify whatever event trackers desired. Sound ok?
@bretg I believe the current default for 1.2 (JS trackers disabled) is actually breaking the behaviour established with native 1.1 where jstracker
was always executed without any further checks. Now with 1.2 we are saying that JS trackers are no longer allowed by default and have to be explicitly enabled. The same threat coming from JS imp trackers existed with 1.1 and nobody complained about any malicious code before, did they? If not, then 1.2 eventtrackers don't pose any bigger risk.
From a different angle, even when an override is possible via a config, we are an SSP and don't have control over the config, this would mean educating all our publishers, regularly check their Prebid setup and getting them update their configs which is a time consuming process. Also, JS trackers are now explicitly required by DV360 and which publisher don't want demand from Google? Maybe a small minority in which case the default behaviour should cover the majority use case, not the minor one.
I don't understand this part of your statement @jbartek25 :
the current default for 1.2 (JS trackers disabled) is actually breaking the behaviour established with native 1.1 where jstracker was always executed without any further checks
Aren't we talking about this specific piece of code? There's no 1.1 anywhere.
mediaTypes['native'] = {
request: JSON.stringify({
context: 1,
plcmttype: 1,
eventtrackers: [
{event: 1, methods: [1]} // this one line of code is the focus of this issue, right?
],
assets: nativeAssets
}),
ver: '1.2' // just 1.2. always.
}
My understanding is that you're proposing this block of code get changed to add event 2:
mediaTypes['native'] = {
request: JSON.stringify({
context: 1,
plcmttype: 1,
eventtrackers: [
{event: 1, methods: [1]},
{event: 2, methods: [1]} // would adding this make everything right?
],
assets: nativeAssets
}),
ver: '1.2'
}
Right? If not, please be very explicit about what you'd like to see.
I'll coordinate a response from the relevant Prebid committees. Prebid doesn't have strong leadership in Native (obviously), so it may take some effort to get on the same page that this is a bug fix and not a breaking change.
@bretg no not event 2, but method 2 should be added, i.e.
{event: 1, methods: [1]}
should be updated to
{event: 1, methods: [1,2]}
Please see here and the ortb spec in the attached image.
Also, by mentioning 1.1 I wasn't refering to the exact code in the PBS adapter but rather to the general handling of 1.1 by Prebid: i.e. the adapters with native support put any JS tracker into nativeAd.javascriptTrackers
bid variable (example and Prebid just dropped the JS tracker in DOM here - there's no Prebid config param to enable JS trackers for native 1.1, it is always on and any JS trackers coming from bidder adapter get executed.
Ok, thanks for the correction. So the proposal is to update the pbsBidAdapter code to
mediaTypes['native'] = {
request: JSON.stringify({
context: 1,
plcmttype: 1,
eventtrackers: [
{event: 1, methods: [1,2]}
],
assets: nativeAssets
}),
ver: '1.2'
}
Dug into the history and read the words above many times. Here's my understanding of what happened:
- 3 years ago Rich and Jason created this block of JSON in the pbsBidAdapter with little fanfare. Looks to me like it may have been top-of-the-head "this works for AppNexus" without necessarily broad community discussion.
- Their decision must not have been terrible, because it took 3 years for someone to point out maybe that decision wasn't quite fully fleshed out. That's this issue.
- Michele, knowing how awesome Rich and Jason are, guessed that maybe the reasoning behind the initial JSON was sensitivity to javascript in general.
- Jozef points out that javascript sensitivity is probably not the case because other parts of Prebid.js support native javascript trackers.
So basically, the argument here is that this 3-year old JSON is missing the number 2. It was an oversight at the time, and is neither a breaking change nor a security vulnerability.
I agree that it's not a breaking change because it pubs currently using this feature would not notice any problems if they upgrade to this version.
However, not sure I agree with the security vulnerability part. In general, I do not like the idea that the pbsBidAdapter is hardcoded with this value with no way for a pub to control it. This is a special adapter that's owned by Prebid and used by thousands of publishers with no recourse. If a pub doesn't like the security behavior of an individual bid adapter, they can choose not to use that bid adapter. But pbsBidAdapter is the only game in town of its type, so if there are any publishers out there who want to control javascript trackers, they would be unable to.
So I return to the suggestion made above: make this configurable in s2sConfig. As @musikele points out, this would only be useful in the 'legacy' native scenario where adunit.native.ortb is not specified.
pbjs.setConfig({
s2sConfig: {
...
ortbNative: {. // replaces the default JSON
context: 1,
plcmttype: 1,
eventtrackers: [
{event: 1, methods: [1]}
]
}
}
});
I'm ok changing the default to include javascript trackers as long as publishers have (simple) recourse to control the behavior if they need to.
I would be careful about labeling this a security parameter; Prebid does not attempt to enforce it, and I bet neither does PBS. If any downstream DSP puts a JS tracker in the response we will run it regardless of what was in this parameter.
Fair. The general point is that hardcoded JSON is asking for trouble. There may well be other native parameters someone might want to tune. An override to a default would allow for this.
I would strongly advocate including JS Event tracker support:
eventtrackers: [
{event: 1, methods: [1,2]}
],
DV360 made JSON tracking of native required as of 7/1 so they can run their fraud script on page. From reading the initial request regarding DV360, I do think it is something buyers will expect. An overide to a default sounds like the right option.
Ok. Leaving it to @patmmccann to prioritize .
@jbartek25 your pull request is welcomed
safe to assume JS trackers are always supported
publishers may want to avoid js in native for other reasons, please make this configurable
@patmmccann
publishers may want to avoid js in native for other reasons, please make this configurable
As part of the same PR? As I understand it, it will be configurable once the native PR is done and the PR for this ticket should be just to change the default.
@jbartek25 once the native pr is merged, we would still want the legacy specification to allow this parameter to be configurable in PBS adapter? We may not want to assume the transition will be quick.
https://github.com/prebid/Prebid.js/blob/3d71e71e0d9c5fe3ecd2fc2a855224eaecea9af2/modules/prebidServerBidAdapter/index.js#L688
The above line is from the branch in the PR.
So to reiterate, what we're proposing is this:
- The new Native PR will provide a way to specify the eventtrackers when a pub changes to use adunit.native.ortb.
- We expect that most publishers will take some time to switch how native is defined in the adunit. i.e. legacy isn't going away soon
- It would be easy to create a way to override the hardcoded JSON in the pbsBidAdapter. This could prove useful for more than just this eventtrackers. The behavior is simple: if s2sConfig.ortbNative is specified, use it instead of what's currently hardcoded in pbsBidAdapter
- We can change the hardcoded value in pbsBidAdapter in PBJS 8.0
I am tackling this as part of https://github.com/prebid/Prebid.js/pull/8738, with this logic:
- use what's currently hardcoded as the default request,
const defaultRequest = { context: 1, plcmttype: 1, eventtrackers: [ {event: 1, methods: [1]} ], };
- take in overrides from s2sConfig.ortbNative,
Object.assign(defaultRequest, s2sConfig.ortbNative)
so that to signal support for js trackers (until 8.0), the pub needs to define
setConfig({
s2sConfig: {
// ....
ortbNative: {
eventtrackers: [
{event: 1, methods: [1, 2]}
],
}
}
})
I think this is slightly different from the proposal because I am not asking them to repeat context
, plcmttype
, etc if they are OK with the defaults.
Actually - reading the native spec - I am not sure that the default we now set for context
and plcmttype
make sense, so maybe it's better to discard them if s2sConfig.ortbNative
is specified?
-
context
: The context in which the ad appears (1
is "Content-centric context such as newsfeed, article, image gallery, video gallery, or similar.") -
plcmttype
: The design/format/layout of the ad unit being offered (1
is "In the feed of content - for example as an item inside the organic feed/grid/listing/carousel.")
Neither is required, so I don't know why we decided to set them, I don't think either is generally correct.
I agree, neither is correct to default to 1
I started adopting native ortb in Improve Digital adapter and unless I missed something, the adapter defaults of any native params can be currently done at ad unit level via bidRequest.mediaTypes.native.ortb
. For PBS the proposal is to allow adapter-level overrides via s2sConfig.ortbNative
. I'm wondering, shouldn't ortbNative
object be moved to the top config level to make the overrides available to all adapters, not just PBS?, i.e.:
setConfig({
// ....
ortbNative: {
eventtrackers: [
{event: 1, methods: [1, 2]}
],
}
})
The issues I see with a top-level config like ortbNative
are:
- most properties do not make sense as global config. looking at the spec, the only ones I see that do are
eventtrackers
andprivacy
. The rest are specific to the adUnit/impression. This is a problem fors2sConfig.ortbNative
as well. - all adapters would need to be updated to read the new config. Adapters that do not talk ORTB to their backend wouldn't necessarily even be able to use it.
In my opinion, it would make more sense to have a higher level control for native trackers. Something like
setConfig({
nativeTrackers: {
click: false // disable native click trackers
impression: {
js: false, // disable native js impression trackers
img: false // disable native pixel impression trackers
},
viewability: {
// maybe? I am not up to speed on how the viewability module does things
}
}
})
(I'm sure someone smarter can think of better names). Those would turn off the logic that runs the trackers, instead of setting a flag on the request (although adapters could still use it to set their request accordingly). This can be done by core without help from the adapters. It also seems more valuable from a publisher POV because it actually means "disable trackers", not "ask downstream DSPs to please not send any trackers".
I'm fine with the nativeTrackers
config object.
Those would turn off the logic that runs the trackers, instead of setting a flag on the request
It's not a good idea to not set flag on the request and just turn off the trackers in the client. This would mean discarding all the bids that include JS trackers from the auction in the client, not giving DSPs/SSPs a chance to bid with a creative that doesn't have JS tracker.