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

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

Open
linux019 opened this issue Jul 2, 2024 · 8 comments
Open

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

linux019 opened this issue Jul 2, 2024 · 8 comments
Assignees

Comments

@linux019
Copy link
Contributor

linux019 commented Jul 2, 2024

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

GPPSID string `json:"gpp_sid"`

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
Copy link
Contributor

bretg commented Jul 2, 2024

@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
Copy link
Contributor

bretg commented Jul 3, 2024

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.

@Slind14
Copy link

Slind14 commented Jul 4, 2024

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
Copy link

Slind14 commented Jul 4, 2024

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
Copy link
Contributor

bretg commented Jul 8, 2024

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
Copy link

Slind14 commented Jul 8, 2024

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
Copy link
Contributor Author

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
Copy link
Contributor

bretg commented Jul 12, 2024

Assigning to @SyntaxNode

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

No branches or pull requests

6 participants