Repository: prebid/prebid-server
Pull request #2070 (2021-11-03 13:39:40)

bidRequest.Source.Ext wasn't cloned in prepareSource
When adding bidder specific schains, the last bidder schain being setup was used in all requests due to the bidRequest was only shallow copied in utils.getAuctionBidderRequests.

There may very well be additional problems related to the shallow copy, but the issue with the wrong bidder schain being sent to all but one bidder is fixed with this PR.

Note I'm not a GO programmer so there may be better ways to solve the issue, so please help me out in that case.

I consider this bug to be very serious since bidder specific schains is not doing at all what it is meant to do right now.


Patch:
diff --git a/config/config.go b/config/config.go
index 5a20e257..83585146 100644
--- a/config/config.go
+++ b/config/config.go
@@ -24,6 +24,10 @@ type Configuration struct {
 	CacheClient HTTPClient `mapstructure:"http_client_cache"`
 	AdminPort   int        `mapstructure:"admin_port"`
 	EnableGzip  bool       `mapstructure:"enable_gzip"`
+	// GarbageCollectorThreshold allocates virtual memory (in bytes) which is not used by PBS but
+	// serves as a hack to trigger the garbage collector only when the heap reaches at least this size.
+	// More info: https://github.com/golang/go/issues/48409
+	GarbageCollectorThreshold int `mapstructure:"garbage_collector_threshold"`
 	// StatusResponse is the string which will be returned by the /status endpoint when things are OK.
 	// If empty, it will return a 204 with no content.
 	StatusResponse    string          `mapstructure:"status_response"`
@@ -634,6 +638,7 @@ func SetupViper(v *viper.Viper, filename string) {
 	v.SetDefault("port", 8000)
 	v.SetDefault("admin_port", 6060)
 	v.SetDefault("enable_gzip", false)
+	v.SetDefault("garbage_collector_threshold", 0)
 	v.SetDefault("status_response", "")
 	v.SetDefault("auction_timeouts_ms.default", 0)
 	v.SetDefault("auction_timeouts_ms.max", 0)
diff --git a/endpoints/openrtb2/auction.go b/endpoints/openrtb2/auction.go
index cd383c25..ec9e4152 100644
--- a/endpoints/openrtb2/auction.go
+++ b/endpoints/openrtb2/auction.go
@@ -1377,12 +1377,12 @@ func (deps *endpointDeps) processStoredRequests(ctx context.Context, requestJson
 	// Apply the Stored BidRequest, if it exists
 	resolvedRequest := requestJson
 
-	if deps.cfg.GenerateRequestID || bidRequestID == "{{UUID}}" {
+	if hasStoredBidRequest {
 		isAppRequest, err := checkIfAppRequest(requestJson)
 		if err != nil {
 			return nil, nil, []error{err}
 		}
-		if isAppRequest && hasStoredBidRequest {
+		if isAppRequest && (deps.cfg.GenerateRequestID || bidRequestID == "{{UUID}}") {
 			uuidPatch, err := generateUuidForBidRequest(deps.uuidGenerator)
 			if err != nil {
 				return nil, nil, []error{err}
@@ -1397,12 +1397,12 @@ func (deps *endpointDeps) processStoredRequests(ctx context.Context, requestJson
 				errL := storedRequestErrorChecker(requestJson, storedRequests, storedBidRequestId)
 				return nil, nil, errL
 			}
-		}
-	} else if hasStoredBidRequest {
-		resolvedRequest, err = jsonpatch.MergePatch(storedRequests[storedBidRequestId], requestJson)
-		if err != nil {
-			errL := storedRequestErrorChecker(requestJson, storedRequests, storedBidRequestId)
-			return nil, nil, errL
+		} else {
+			resolvedRequest, err = jsonpatch.MergePatch(storedRequests[storedBidRequestId], requestJson)
+			if err != nil {
+				errL := storedRequestErrorChecker(requestJson, storedRequests, storedBidRequestId)
+				return nil, nil, errL
+			}
 		}
 	}
 
diff --git a/exchange/utils.go b/exchange/utils.go
index 5a68ba45..58a38836 100644
--- a/exchange/utils.go
+++ b/exchange/utils.go
@@ -309,6 +309,9 @@ func prepareSource(req *openrtb2.BidRequest, bidder string, sChainsByBidder map[
 	// set source
 	if req.Source == nil {
 		req.Source = &openrtb2.Source{}
+	} else {
+		sourceCopy := *req.Source
+		req.Source = &sourceCopy
 	}
 	schain := openrtb_ext.ExtRequestPrebidSChain{
 		SChain: *selectedSChain,
diff --git a/main.go b/main.go
index 76fa64f7..c1038631 100644
--- a/main.go
+++ b/main.go
@@ -4,6 +4,7 @@ import (
 	"flag"
 	"math/rand"
 	"net/http"
+	"runtime"
 	"time"
 
 	"github.com/prebid/prebid-server/config"
@@ -28,6 +29,14 @@ func main() {
 		glog.Exitf("Configuration could not be loaded or did not pass validation: %v", err)
 	}
 
+	// Create a soft memory limit on the total amount of memory that PBS uses to tune the behavior
+	// of the Go garbage collector. In summary, `cfg.GarbageCollectorThreshold` serves as a fixed cost
+	// of memory that is going to be held garbage before a garbage collection cycle is triggered.
+	// This amount of virtual memory won’t translate into physical memory allocation unless we attempt
+	// to read or write to the slice below, which PBS will not do.
+	garbageCollectionThreshold := make([]byte, cfg.GarbageCollectorThreshold)
+	defer runtime.KeepAlive(garbageCollectionThreshold)
+
 	err = serve(cfg)
 	if err != nil {
 		glog.Exitf("prebid-server failed: %v", err)
diff --git a/static/bidder-info/adot.yaml b/static/bidder-info/adot.yaml
index 6a7aca06..cd2cd3c4 100644
--- a/static/bidder-info/adot.yaml
+++ b/static/bidder-info/adot.yaml
@@ -1,5 +1,6 @@
 maintainer:
   email: "admin@we-are-adot.com"
+gvlVendorID: 272
 modifyingVastXmlAllowed: false
 capabilities:
   app:
diff --git a/static/bidder-info/triplelift.yaml b/static/bidder-info/triplelift.yaml
index 99026b6f..84eea869 100644
--- a/static/bidder-info/triplelift.yaml
+++ b/static/bidder-info/triplelift.yaml
@@ -11,6 +11,13 @@ capabilities:
       - banner
       - video
 userSync:
+  # Triplelift supports user syncing but requires configuration by the host as the RedirectURL domain must be allowlisted. 
+  # Contact this bidder directly at the email address above to ask about enabling user sync.
+  # 
+  default: iframe
+  iframe:
+    url: "https://eb2.3lift.com/sync?gdpr={{.GDPR}}&cmp_cs={{.GDPRConsent}}&us_privacy={{.USPrivacy}}&redir={{.RedirectURL}}"
+    userMacro: $UID
   redirect:
     url: "https://eb2.3lift.com/getuid?gdpr={{.GDPR}}&cmp_cs={{.GDPRConsent}}&us_privacy={{.USPrivacy}}&redir={{.RedirectURL}}"
-    userMacro: "$UID"
+    userMacro: "$UID"
\ No newline at end of file
diff --git a/static/bidder-info/triplelift_native.yaml b/static/bidder-info/triplelift_native.yaml
index 32eb07ef..dc60d572 100644
--- a/static/bidder-info/triplelift_native.yaml
+++ b/static/bidder-info/triplelift_native.yaml
@@ -9,6 +9,13 @@ capabilities:
     mediaTypes:
       - native
 userSync:
+  # Triplelift supports user syncing but requires configuration by the host as the RedirectURL domain must be allowlisted. 
+  # Contact this bidder directly at the email address above to ask about enabling user sync.
+  # 
+  default: iframe
+  iframe:
+    url: "https://eb2.3lift.com/sync?gdpr={{.GDPR}}&cmp_cs={{.GDPRConsent}}&us_privacy={{.USPrivacy}}&redir={{.RedirectURL}}"
+    userMacro: $UID
   redirect:
     url: "https://eb2.3lift.com/getuid?gdpr={{.GDPR}}&cmp_cs={{.GDPRConsent}}&us_privacy={{.USPrivacy}}&redir={{.RedirectURL}}"
     userMacro: "$UID"
\ No newline at end of file


Test patch:
diff --git a/config/config_test.go b/config/config_test.go
index 9e9c7037..f86f13bd 100644
--- a/config/config_test.go
+++ b/config/config_test.go
@@ -289,6 +289,7 @@ external_url: http://prebid-server.prebid.org/
 host: prebid-server.prebid.org
 port: 1234
 admin_port: 5678
+garbage_collector_threshold: 1
 auction_timeouts_ms:
   max: 123
   default: 50
@@ -426,6 +427,7 @@ func TestFullConfig(t *testing.T) {
 	cmpStrings(t, "host", cfg.Host, "prebid-server.prebid.org")
 	cmpInts(t, "port", cfg.Port, 1234)
 	cmpInts(t, "admin_port", cfg.AdminPort, 5678)
+	cmpInts(t, "garbage_collector_threshold", cfg.GarbageCollectorThreshold, 1)
 	cmpInts(t, "auction_timeouts_ms.default", int(cfg.AuctionTimeouts.Default), 50)
 	cmpInts(t, "auction_timeouts_ms.max", int(cfg.AuctionTimeouts.Max), 123)
 	cmpStrings(t, "cache.scheme", cfg.CacheURL.Scheme, "http")
diff --git a/endpoints/openrtb2/auction_test.go b/endpoints/openrtb2/auction_test.go
index 5b234b12..8439cc65 100644
--- a/endpoints/openrtb2/auction_test.go
+++ b/endpoints/openrtb2/auction_test.go
@@ -1174,6 +1174,7 @@ func TestStoredRequestGenerateUuid(t *testing.T) {
 		givenRawData           string
 		givenGenerateRequestID bool
 		expectedID             string
+		expectedCur            string
 	}{
 		{
 			description:            "GenerateRequestID is true, rawData is an app request and has stored bid request we should generate uuid",
@@ -1217,6 +1218,20 @@ func TestStoredRequestGenerateUuid(t *testing.T) {
 			givenGenerateRequestID: false,
 			expectedID:             "ThisID",
 		},
+		{
+			description:            "Test to check that stored requests are being merged when Macro ID is present with a site rquest",
+			givenRawData:           testBidRequests[5],
+			givenGenerateRequestID: false,
+			expectedID:             "ThisID",
+			expectedCur:            "USD",
+		},
+		{
+			description:            "Test to check that stored requests are being merged when Generate Request ID flag with a site rquest",
+			givenRawData:           testBidRequests[5],
+			givenGenerateRequestID: true,
+			expectedID:             "ThisID",
+			expectedCur:            "USD",
+		},
 	}
 
 	for _, test := range testCases {
@@ -1229,6 +1244,9 @@ func TestStoredRequestGenerateUuid(t *testing.T) {
 		if err := json.Unmarshal(newRequest, req); err != nil {
 			t.Errorf("processStoredRequests Error: %s", err.Error())
 		}
+		if test.expectedCur != "" {
+			assert.Equalf(t, test.expectedCur, req.Cur[0], "The stored request wasn't merged properly: %s\n", test.description)
+		}
 		assert.Equalf(t, test.expectedID, req.ID, "The Bid Request ID is incorrect: %s\n", test.description)
 	}
 }
@@ -3185,6 +3203,7 @@ var testStoredRequestData = map[string]json.RawMessage{
 						}
 				}}
 		}`),
+	"4": json.RawMessage(`{"id": "{{UUID}}", "cur": ["USD"]}`),
 }
 
 // Stored Imp Requests
@@ -3762,6 +3781,39 @@ var testBidRequests = []string{
 			}
 		}
 	}`,
+	`{
+		"id": "ThisID",
+		"imp": [{
+			"id": "some-impression-id",
+			"banner": {
+				"format": [{
+						"w": 600,
+						"h": 500
+					},
+					{
+						"w": 300,
+						"h": 600
+					}
+				]
+			},
+			"ext": {
+				"appnexus": {
+					"placementId": 12883451
+				}
+			}
+		}],
+		"ext": {
+			"prebid": {
+				"debug": true,
+				"storedrequest": {
+					"id": "4"
+				}
+			}
+		},
+	  "site": {
+		"page": "https://example.com"
+	  }
+	}`,
 }
 
 type mockStoredReqFetcher struct {
diff --git a/exchange/utils_test.go b/exchange/utils_test.go
index ef7d0bdb..7c52c88b 100644
--- a/exchange/utils_test.go
+++ b/exchange/utils_test.go
@@ -5,12 +5,12 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"github.com/prebid/prebid-server/firstpartydata"
 	"testing"
 
 	"github.com/mxmCherry/openrtb/v15/openrtb2"
 	"github.com/prebid/prebid-server/config"
 	"github.com/prebid/prebid-server/errortypes"
+	"github.com/prebid/prebid-server/firstpartydata"
 	"github.com/prebid/prebid-server/gdpr"
 	"github.com/prebid/prebid-server/metrics"
 	"github.com/prebid/prebid-server/openrtb_ext"
@@ -2609,8 +2609,87 @@ func TestBuildXPrebidHeader(t *testing.T) {
 	}
 }
 
-func TestApplyFPD(t *testing.T) {
+func TestSourceExtSChainCopied(t *testing.T) {
+	bidRequest := newBidRequest(t)
+
+	bidderSchains := map[string]*openrtb_ext.ExtRequestPrebidSChainSChain{
+		"bidder1": {
+			Ver:      "1.0",
+			Complete: 1,
+			Nodes: []*openrtb_ext.ExtRequestPrebidSChainSChainNode{
+				{
+					ASI: "bidder1.com",
+					SID: "0001",
+					HP:  1,
+				},
+			},
+		},
+		"bidder2": {
+			Ver:      "1.0",
+			Complete: 1,
+			Nodes: []*openrtb_ext.ExtRequestPrebidSChainSChainNode{
+				{
+					ASI: "bidder2.com",
+					SID: "0002",
+					HP:  1,
+				},
+			},
+		},
+	}
+
+	copy1 := *bidRequest
+	originalTid := copy1.Source.TID
+	prepareSource(&copy1, "bidder1", bidderSchains)
+	copy2 := *bidRequest
+	copy2.Source.TID = "new TID"
+	prepareSource(&copy2, "bidder2", bidderSchains)
+
+	assert.Equal(t, json.RawMessage(`{"schain":{"complete":1,"nodes":[{"asi":"bidder1.com","sid":"0001","hp":1}],"ver":"1.0"}}`), copy1.Source.Ext, "First schain was overwritten or not set")
+	assert.Equal(t, originalTid, copy1.Source.TID, "Original TID was overwritten")
+	assert.Equal(t, json.RawMessage(`{"schain":{"complete":1,"nodes":[{"asi":"bidder2.com","sid":"0002","hp":1}],"ver":"1.0"}}`), copy2.Source.Ext, "Second schain was overwritten or not set")
+	assert.Equal(t, "new TID", copy2.Source.TID, "New TID was not set")
+}
 
+func TestCleanOpenRTBRequestsSChainMultipleBidders(t *testing.T) {
+	req := &openrtb2.BidRequest{
+		Site: &openrtb2.Site{},
+		Source: &openrtb2.Source{
+			TID: "61018dc9-fa61-4c41-b7dc-f90b9ae80e87",
+		},
+		Imp: []openrtb2.Imp{{
+			Ext: json.RawMessage(`{"appnexus": {"placementId": 1}, "axonix": { "supplyId": "123"}}`),
+		}},
+		Ext: json.RawMessage(`{"prebid":{"schains":[{ "bidders":["appnexus"],"schain":{"complete":1,"nodes":[{"asi":"directseller1.com","sid":"00001","rid":"BidRequest1","hp":1}],"ver":"1.0"}}, {"bidders":["axonix"],"schain":{"complete":1,"nodes":[{"asi":"directseller2.com","sid":"00002","rid":"BidRequest2","hp":1}],"ver":"1.0"}}]}}`),
+	}
+
+	unmarshaledExt, err := extractBidRequestExt(req)
+	assert.NoErrorf(t, err, "Error unmarshaling inExt")
+	extRequest := unmarshaledExt
+
+	auctionReq := AuctionRequest{
+		BidRequest: req,
+		UserSyncs:  &emptyUsersync{},
+	}
+	bidderToSyncerKey := map[string]string{}
+	permissions := permissionsMock{allowAllBidders: true, passGeo: true, passID: true}
+	metrics := metrics.MetricsEngineMock{}
+	bidderRequests, _, errs := cleanOpenRTBRequests(context.Background(), auctionReq, extRequest, bidderToSyncerKey, &permissions, &metrics, gdpr.SignalNo, config.Privacy{}, nil)
+
+	assert.Nil(t, errs)
+	assert.Len(t, bidderRequests, 2, "Bid request count is not 2")
+
+	bidRequestSourceExts := map[openrtb_ext.BidderName]json.RawMessage{}
+	for _, bidderRequest := range bidderRequests {
+		bidRequestSourceExts[bidderRequest.BidderName] = bidderRequest.BidRequest.Source.Ext
+	}
+
+	appnexusPrebidSchainsSchain := json.RawMessage(`{"schain":{"complete":1,"nodes":[{"asi":"directseller1.com","sid":"00001","rid":"BidRequest1","hp":1}],"ver":"1.0"}}`)
+	axonixPrebidSchainsSchain := json.RawMessage(`{"schain":{"complete":1,"nodes":[{"asi":"directseller2.com","sid":"00002","rid":"BidRequest2","hp":1}],"ver":"1.0"}}`)
+	assert.Equal(t, appnexusPrebidSchainsSchain, bidRequestSourceExts["appnexus"], "Incorrect appnexus bid request schain in source.ext")
+	assert.Equal(t, axonixPrebidSchainsSchain, bidRequestSourceExts["axonix"], "Incorrect axonix bid request schain in source.ext")
+}
+
+func TestApplyFPD(t *testing.T) {
 	fpdBidderTest := map[openrtb_ext.BidderName]*firstpartydata.ResolvedFirstPartyData{}
 	bidderTest := openrtb_ext.BidderName("test")
 	fpdBidderTest[bidderTest] = &firstpartydata.ResolvedFirstPartyData{Site: nil, App: nil, User: nil}