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

Consent management module has race-condition with requestBids #2564

Closed
snapwich opened this issue May 17, 2018 · 10 comments
Closed

Consent management module has race-condition with requestBids #2564

snapwich opened this issue May 17, 2018 · 10 comments

Comments

@snapwich
Copy link
Collaborator

Type of issue

Bug

Description

The GDPR hello world example has the consent management library as loading async: https://github.com/prebid/Prebid.js/blob/master/integrationExamples/gpt/gdpr_hello_world.html#L4 which is probably the correct thing to do.

However, the consent management module does not properly account for the library loading asynchronously. Here in the lookupIabConsent function (which is executed immediately on a call to requestBids): https://github.com/prebid/Prebid.js/blob/master/modules/consentManagement.js#L49

 // if it's found, directly call the CMP via it's API and call the cmpSuccess callback.
  // if it's not found, assume the prebid code may be inside an iframe and the CMP code is located in a higher parent window.
  // in this case, use the IAB's iframe locator sample code (which is slightly cutomized) to try to find the CMP and use postMessage() to communicate with the CMP.
  if (utils.isFn(window.__cmp)) {
    window.__cmp('getVendorConsents', null, cmpSuccess);
  } else if (inASafeFrame() && typeof window.$sf.ext.cmp === 'function') {
    callCmpWhileInSafeFrame();
  } else {
    callCmpWhileInIframe();
  }

It checks for the window.__cmp function (an indicator that the library has loaded) and immediately assumes if it is not found that we're either in an iFrame or SafeFrame. But there's also the possibility that the library has not loaded yet.

At this point the consent management module should wait until the library is loaded, or even more ideally, load further execution in a code block that the consent management library calls when it has loaded (similar to pbjs queue blocks) so that it can execute immediately when the CMP library has loaded.

Also, I think there needs to be better logic to detect whether we're in an iFrame or SafeFrame, because we don't want those methods to be waiting if they don't have to.

Steps to reproduce

Load the <script src="//acdn.adnxs.com/cmp/cmp.complete.bundle.js" async></script> tag using javascript in a setTimeout to defer loading for a few seconds in the gdpr_hello_world.html example.

Expected results

The auction is delayed while the consent management library loads.

Actual results

The consent module will fail inside callCmpWhileInIframe.

@mkendall07
Copy link
Member

Your right about that. Sorry we need to update the example to add the queue stuff directly example page

@snapwich
Copy link
Collaborator Author

@mkendall07 do you mean queue the prebid stuff until the consent management library has loaded? If so that could be problematic in cases like prebid express (which is how we discovered this issue) as it initiates a requestBids call as soon as the module is loaded without explicit code.

@mkendall07
Copy link
Member

No, the CMP has some queuing code that can be loaded sync to handle the race condition

@snapwich
Copy link
Collaborator Author

Okay. Not sure I quite understand, I was thinking of the consent module would have to handle the race-condition. But I might get it when I see the updated example :)

@snapwich
Copy link
Collaborator Author

snapwich commented May 23, 2018

@mkendall07 our QA is asking if there's any progress towards updating the example page. Unfortunately this issue still causes occasional test failures in our testing suite.

@mkendall07
Copy link
Member

@jsnellbaker
Do you know what needs to be updated to make this a non-race condition with the CMP?

@YOzaz
Copy link

YOzaz commented May 24, 2018

@snapwich such approach would be difficult to implement, and not sure if required - IAB recommends implementing __cmp stub as high as possible in document head: https://github.com/InteractiveAdvertisingBureau/GDPR-Transparency-and-Consent-Framework/blob/master/CMP%20JS%20API%20v1.1%20Final.md#CMP-stub-sample, which would queue __cmp calls, and loaded CMP would process them as required.
I'm not sure if AdNexus CMP has capabilities of processing such queues, but basically - example page can be updated to accomodate this.

@mkendall07
Copy link
Member

@YOzaz
That's correct.
@snapwich
This is the code that does the queueing. You should load this before Prebid.js
http://acdn.adnxs.com/cmp/docs/#/setup

@jsnellbaker
Copy link
Collaborator

I'll update the sample test page to use the full setup example denoted on the 'setup' page instead of the 'quickstart' example.

@jsnellbaker
Copy link
Collaborator

The updated code is now incorporated to the gdpr test page and merged into master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants