prebid / prebid-server

Open-source solution for running real-time advertising auctions in the cloud.
https://prebid.org/product-suite/prebid-server/
Apache License 2.0
431 stars 739 forks source link

Cookie sync: gpp_sid cannot be unmarshalled from array to string #3787

Open linux019 opened 4 months ago

linux019 commented 4 months ago
curl 'http://localhost:8000/cookie_sync' \
  -H 'accept: */*' \
  -H 'accept-language: en-US,en;q=0.9' \
  -H 'cache-control: no-cache' \
  -H 'content-type: text/plain' \
  -H 'origin: https://www.somesite.com' \
  -H 'pragma: no-cache' \
  -H 'priority: u=1, i' \
  -H 'referer: https://www.somesite.com/' \
  -H 'sec-ch-ua: "Not/A)Brand";v="8", "Chromium";v="126", "Google Chrome";v="126"' \
  -H 'sec-ch-ua-mobile: ?0' \
  -H 'sec-ch-ua-platform: "Windows"' \
  -H 'sec-fetch-dest: empty' \
  -H 'sec-fetch-mode: cors' \
  -H 'sec-fetch-site: cross-site' \
  -H 'user-agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/126.0.0.0 Safari/537.36' \
  --data-raw '{"uuid":"11111111-7bfa-4d8f-80ae-9d3dbd36ee74","bidders":["33across","adf","adkernel","adman","appnexus","axonix","conversant","criteo","ix","medianet","minutemedia","onetag","openx","pubmatic","pulsepoint","richaudience","rubicon","sharethrough","smartadserver","smartx","sonobi","sovrn","triplelift","undertone","unruly","vidoomy","yahooAds","yieldmo","zeta_global_ssp"],"account":"org_MGKvAuomGHjxwudZt","filterSettings":{"image":{"bidders":"*","filter":"include"},"iframe":{"bidders":"*","filter":"include"}},"gpp_sid":[7],"gpp":"DBABLA~BVQqAAAACgA.QA"}'

Error:

JSON parsing failed: cannot unmarshal endpoints.cookieSyncRequest.GPPSID: expects " or n, but found [

https://github.com/InteractiveAdvertisingBureau/openrtb2.x/blob/main/2.6.md#323---object-regs- gpp_sid should be integer array but it defined as string https://github.com/prebid/prebid-server/blob/670e1d4d0d30fa22c46a9059c68fddbde95d701c/endpoints/cookie_sync.go#L542

Should PBS support both value types - string and array? If it should has array type, there are URL templates that should be fixed too

bretg commented 4 months ago

@linux019 - where is the array value coming from besides a manual call to curl? The IAB doc states that GPP_SID as a URL parameter is a comma-separated string

Screenshot 2024-07-02 at 1 45 40 PM

The /cookie_sync endpoint will actually accept gpp_sid on either the POST body or the GET query string.

The way this is supposed to work is that the PrebidServerBidAdapter sends gpp_sid to /cookie_sync as a comma-separated string right here -- https://github.com/prebid/Prebid.js/blob/33373f036d0d42696ceecaad6ae405a55bf0010b/modules/prebidServerBidAdapter/index.js#L263

Is there a use case we're not thinking about?

bretg commented 4 months ago

Discussed in committee. Because gpp_sid can be passed on the query string for /cookie_sync and is always copied through to the query string for /setuid, we believe that gpp_sid needs to remain a comma-separated string.

https://github.com/InteractiveAdvertisingBureau/Global-Privacy-Platform/issues/85

However, we'll leave this open for a bit to understand whether there's a use case we're not thinking of.

Slind14 commented 4 months ago

Discussed in committee. Because gpp_sid can be passed on the query string for /cookie_sync and is always copied through to the query string for /setuid, we believe that gpp_sid needs to remain a comma-separated string.

InteractiveAdvertisingBureau/Global-Privacy-Platform#85

However, we'll leave this open for a bit to understand whether there's a use case we're not thinking of.

We have a case where the GPP_SID is being sent as an array from PBJS. We don't see this with other setups though. So it might be from a custom PBJS modification specific to this one setup. We will do some digging, then update. Thanks.

Update: this is from an original PBJS build for video requests.

Slind14 commented 4 months ago

Old prebid Versions are sending it as an array.

https://github.com/prebid/Prebid.js/blob/7.48.0/modules%2FprebidServerBidAdapter%2Findex.js#L265

We already patched this in our prebid server fork. With what PBJS version is this repo supposed to be compatible?

bretg commented 4 months ago

Thanks @Slind14 - 7.48 isn't even that old.

We already patched this in our prebid server fork

Perhaps submit the patch to open source? Is it just a pre-processing step at the start of /cookie_sync and /setuid ?

Slind14 commented 4 months ago

Thanks @Slind14 - 7.48 isn't even that old.

We already patched this in our prebid server fork

Perhaps submit the patch to open source? Is it just a pre-processing step at the start of /cookie_sync and /setuid ?

We wanted to ask what is expected before submitting an MR that might be rejected :)

linux019 commented 4 months ago
type GPPSIDMultiValue struct {
    isString bool
    isSlice  bool
    str      string
    slice    []int8
}

func (g *GPPSIDMultiValue) UnmarshalJSON(src []byte) error {
    if len(src) > 2 {
        if src[0] == '[' && src[len(src)-1] == ']' {

            if err := jsonutil.Unmarshal(src, &g.slice); err != nil {
                return err
            }
            g.isSlice = true
            return nil
        }

        if src[0] == '"' && src[len(src)-1] == '"' {
            g.str = string(src[1 : len(src)-1])
            g.isString = true

            return nil
        }

        return errors.New("invalid type")
    }

    if _, err := strconv.Atoi(string(src)); err != nil {
        return err
    }
    g.isString = true
    g.str = string(src)

    return nil
}

func (g *GPPSIDMultiValue) String() string {
    if g.isString {
        return g.str
    }
    if g.isSlice {
        items := make([]string, len(g.slice))
        for i, v := range g.slice {
            items[i] = strconv.Itoa(int(v))
        }
        return strings.Join(items, ",")
    }

    return ""
}

It might not looks good, it's just a draft hotfix for the old pbjs

bretg commented 4 months ago

Assigning to @SyntaxNode