From df58baff01da6946cd7f075a10790cd1c52ea1f0 Mon Sep 17 00:00:00 2001 From: Brian Sardo <1168933+bsardo@users.noreply.github.com> Date: Tue, 29 Oct 2024 21:05:07 -0400 Subject: [PATCH] Cookie Sync: Use max when limit is 0 (#4022) --- endpoints/cookie_sync.go | 45 ++++-- endpoints/cookie_sync_test.go | 258 ++++++++++++++++++++++++---------- 2 files changed, 216 insertions(+), 87 deletions(-) diff --git a/endpoints/cookie_sync.go b/endpoints/cookie_sync.go index 42b67aa551b..8f0a6a12d59 100644 --- a/endpoints/cookie_sync.go +++ b/endpoints/cookie_sync.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io" + "math" "net/http" "strconv" "strings" @@ -174,6 +175,11 @@ func (c *cookieSyncEndpoint) parseRequest(r *http.Request) (usersync.Request, ma tcf2Cfg := c.privacyConfig.tcf2ConfigBuilder(c.privacyConfig.gdprConfig.TCF2, account.GDPR) gdprPerms := c.privacyConfig.gdprPermissionsBuilder(tcf2Cfg, gdprRequestInfo) + limit := math.MaxInt + if request.Limit != nil { + limit = *request.Limit + } + rx := usersync.Request{ Bidders: request.Bidders, Cooperative: usersync.Cooperative{ @@ -181,7 +187,7 @@ func (c *cookieSyncEndpoint) parseRequest(r *http.Request) (usersync.Request, ma PriorityGroups: c.config.UserSync.PriorityGroups, }, Debug: request.Debug, - Limit: request.Limit, + Limit: limit, Privacy: usersyncPrivacy{ gdprPermissions: gdprPerms, ccpaParsedPolicy: ccpaParsedPolicy, @@ -285,17 +291,38 @@ func (c *cookieSyncEndpoint) writeParseRequestErrorMetrics(err error) { } func (c *cookieSyncEndpoint) setLimit(request cookieSyncRequest, cookieSyncConfig config.CookieSync) cookieSyncRequest { - if request.Limit <= 0 && cookieSyncConfig.DefaultLimit != nil { - request.Limit = *cookieSyncConfig.DefaultLimit + limit := getEffectiveLimit(request.Limit, cookieSyncConfig.DefaultLimit) + maxLimit := getEffectiveMaxLimit(cookieSyncConfig.MaxLimit) + if maxLimit < limit { + request.Limit = &maxLimit + } else { + request.Limit = &limit } - if cookieSyncConfig.MaxLimit != nil && (request.Limit <= 0 || request.Limit > *cookieSyncConfig.MaxLimit) { - request.Limit = *cookieSyncConfig.MaxLimit + return request +} + +func getEffectiveLimit(reqLimit *int, defaultLimit *int) int { + limit := reqLimit + + if limit == nil { + limit = defaultLimit } - if request.Limit < 0 { - request.Limit = 0 + + if limit != nil && *limit > 0 { + return *limit } - return request + return math.MaxInt +} + +func getEffectiveMaxLimit(maxLimit *int) int { + limit := maxLimit + + if limit != nil && *limit > 0 { + return *limit + } + + return math.MaxInt } func (c *cookieSyncEndpoint) setCooperativeSync(request cookieSyncRequest, cookieSyncConfig config.CookieSync) cookieSyncRequest { @@ -537,7 +564,7 @@ type cookieSyncRequest struct { GDPR *int `json:"gdpr"` GDPRConsent string `json:"gdpr_consent"` USPrivacy string `json:"us_privacy"` - Limit int `json:"limit"` + Limit *int `json:"limit"` GPP string `json:"gpp"` GPPSID string `json:"gpp_sid"` CooperativeSync *bool `json:"coopSync"` diff --git a/endpoints/cookie_sync_test.go b/endpoints/cookie_sync_test.go index 0ae30e60be8..2b85f232b6e 100644 --- a/endpoints/cookie_sync_test.go +++ b/endpoints/cookie_sync_test.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io" + "math" "net/http" "net/http/httptest" "strconv" @@ -692,6 +693,7 @@ func TestCookieSyncParseRequest(t *testing.T) { givenCCPAEnabled: true, expectedPrivacy: macros.UserSyncPrivacy{}, expectedRequest: usersync.Request{ + Limit: math.MaxInt, Privacy: usersyncPrivacy{ gdprPermissions: &fakePermissions{}, activityRequest: emptyActivityPoliciesRequest, @@ -720,6 +722,7 @@ func TestCookieSyncParseRequest(t *testing.T) { Enabled: true, PriorityGroups: [][]string{{"a", "b", "c"}}, }, + Limit: math.MaxInt, Privacy: usersyncPrivacy{ gdprPermissions: &fakePermissions{}, activityRequest: emptyActivityPoliciesRequest, @@ -748,6 +751,7 @@ func TestCookieSyncParseRequest(t *testing.T) { Enabled: false, PriorityGroups: [][]string{{"a", "b", "c"}}, }, + Limit: math.MaxInt, Privacy: usersyncPrivacy{ gdprPermissions: &fakePermissions{}, activityRequest: emptyActivityPoliciesRequest, @@ -776,6 +780,7 @@ func TestCookieSyncParseRequest(t *testing.T) { Enabled: false, PriorityGroups: [][]string{{"a", "b", "c"}}, }, + Limit: math.MaxInt, Privacy: usersyncPrivacy{ gdprPermissions: &fakePermissions{}, activityRequest: emptyActivityPoliciesRequest, @@ -804,6 +809,7 @@ func TestCookieSyncParseRequest(t *testing.T) { Enabled: false, PriorityGroups: [][]string{{"a", "b", "c"}}, }, + Limit: math.MaxInt, Privacy: usersyncPrivacy{ gdprPermissions: &fakePermissions{}, activityRequest: emptyActivityPoliciesRequest, @@ -832,6 +838,7 @@ func TestCookieSyncParseRequest(t *testing.T) { Enabled: true, PriorityGroups: [][]string{{"a", "b", "c"}}, }, + Limit: math.MaxInt, Privacy: usersyncPrivacy{ gdprPermissions: &fakePermissions{}, activityRequest: emptyActivityPoliciesRequest, @@ -860,6 +867,7 @@ func TestCookieSyncParseRequest(t *testing.T) { Enabled: true, PriorityGroups: [][]string{{"a", "b", "c"}}, }, + Limit: math.MaxInt, Privacy: usersyncPrivacy{ gdprPermissions: &fakePermissions{}, activityRequest: emptyActivityPoliciesRequest, @@ -878,6 +886,7 @@ func TestCookieSyncParseRequest(t *testing.T) { givenCCPAEnabled: true, expectedPrivacy: macros.UserSyncPrivacy{}, expectedRequest: usersync.Request{ + Limit: math.MaxInt, Privacy: usersyncPrivacy{ gdprPermissions: &fakePermissions{}, activityRequest: emptyActivityPoliciesRequest, @@ -898,6 +907,7 @@ func TestCookieSyncParseRequest(t *testing.T) { USPrivacy: "1NYN", }, expectedRequest: usersync.Request{ + Limit: math.MaxInt, Privacy: usersyncPrivacy{ gdprPermissions: &fakePermissions{}, activityRequest: emptyActivityPoliciesRequest, @@ -939,6 +949,7 @@ func TestCookieSyncParseRequest(t *testing.T) { GDPR: "0", }, expectedRequest: usersync.Request{ + Limit: math.MaxInt, Privacy: usersyncPrivacy{ gdprPermissions: &fakePermissions{}, activityRequest: emptyActivityPoliciesRequest, @@ -966,6 +977,7 @@ func TestCookieSyncParseRequest(t *testing.T) { GDPR: "", }, expectedRequest: usersync.Request{ + Limit: math.MaxInt, Privacy: usersyncPrivacy{ gdprPermissions: &fakePermissions{}, activityRequest: emptyActivityPoliciesRequest, @@ -1135,152 +1147,242 @@ func TestCookieSyncParseRequest(t *testing.T) { } } -func TestSetLimit(t *testing.T) { - intNegative1 := -1 - int20 := 20 - int30 := 30 - int40 := 40 +func TestGetEffectiveLimit(t *testing.T) { + intNegative := ptrutil.ToPtr(-1) + int0 := ptrutil.ToPtr(0) + int30 := ptrutil.ToPtr(30) + int40 := ptrutil.ToPtr(40) + intMax := ptrutil.ToPtr(math.MaxInt) - testCases := []struct { - description string - givenRequest cookieSyncRequest - givenAccount *config.Account - expectedRequest cookieSyncRequest + tests := []struct { + name string + reqLimit *int + defaultLimit *int + expectedLimit int }{ { - description: "Default Limit is Applied (request limit = 0)", - givenRequest: cookieSyncRequest{ - Limit: 0, - }, - givenAccount: &config.Account{ - CookieSync: config.CookieSync{ - DefaultLimit: &int20, - }, - }, - expectedRequest: cookieSyncRequest{ - Limit: 20, - }, + name: "nil", + reqLimit: nil, + defaultLimit: nil, + expectedLimit: math.MaxInt, }, { - description: "Default Limit is Not Applied (default limit not set)", - givenRequest: cookieSyncRequest{ - Limit: 0, - }, - givenAccount: &config.Account{ - CookieSync: config.CookieSync{ - DefaultLimit: nil, - }, - }, - expectedRequest: cookieSyncRequest{ - Limit: 0, - }, + name: "req_limit_negative", + reqLimit: intNegative, + defaultLimit: nil, + expectedLimit: math.MaxInt, }, { - description: "Default Limit is Not Applied (request limit > 0)", - givenRequest: cookieSyncRequest{ - Limit: 10, - }, - givenAccount: &config.Account{ - CookieSync: config.CookieSync{ - DefaultLimit: &int20, - }, - }, - expectedRequest: cookieSyncRequest{ - Limit: 10, - }, + name: "req_limit_zero", + reqLimit: int0, + defaultLimit: nil, + expectedLimit: math.MaxInt, + }, + { + name: "req_limit_in_range", + reqLimit: int30, + defaultLimit: nil, + expectedLimit: 30, + }, + { + name: "req_limit_at_max", + reqLimit: intMax, + defaultLimit: nil, + expectedLimit: math.MaxInt, + }, + { + name: "default_limit_negative", + reqLimit: nil, + defaultLimit: intNegative, + expectedLimit: math.MaxInt, + }, + { + name: "default_limit_zero", + reqLimit: nil, + defaultLimit: intNegative, + expectedLimit: math.MaxInt, + }, + { + name: "default_limit_in_range", + reqLimit: nil, + defaultLimit: int30, + expectedLimit: 30, + }, + { + name: "default_limit_at_max", + reqLimit: nil, + defaultLimit: intMax, + expectedLimit: math.MaxInt, + }, + { + name: "both_in_range", + reqLimit: int30, + defaultLimit: int40, + expectedLimit: 30, }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + result := getEffectiveLimit(test.reqLimit, test.defaultLimit) + assert.Equal(t, test.expectedLimit, result) + }) + } +} + +func TestGetEffectiveMaxLimit(t *testing.T) { + intNegative := ptrutil.ToPtr(-1) + int0 := ptrutil.ToPtr(0) + int30 := ptrutil.ToPtr(30) + intMax := ptrutil.ToPtr(math.MaxInt) + + tests := []struct { + name string + maxLimit *int + expectedLimit int + }{ + { + name: "nil", + maxLimit: nil, + expectedLimit: math.MaxInt, + }, + { + name: "req_limit_negative", + maxLimit: intNegative, + expectedLimit: math.MaxInt, + }, + { + name: "req_limit_zero", + maxLimit: int0, + expectedLimit: math.MaxInt, + }, + { + name: "req_limit_in_range", + maxLimit: int30, + expectedLimit: 30, + }, + { + name: "req_limit_too_large", + maxLimit: intMax, + expectedLimit: math.MaxInt, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + result := getEffectiveMaxLimit(test.maxLimit) + assert.Equal(t, test.expectedLimit, result) + }) + } +} + +func TestSetLimit(t *testing.T) { + intNegative := ptrutil.ToPtr(-1) + int0 := ptrutil.ToPtr(0) + int10 := ptrutil.ToPtr(10) + int20 := ptrutil.ToPtr(20) + int30 := ptrutil.ToPtr(30) + intMax := ptrutil.ToPtr(math.MaxInt) + + tests := []struct { + name string + givenRequest cookieSyncRequest + givenAccount *config.Account + expectedRequest cookieSyncRequest + }{ { - description: "Max Limit is Applied (request limit <= 0)", + name: "nil_limits", givenRequest: cookieSyncRequest{ - Limit: 0, + Limit: nil, }, givenAccount: &config.Account{ CookieSync: config.CookieSync{ - MaxLimit: &int30, + DefaultLimit: nil, + MaxLimit: nil, }, }, expectedRequest: cookieSyncRequest{ - Limit: 30, + Limit: intMax, }, }, { - description: "Max Limit is Applied (0 < max < limit)", + name: "limit_negative", givenRequest: cookieSyncRequest{ - Limit: 40, + Limit: intNegative, }, givenAccount: &config.Account{ CookieSync: config.CookieSync{ - MaxLimit: &int30, + DefaultLimit: int20, }, }, expectedRequest: cookieSyncRequest{ - Limit: 30, + Limit: intMax, }, }, { - description: "Max Limit is Not Applied (max not set)", + name: "limit_zero", givenRequest: cookieSyncRequest{ - Limit: 10, + Limit: int0, }, givenAccount: &config.Account{ CookieSync: config.CookieSync{ - MaxLimit: nil, + DefaultLimit: int20, }, }, expectedRequest: cookieSyncRequest{ - Limit: 10, + Limit: intMax, }, }, { - description: "Max Limit is Not Applied (0 < limit < max)", + name: "limit_less_than_max", givenRequest: cookieSyncRequest{ - Limit: 10, + Limit: int10, }, givenAccount: &config.Account{ CookieSync: config.CookieSync{ - MaxLimit: &int30, + DefaultLimit: int20, + MaxLimit: int30, }, }, expectedRequest: cookieSyncRequest{ - Limit: 10, + Limit: int10, }, }, { - description: "Max Limit is Applied After applying the default", + name: "limit_greater_than_max", givenRequest: cookieSyncRequest{ - Limit: 0, + Limit: int30, }, givenAccount: &config.Account{ CookieSync: config.CookieSync{ - DefaultLimit: &int40, - MaxLimit: &int30, + DefaultLimit: int20, + MaxLimit: int10, }, }, expectedRequest: cookieSyncRequest{ - Limit: 30, + Limit: int10, }, }, { - description: "Negative Value Check", + name: "limit_at_max", givenRequest: cookieSyncRequest{ - Limit: 0, + Limit: intMax, }, givenAccount: &config.Account{ - CookieSync: config.CookieSync{ - DefaultLimit: &intNegative1, - MaxLimit: &intNegative1, - }, + CookieSync: config.CookieSync{}, }, expectedRequest: cookieSyncRequest{ - Limit: 0, + Limit: intMax, }, }, } - for _, test := range testCases { - endpoint := cookieSyncEndpoint{} - request := endpoint.setLimit(test.givenRequest, test.givenAccount.CookieSync) - assert.Equal(t, test.expectedRequest, request, test.description) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + endpoint := cookieSyncEndpoint{} + request := endpoint.setLimit(test.givenRequest, test.givenAccount.CookieSync) + assert.Equal(t, test.expectedRequest, request) + }) } }