Skip to content
This repository has been archived by the owner on Dec 22, 2022. It is now read-only.

Commit

Permalink
limit the size of the uids cookie (prebid#1004)
Browse files Browse the repository at this point in the history
* some research

* Modifications and test cases in place

* Hans' corrections and random expiration date to cookies in test case

* Cookie size gets defined in configmap.yaml now

* removed MaxCookieSizeBytes duplicate

* removed unnecessary constant declaration

* Set unlimited size as default, added unlimited size test case

* corrected test case

* symplified solution by pulling size from Configuration struct itself

* substituted time.Sub() for time.Until() and tuned adapter test cases

* removed vim .swp files

* MIN_COOKIE_SIZE_BYTES implemented in config.go

* correct SetCookieOnResponse() return value

* Printed MIN_COOKIE_SIZE_BYTES in error message, added one more test case in usersync/cookie_test.go

* Resolved conflict in config/config.go

* Resolved another conflict
  • Loading branch information
guscarreon authored and hhhjort committed Sep 10, 2019
1 parent a6b6332 commit 3131f40
Show file tree
Hide file tree
Showing 14 changed files with 141 additions and 27 deletions.
2 changes: 1 addition & 1 deletion adapters/adform/adform_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ func preparePrebidRequest(serverUrl string, t *testing.T) *pbs.PBSRequest {
pbsCookie := usersync.ParsePBSCookieFromRequest(prebidHttpRequest, &config.HostCookie{})
pbsCookie.TrySync("adform", adformTestData.buyerUID)
fakeWriter := httptest.NewRecorder()
pbsCookie.SetCookieOnResponse(fakeWriter, "", time.Minute)
pbsCookie.SetCookieOnResponse(fakeWriter, &config.HostCookie{Domain: ""}, time.Minute)
prebidHttpRequest.Header.Add("Cookie", fakeWriter.Header().Get("Set-Cookie"))

cacheClient, _ := dummycache.New()
Expand Down
2 changes: 1 addition & 1 deletion adapters/appnexus/appnexus_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ func TestAppNexusBasicResponse(t *testing.T) {
pc := usersync.ParsePBSCookieFromRequest(req, &config.HostCookie{})
pc.TrySync("adnxs", andata.buyerUID)
fakewriter := httptest.NewRecorder()
pc.SetCookieOnResponse(fakewriter, "", 90*24*time.Hour)
pc.SetCookieOnResponse(fakewriter, &config.HostCookie{Domain: ""}, 90*24*time.Hour)
req.Header.Add("Cookie", fakewriter.Header().Get("Set-Cookie"))

cacheClient, _ := dummycache.New()
Expand Down
2 changes: 1 addition & 1 deletion adapters/audienceNetwork/facebook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ func GenerateBidRequestForTestData(fbdata bidInfo, url string) (*pbs.PBSRequest,
pc := usersync.ParsePBSCookieFromRequest(req, &config.HostCookie{})
pc.TrySync("audienceNetwork", fbdata.buyerUID)
fakewriter := httptest.NewRecorder()
pc.SetCookieOnResponse(fakewriter, "", 90*24*time.Hour)
pc.SetCookieOnResponse(fakewriter, &config.HostCookie{Domain: ""}, 90*24*time.Hour)
req.Header.Add("Cookie", fakewriter.Header().Get("Set-Cookie"))

cacheClient, _ := dummycache.New()
Expand Down
2 changes: 1 addition & 1 deletion adapters/lifestreet/lifestreet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ func TestLifestreetBasicResponse(t *testing.T) {

pc := usersync.ParsePBSCookieFromRequest(req, &config.HostCookie{})
fakewriter := httptest.NewRecorder()
pc.SetCookieOnResponse(fakewriter, "", 90*24*time.Hour)
pc.SetCookieOnResponse(fakewriter, &config.HostCookie{Domain: ""}, 90*24*time.Hour)
req.Header.Add("Cookie", fakewriter.Header().Get("Set-Cookie"))

cacheClient, _ := dummycache.New()
Expand Down
2 changes: 1 addition & 1 deletion adapters/pubmatic/pubmatic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -656,7 +656,7 @@ func TestPubmaticSampleRequest(t *testing.T) {
pc := usersync.ParsePBSCookieFromRequest(httpReq, &config.HostCookie{})
pc.TrySync("pubmatic", "12345")
fakewriter := httptest.NewRecorder()
pc.SetCookieOnResponse(fakewriter, "", 90*24*time.Hour)
pc.SetCookieOnResponse(fakewriter, &config.HostCookie{Domain: ""}, 90*24*time.Hour)
httpReq.Header.Add("Cookie", fakewriter.Header().Get("Set-Cookie"))

cacheClient, _ := dummycache.New()
Expand Down
2 changes: 1 addition & 1 deletion adapters/pulsepoint/pulsepoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ func SampleRequest(numberOfImpressions int, t *testing.T) *pbs.PBSRequest {
pc := usersync.ParsePBSCookieFromRequest(httpReq, &config.HostCookie{})
pc.TrySync("pulsepoint", "pulsepointUser123")
fakewriter := httptest.NewRecorder()
pc.SetCookieOnResponse(fakewriter, "", 90*24*time.Hour)
pc.SetCookieOnResponse(fakewriter, &config.HostCookie{Domain: ""}, 90*24*time.Hour)
httpReq.Header.Add("Cookie", fakewriter.Header().Get("Set-Cookie"))
// parse the http request
cacheClient, _ := dummycache.New()
Expand Down
2 changes: 1 addition & 1 deletion adapters/rubicon/rubicon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -945,7 +945,7 @@ func CreatePrebidRequest(server *httptest.Server, t *testing.T) (an *RubiconAdap
pc := usersync.ParsePBSCookieFromRequest(req, &config.HostCookie{})
pc.TrySync("rubicon", rubidata.buyerUID)
fakewriter := httptest.NewRecorder()
pc.SetCookieOnResponse(fakewriter, "", 90*24*time.Hour)
pc.SetCookieOnResponse(fakewriter, &config.HostCookie{Domain: ""}, 90*24*time.Hour)
req.Header.Add("Cookie", fakewriter.Header().Get("Set-Cookie"))

cacheClient, _ := dummycache.New()
Expand Down
2 changes: 1 addition & 1 deletion adapters/sovrn/sovrn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ func SampleSovrnRequest(numberOfImpressions int, t *testing.T) *pbs.PBSRequest {
pc := usersync.ParsePBSCookieFromRequest(httpReq, &config.HostCookie{})
pc.TrySync("sovrn", testSovrnUserId)
fakewriter := httptest.NewRecorder()
pc.SetCookieOnResponse(fakewriter, "", 90*24*time.Hour)
pc.SetCookieOnResponse(fakewriter, &config.HostCookie{Domain: ""}, 90*24*time.Hour)
httpReq.Header.Add("Cookie", fakewriter.Header().Get("Set-Cookie"))
// parse the http request
cacheClient, _ := dummycache.New()
Expand Down
32 changes: 25 additions & 7 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ type Configuration struct {
BlacklistedAcctMap map[string]bool
}

const MIN_COOKIE_SIZE_BYTES = 500

type HTTPClient struct {
MaxIdleConns int `mapstructure:"max_idle_connections"`
MaxIdleConnsPerHost int `mapstructure:"max_idle_connections_per_host"`
Expand Down Expand Up @@ -175,12 +177,13 @@ type FileLogs struct {
}

type HostCookie struct {
Domain string `mapstructure:"domain"`
Family string `mapstructure:"family"`
CookieName string `mapstructure:"cookie_name"`
OptOutURL string `mapstructure:"opt_out_url"`
OptInURL string `mapstructure:"opt_in_url"`
OptOutCookie Cookie `mapstructure:"optout_cookie"`
Domain string `mapstructure:"domain"`
Family string `mapstructure:"family"`
CookieName string `mapstructure:"cookie_name"`
OptOutURL string `mapstructure:"opt_out_url"`
OptInURL string `mapstructure:"opt_in_url"`
MaxCookieSizeBytes int `mapstructure:"max_cookie_size_bytes"`
OptOutCookie Cookie `mapstructure:"optout_cookie"`
// Cookie timeout in days
TTL int64 `mapstructure:"ttl_days"`
}
Expand Down Expand Up @@ -389,7 +392,7 @@ func New(v *viper.Viper) (*Configuration, error) {
}
c.setDerivedDefaults()

// To look for a request's publisher_id into the NonStandardPublishers in
// To look for a request's publisher_id in the NonStandardPublishers list in
// O(1) time, we fill this hash table located in the NonStandardPublisherMap field of GDPR
c.GDPR.NonStandardPublisherMap = make(map[string]int)
for i := 0; i < len(c.GDPR.NonStandardPublishers); i++ {
Expand All @@ -410,6 +413,11 @@ func New(v *viper.Viper) (*Configuration, error) {
c.BlacklistedAcctMap[c.BlacklistedAccts[i]] = true
}

if err := isValidCookieSize(c.HostCookie.MaxCookieSizeBytes); err != nil {
glog.Fatal(fmt.Printf("Max cookie size %d cannot be less than %d \n", c.HostCookie.MaxCookieSizeBytes, MIN_COOKIE_SIZE_BYTES))
return nil, err
}

glog.Info("Logging the resolved configuration:")
logGeneral(reflect.ValueOf(c), " \t")
if errs := c.validate(); len(errs) > 0 {
Expand Down Expand Up @@ -525,6 +533,7 @@ func SetupViper(v *viper.Viper, filename string) {
v.SetDefault("host_cookie.optout_cookie.name", "")
v.SetDefault("host_cookie.value", "")
v.SetDefault("host_cookie.ttl_days", 90)
v.SetDefault("host_cookie.max_cookie_size_bytes", 0)
v.SetDefault("http_client.max_idle_connections", 400)
v.SetDefault("http_client.max_idle_connections_per_host", 10)
v.SetDefault("http_client.idle_connection_timeout_seconds", 60)
Expand Down Expand Up @@ -683,3 +692,12 @@ func setBidderDefaults(v *viper.Viper, bidder string) {
v.SetDefault(adapterCfgPrefix+bidder+".disabled", false)
v.SetDefault(adapterCfgPrefix+bidder+".partner_id", "")
}

func isValidCookieSize(maxCookieSize int) error {
// If a non-zero-less-than-500-byte "host_cookie.max_cookie_size_bytes" value was specified in the
// environment configuration of prebid-server, default to 500 bytes
if maxCookieSize != 0 && maxCookieSize < MIN_COOKIE_SIZE_BYTES {
return fmt.Errorf("Configured cookie size is less than allowed minimum size of %d \n", MIN_COOKIE_SIZE_BYTES)
}
return nil
}
25 changes: 25 additions & 0 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package config

import (
"bytes"
"fmt"
"strings"
"testing"
"time"
Expand All @@ -22,6 +23,7 @@ func TestDefaults(t *testing.T) {
cmpInts(t, "auction_timeouts_ms.max", int(cfg.AuctionTimeouts.Max), 0)
cmpInts(t, "max_request_size", int(cfg.MaxRequestSize), 1024*256)
cmpInts(t, "host_cookie.ttl_days", int(cfg.HostCookie.TTL), 90)
cmpInts(t, "host_cookie.max_cookie_size_bytes", cfg.HostCookie.MaxCookieSizeBytes, 0)
cmpStrings(t, "datacache.type", cfg.DataCache.Type, "dummy")
cmpStrings(t, "adapters.pubmatic.endpoint", cfg.Adapters[string(openrtb_ext.BidderPubmatic)].Endpoint, "http://hbopenbid.pubmatic.com/translator?source=prebid-server")
cmpInts(t, "currency_converter.fetch_interval_seconds", cfg.CurrencyConverter.FetchIntervalSeconds, 1800)
Expand All @@ -39,6 +41,7 @@ host_cookie:
domain: cookies.prebid.org
opt_out_url: http://prebid.org/optout
opt_in_url: http://prebid.org/optin
max_cookie_size_bytes: 32768
external_url: http://prebid-server.prebid.org/
host: prebid-server.prebid.org
port: 1234
Expand Down Expand Up @@ -300,6 +303,28 @@ func TestLimitTimeout(t *testing.T) {
doTimeoutTest(t, 15, 0, 20, 15)
}

func TestCookieSizeError(t *testing.T) {
type aTest struct {
cookieHost *HostCookie
expectError bool
}
testCases := []aTest{
{cookieHost: &HostCookie{MaxCookieSizeBytes: 1 << 15}, expectError: false}, //32 KB, no error
{cookieHost: &HostCookie{MaxCookieSizeBytes: 800}, expectError: false},
{cookieHost: &HostCookie{MaxCookieSizeBytes: 500}, expectError: false},
{cookieHost: &HostCookie{MaxCookieSizeBytes: 0}, expectError: false},
{cookieHost: &HostCookie{MaxCookieSizeBytes: 200}, expectError: true},
{cookieHost: &HostCookie{MaxCookieSizeBytes: -100}, expectError: true},
}
for i := range testCases {
if testCases[i].expectError {
assert.Error(t, isValidCookieSize(testCases[i].cookieHost.MaxCookieSizeBytes), fmt.Sprintf("Configuration.HostCooki.MaxCookieSizeBytes less than MIN_COOKIE_SIZE_BYTES = %d and not equal to zero should return an error", MIN_COOKIE_SIZE_BYTES))
} else {
assert.NoError(t, isValidCookieSize(testCases[i].cookieHost.MaxCookieSizeBytes), fmt.Sprintf("Configuration.HostCooki.MaxCookieSizeBytes greater than MIN_COOKIE_SIZE_BYTES = %d or equal to zero should not return an error", MIN_COOKIE_SIZE_BYTES))
}
}
}

func newDefaultConfig(t *testing.T) *Configuration {
v := viper.New()
SetupViper(v, "")
Expand Down
2 changes: 1 addition & 1 deletion endpoints/setuid.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func NewSetUIDEndpoint(cfg config.HostCookie, perms gdpr.Permissions, pbsanalyti
so.Success = true
}

pc.SetCookieOnResponse(w, cfg.Domain, cookieTTL)
pc.SetCookieOnResponse(w, &cfg, cookieTTL)
})
}

Expand Down
2 changes: 1 addition & 1 deletion pbs/usersync.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func (deps *UserSyncDeps) OptOut(w http.ResponseWriter, r *http.Request, _ httpr
pc := usersync.ParsePBSCookieFromRequest(r, deps.HostCookieConfig)
pc.SetPreference(optout == "")

pc.SetCookieOnResponse(w, deps.HostCookieConfig.Domain, deps.HostCookieConfig.TTLDuration())
pc.SetCookieOnResponse(w, deps.HostCookieConfig, deps.HostCookieConfig.TTLDuration())
if optout == "" {
http.Redirect(w, r, deps.HostCookieConfig.OptInURL, 301)
} else {
Expand Down
24 changes: 23 additions & 1 deletion usersync/cookie.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"encoding/base64"
"encoding/json"
"errors"
"math"
"net/http"
"time"

Expand Down Expand Up @@ -160,11 +161,32 @@ func (cookie *PBSCookie) GetId(bidderName openrtb_ext.BidderName) (id string, ex
}

// SetCookieOnResponse is a shortcut for "ToHTTPCookie(); cookie.setDomain(domain); setCookie(w, cookie)"
func (cookie *PBSCookie) SetCookieOnResponse(w http.ResponseWriter, domain string, ttl time.Duration) {
func (cookie *PBSCookie) SetCookieOnResponse(w http.ResponseWriter, cfg *config.HostCookie, ttl time.Duration) {
httpCookie := cookie.ToHTTPCookie(ttl)
var domain string = cfg.Domain

if domain != "" {
httpCookie.Domain = domain
}

var currSize int = len([]byte(httpCookie.String()))
for cfg.MaxCookieSizeBytes > 0 && currSize > cfg.MaxCookieSizeBytes && len(cookie.uids) > 0 {
var oldestElem string = ""
var oldestDate int64 = math.MaxInt64
for key, value := range cookie.uids {
timeUntilExpiration := time.Until(value.Expires)
if timeUntilExpiration < time.Duration(oldestDate) {
oldestElem = key
oldestDate = int64(timeUntilExpiration)
}
}
delete(cookie.uids, oldestElem)
httpCookie = cookie.ToHTTPCookie(ttl)
if domain != "" {
httpCookie.Domain = domain
}
currSize = len([]byte(httpCookie.String()))
}
http.SetCookie(w, httpCookie)
}

Expand Down
67 changes: 58 additions & 9 deletions usersync/cookie_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func TestBidderNameGets(t *testing.T) {
func TestRejectAudienceNetworkCookie(t *testing.T) {
raw := &PBSCookie{
uids: map[string]uidWithExpiry{
"audienceNetwork": newTempId("0"),
"audienceNetwork": newTempId("0", 10),
},
optOut: false,
birthday: timestamp(),
Expand Down Expand Up @@ -162,7 +162,7 @@ func TestParseOtherCookie(t *testing.T) {
func TestCookieReadWrite(t *testing.T) {
cookie := newSampleCookie()

received := writeThenRead(cookie)
received := writeThenRead(cookie, 0)
uid, exists, isLive := received.GetUID("adnxs")
if !exists || !isLive || uid != "123" {
t.Errorf("Received cookie should have the adnxs ID=123. Got %s", uid)
Expand Down Expand Up @@ -262,6 +262,37 @@ func TestGetUIDsWithNilCookie(t *testing.T) {
assert.Len(t, uids, 0, "GetUIDs shouldn't return any user syncs for a nil cookie")
}

func TestTrimCookiesClosestExpirationDates(t *testing.T) {
cookieToSend, cookieToSendLen := newTestCookie()
closestToExpirationDate := "key7"

type aTest struct {
maxCookieSize int
expAction string
}
testCases := []aTest{
{maxCookieSize: 2000, expAction: "equal"}, //1 don't trim, set
{maxCookieSize: 0, expAction: "equal"}, //2 unlimited size: don't trim, set
{maxCookieSize: 800, expAction: "trim"}, //3 trim to size and set
{maxCookieSize: 500, expAction: "trim"}, //4 trim to size and set
{maxCookieSize: 200, expAction: "empty"}, //5 insufficient size, trim to zero lenght and set
{maxCookieSize: -100, expAction: "empty"}, //6 invalid size, trim to zero lenght and set
}
for i := range testCases {
processedCookie := writeThenRead(cookieToSend, testCases[i].maxCookieSize)
switch testCases[i].expAction {
case "equal":
assert.Equal(t, cookieToSendLen, len(processedCookie.uids), "[Test %d] MaxCookieSizeBytes equal to zero or bigger than %d bytes should be enough to set and remain cookie unchanged \n", i+1, len(processedCookie.uids))
assert.Containsf(t, processedCookie.uids, closestToExpirationDate, "[Test %d] Oldest entry in cookie should not have been eliminated", i+1)
case "trim":
assert.Equal(t, cookieToSendLen > len(processedCookie.uids), true, "[Test %d] MaxCookieSizeBytes of %d is smaller than %d bytes and cookie entries should have been removed\n", i+1, testCases[i].maxCookieSize, cookieToSendLen)
assert.NotContainsf(t, processedCookie.uids, closestToExpirationDate, "[Test %d] Oldest entry in cookie should not have been eliminated", i+1)
case "empty":
assert.Equal(t, len(processedCookie.uids), 0, "[Test %d] MaxCookieSizeBytes of %d is too small, processedCookie.uids should be empty\n", i+1)
}
}
}

func ensureEmptyMap(t *testing.T, cookie *PBSCookie) {
if !cookie.AllowSyncs() {
t.Error("Empty cookies should allow user syncs.")
Expand Down Expand Up @@ -333,31 +364,49 @@ func ensureConsistency(t *testing.T, cookie *PBSCookie) {
}
}

func newTempId(uid string) uidWithExpiry {
func newTempId(uid string, offset int) uidWithExpiry {
return uidWithExpiry{
UID: uid,
Expires: time.Now().Add(10 * time.Minute),
Expires: time.Now().Add(time.Duration(offset) * time.Minute),
}
}

func newSampleCookie() *PBSCookie {
return &PBSCookie{
uids: map[string]uidWithExpiry{
"adnxs": newTempId("123"),
"rubicon": newTempId("456"),
"adnxs": newTempId("123", 10),
"rubicon": newTempId("456", 10),
},
optOut: false,
birthday: timestamp(),
}
}

func newTestCookie() (*PBSCookie, int) {
var mediumSizeCookie *PBSCookie = &PBSCookie{
uids: map[string]uidWithExpiry{
"key1": newTempId("12345678901234567890123456789012345678901234567890", 7),
"key2": newTempId("abcdefghijklmnopqrstuvwxyz", 6),
"key3": newTempId("ABCDEFGHIJKLMNOPQRSTUVWXYZ", 6),
"key4": newTempId("12345678901234567890123456789612345678901234567890", 5),
"key5": newTempId("aAbBcCdDeEfFgGhHiIjJkKlLmMnNoOpPqQrRsStTuUvVwWxXyYzZ", 4),
"key6": newTempId("12345678901234567890123456789012345678901234567890", 3),
"key7": newTempId("abcdefghijklmnopqrstuvwxyz", 2),
},
optOut: false,
birthday: timestamp(),
}
return mediumSizeCookie, len(mediumSizeCookie.uids)
}

func writeThenRead(cookie *PBSCookie) *PBSCookie {
func writeThenRead(cookie *PBSCookie, maxCookieSize int) *PBSCookie {
w := httptest.NewRecorder()
cookie.SetCookieOnResponse(w, "mock-domain", 90*24*time.Hour)
hostCookie := &config.HostCookie{Domain: "mock-domain", MaxCookieSizeBytes: maxCookieSize}
cookie.SetCookieOnResponse(w, hostCookie, 90*24*time.Hour)
writtenCookie := w.HeaderMap.Get("Set-Cookie")

header := http.Header{}
header.Add("Cookie", writtenCookie)
request := http.Request{Header: header}
return ParsePBSCookieFromRequest(&request, &config.HostCookie{})
return ParsePBSCookieFromRequest(&request, hostCookie)
}

0 comments on commit 3131f40

Please sign in to comment.