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

getConfig() is returning reference to the config object #7238

Closed
lksharma opened this issue Jul 29, 2021 · 8 comments
Closed

getConfig() is returning reference to the config object #7238

lksharma opened this issue Jul 29, 2021 · 8 comments
Assignees
Labels

Comments

@lksharma
Copy link
Collaborator

lksharma commented Jul 29, 2021

Type of issue

Description

When adapters retrieve the config object by calling config.getConfig(), they are also able to manipulate the underlying object.

This is because the _getConfig() function uses Object.assign() when returning the configuration.

Since Object. assign() copies an object's reference value, we can get scenarios where one adapter updates the config and another adapter is affected.

For example:

Step 1: Config set by setConfig() on the page

pbjs.setConfig({
    ortb2: {
        user: {
            gender: “M”
        }
    }
});

Step 2: Another adapter updates the config by first calling config.getConfig()

var fpd = getConfig(‘ortb2’)
fpd.user.gender = “F”

Step 3: Downstream adapters now retrieve the manipulated config object

var fpd = getConfig(‘ortb2’)
console.log(fpd)
// Output: {ortb2: { user: {gender: “F”}}}

Currently, the Sovrn adapter retrieves the fpd by calling config.getConfig(‘ortb2’), and using that fpd object to update ortb2.user.ext for their request.

https://github.com/prebid/Prebid.js/blob/master/modules/sovrnBidAdapter.js#L86-L118

In the case where Sovrn is loaded before Index Exchange, the IX adapter will see the changes made by Sovrn

Test Page

Steps to Reproduce

  1. Verify the config object set by pbjs.setConfig()
    • You can look at the page source for this (L49-L203)
  2. Refresh the page and call the following function in the console
    • pbjs.getConfig('ortb2')
  3. Verify the returned object is not the same as the one being set at the page level.
    • Call pbjs.getConfig('ortb2') in console
    • You will see the eids & tpid being added to the configuration by the sovrn adapter.

Expected results

The _getConfig() should return a deepClone of the config object and not allow individual adapters to manipulate the config object.

Actual results

The _getConfig() function uses Object.assign() to return the stored config object, allowing adapter to manipulate data used by other downstream adapters.

Platform details

Chrome: Version 91.0.4472.164 (Official Build) (x86_64)
Mac OS: Catalina; Version: 10.15.7
prebidJS: 5.6.0-pre

Other information

Potential solution: PR #7237

@gglas
Copy link

gglas commented Aug 2, 2021

Thank you for opening this issue -- we are planning on discussing in the PMC meeting this week, as this would be a breaking change we will be strategizing on how to achieve the desired behavior.

@patmmccann
Copy link
Collaborator

Just noting that many publisher may rely on this behavior

@patmmccann
Copy link
Collaborator

@jrosendahl please fix behavior identified on line86 of your bidder

@patmmccann
Copy link
Collaborator

Discussion in prebid committee seems to indicate we should make a readConfig function now that is a deep clone, regardless of future deprecation decisions.

@jrosendahl
Copy link
Contributor

This work is in queue

@patmmccann
Copy link
Collaborator

This was discussed in the publisher committee today; some publishers may be relying on this. The sense of the group was to create a new function, allow people to migrate to it, and very slowly add warnings to and eventually deprecate getConfig

@pm-harshad-mane
Copy link
Contributor

Can we close this issue?

@umakajan
Copy link
Contributor

Closing this ticket as work is completed in #7237

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

No branches or pull requests

7 participants