diff --git a/config/account.go b/config/account.go index 2d2bfb5cd83..29e3fb43827 100644 --- a/config/account.go +++ b/config/account.go @@ -54,6 +54,7 @@ type Account struct { Privacy AccountPrivacy `mapstructure:"privacy" json:"privacy"` PreferredMediaType openrtb_ext.PreferredMediaType `mapstructure:"preferredmediatype" json:"preferredmediatype"` TargetingPrefix string `mapstructure:"targeting_prefix" json:"targeting_prefix"` + StoredRequest AccountStoredRequest `mapstructure:"stored_request" json:"stored_request"` } // CookieSync represents the account-level defaults for the cookie sync endpoint. @@ -395,3 +396,18 @@ func (ip *IPv4) Validate(errs []error) []error { } return errs } + +// AccountStoredRequest represents account-specific stored request configuration +type AccountStoredRequest struct { + ArrayMerge ArrayMergeMode `mapstructure:"array_merge" json:"array_merge"` +} + +// ArrayMergeMode defines how array fields are merged during stored request processing +// "replace" (default): Arrays are replaced, RFC 7386 behavior +// "concat": Arrays are concatenated +type ArrayMergeMode string + +const ( + ArrayMergeReplace ArrayMergeMode = "replace" + ArrayMergeConcat ArrayMergeMode = "concat" +) diff --git a/config/config.go b/config/config.go index e8f23b4ace0..295290ee6cc 100644 --- a/config/config.go +++ b/config/config.go @@ -1228,6 +1228,7 @@ func SetupViper(v *viper.Viper, filename string, bidderInfos BidderInfos) { v.SetDefault("account_defaults.privacy.privacysandbox.topicsdomain", "") v.SetDefault("account_defaults.privacy.privacysandbox.cookiedeprecation.enabled", false) v.SetDefault("account_defaults.privacy.privacysandbox.cookiedeprecation.ttl_sec", 604800) + v.SetDefault("account_defaults.stored_request.array_merge", "replace") v.SetDefault("account_defaults.events_enabled", false) v.BindEnv("account_defaults.privacy.dsa.default") diff --git a/config/config_test.go b/config/config_test.go index 7d7ebe91018..94d61c074db 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -225,6 +225,7 @@ func TestDefaults(t *testing.T) { cmpStrings(t, "account_defaults.privacy.topicsdomain", "", cfg.AccountDefaults.Privacy.PrivacySandbox.TopicsDomain) cmpBools(t, "account_defaults.privacy.privacysandbox.cookiedeprecation.enabled", false, cfg.AccountDefaults.Privacy.PrivacySandbox.CookieDeprecation.Enabled) cmpInts(t, "account_defaults.privacy.privacysandbox.cookiedeprecation.ttl_sec", 604800, cfg.AccountDefaults.Privacy.PrivacySandbox.CookieDeprecation.TTLSec) + cmpStrings(t, "account_defaults.stored_request.array_merge", "replace", string(cfg.AccountDefaults.StoredRequest.ArrayMerge)) cmpBools(t, "account_defaults.events.enabled", false, cfg.AccountDefaults.Events.Enabled) @@ -579,6 +580,8 @@ account_defaults: cookiedeprecation: enabled: true ttl_sec: 86400 + stored_request: + array_merge: "replace" tmax_adjustments: enabled: true bidder_response_duration_min_ms: 700 @@ -726,6 +729,7 @@ func TestFullConfig(t *testing.T) { cmpInts(t, "account_defaults.price_floors.fetch.period_sec", 2000, cfg.AccountDefaults.PriceFloors.Fetcher.Period) cmpInts(t, "account_defaults.price_floors.fetch.max_age_sec", 6000, cfg.AccountDefaults.PriceFloors.Fetcher.MaxAge) cmpInts(t, "account_defaults.price_floors.fetch.max_schema_dims", 10, cfg.AccountDefaults.PriceFloors.Fetcher.MaxSchemaDims) + cmpStrings(t, "account_defaults.stored_request.array_merge", "replace", string(cfg.AccountDefaults.StoredRequest.ArrayMerge)) assert.Equal(t, RoundingModeUp, cfg.AccountDefaults.BidRounding) diff --git a/endpoints/openrtb2/auction.go b/endpoints/openrtb2/auction.go index 0eaf0e302eb..ed416637318 100644 --- a/endpoints/openrtb2/auction.go +++ b/endpoints/openrtb2/auction.go @@ -532,7 +532,7 @@ func (deps *endpointDeps) parseRequest(httpRequest *http.Request, labels *metric } // Fetch the Stored Request data and merge it into the HTTP request. - if requestJson, impExtInfoMap, errs = deps.processStoredRequests(requestJson, impInfo, storedRequests, storedImps, storedBidRequestId, hasStoredBidRequest); len(errs) > 0 { + if requestJson, impExtInfoMap, errs = deps.processStoredRequests(requestJson, impInfo, storedRequests, storedImps, storedBidRequestId, hasStoredBidRequest, account); len(errs) > 0 { return } @@ -1689,7 +1689,69 @@ func (deps *endpointDeps) getStoredRequests(ctx context.Context, requestJson []b return storedBidRequestId, hasStoredBidRequest, storedRequests, storedImps, errs } -func (deps *endpointDeps) processStoredRequests(requestJson []byte, impInfo []ImpExtPrebidData, storedRequests map[string]json.RawMessage, storedImps map[string]json.RawMessage, storedBidRequestId string, hasStoredBidRequest bool) ([]byte, map[string]exchange.ImpExtInfo, []error) { +// mergeWithArrayConcat performs JSON merge with array concatenation for specific fields. +// For the specified arrayFields (e.g., "bcat", "badv"), arrays are concatenated +// All other fields follow RFC 7386 JSON Merge Patch semantics. +// Empty arrays inside the patch request will clear the base request arrays (preserving RFC 7386 semantics). +func mergeWithArrayConcat(base, patch []byte, arrayFields []string) ([]byte, error) { + if len(base) == 0 { + return patch, nil + } + if len(patch) == 0 { + return base, nil + } + + // Parse both JSON documents + var baseObj, patchObj map[string]interface{} + if err := jsonutil.UnmarshalValid(base, &baseObj); err != nil { + return nil, fmt.Errorf("failed to unmarshal base request: %w", err) + } + if err := jsonutil.UnmarshalValid(patch, &patchObj); err != nil { + return nil, fmt.Errorf("failed to unmarshal patch request: %w", err) + } + + // For each array field, concat if both exist and are non-empty arrays, otherwise preserve RFC 7386 semantics + for _, field := range arrayFields { + baseVal, ok := baseObj[field] + if !ok { + continue + } + patchVal, ok := patchObj[field] + if !ok { + continue + } + + baseArr, ok := baseVal.([]interface{}) + if !ok { + continue + } + patchArr, ok := patchVal.([]interface{}) + if !ok { + continue + } + + if len(patchArr) > 0 && len(baseArr) > 0 { + combined := make([]interface{}, len(baseArr)+len(patchArr)) + copy(combined, baseArr) + copy(combined[len(baseArr):], patchArr) + patchObj[field] = combined + } + } + + // Perform RFC 7386 merge on the modified objects + baseBytes, err := jsonutil.Marshal(baseObj) + if err != nil { + return nil, fmt.Errorf("failed to marshal base object: %w", err) + } + patchBytes, err := jsonutil.Marshal(patchObj) + if err != nil { + return nil, fmt.Errorf("failed to marshal patch object: %w", err) + } + + return jsonpatch.MergePatch(baseBytes, patchBytes) +} + +func (deps *endpointDeps) processStoredRequests(requestJson []byte, impInfo []ImpExtPrebidData, storedRequests map[string]json.RawMessage, storedImps map[string]json.RawMessage, storedBidRequestId string, hasStoredBidRequest bool, account *config.Account) ([]byte, map[string]exchange.ImpExtInfo, []error) { bidRequestID, err := getBidRequestID(storedRequests[storedBidRequestId]) if err != nil { return nil, nil, []error{err} @@ -1698,6 +1760,14 @@ func (deps *endpointDeps) processStoredRequests(requestJson []byte, impInfo []Im // Apply the Stored BidRequest, if it exists resolvedRequest := requestJson + useConcatMode := account != nil && account.StoredRequest.ArrayMerge == config.ArrayMergeConcat + merge := func(base, patch []byte) ([]byte, error) { + if useConcatMode { + return mergeWithArrayConcat(base, patch, []string{"bcat", "badv"}) + } + return jsonpatch.MergePatch(base, patch) + } + if hasStoredBidRequest { isAppRequest, err := checkIfAppRequest(requestJson) if err != nil { @@ -1713,13 +1783,14 @@ func (deps *endpointDeps) processStoredRequests(requestJson []byte, impInfo []Im errL := storedRequestErrorChecker(requestJson, storedRequests, storedBidRequestId) return nil, nil, errL } - resolvedRequest, err = jsonpatch.MergePatch(requestJson, uuidPatch) + + resolvedRequest, err = merge(requestJson, uuidPatch) if err != nil { errL := storedRequestErrorChecker(requestJson, storedRequests, storedBidRequestId) return nil, nil, errL } } else { - resolvedRequest, err = jsonpatch.MergePatch(storedRequests[storedBidRequestId], requestJson) + resolvedRequest, err = merge(storedRequests[storedBidRequestId], requestJson) if err != nil { errL := storedRequestErrorChecker(requestJson, storedRequests, storedBidRequestId) return nil, nil, errL @@ -1729,7 +1800,7 @@ func (deps *endpointDeps) processStoredRequests(requestJson []byte, impInfo []Im // apply default stored request if deps.defaultRequest { - merged, err := jsonpatch.MergePatch(deps.defReqJSON, resolvedRequest) + merged, err := merge(deps.defReqJSON, resolvedRequest) if err != nil { hasErr, Err := getJsonSyntaxError(resolvedRequest) if hasErr { diff --git a/endpoints/openrtb2/auction_test.go b/endpoints/openrtb2/auction_test.go index 65e520a2190..91fa42af3ff 100644 --- a/endpoints/openrtb2/auction_test.go +++ b/endpoints/openrtb2/auction_test.go @@ -43,6 +43,7 @@ import ( "github.com/prebid/prebid-server/v3/util/jsonutil" "github.com/prebid/prebid-server/v3/util/ptrutil" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) const jsonFileExtension string = ".json" @@ -1205,7 +1206,7 @@ func TestStoredRequests(t *testing.T) { assert.Len(t, errs, 0, "No errors should be returned") storedBidRequestId, hasStoredBidRequest, storedRequests, storedImps, errs := deps.getStoredRequests(context.Background(), json.RawMessage(requestData), impInfo) assert.Len(t, errs, 0, "No errors should be returned") - newRequest, impExtInfoMap, errList := deps.processStoredRequests(json.RawMessage(requestData), impInfo, storedRequests, storedImps, storedBidRequestId, hasStoredBidRequest) + newRequest, impExtInfoMap, errList := deps.processStoredRequests(json.RawMessage(requestData), impInfo, storedRequests, storedImps, storedBidRequestId, hasStoredBidRequest, nil) if len(errList) != 0 { for _, err := range errList { if err != nil { @@ -2499,7 +2500,7 @@ func TestStoredRequestGenerateUuid(t *testing.T) { assert.Empty(t, errs, test.description) storedBidRequestId, hasStoredBidRequest, storedRequests, storedImps, errs := deps.getStoredRequests(context.Background(), json.RawMessage(test.givenRawData), impInfo) assert.Empty(t, errs, test.description) - newRequest, _, errList := deps.processStoredRequests(json.RawMessage(test.givenRawData), impInfo, storedRequests, storedImps, storedBidRequestId, hasStoredBidRequest) + newRequest, _, errList := deps.processStoredRequests(json.RawMessage(test.givenRawData), impInfo, storedRequests, storedImps, storedBidRequestId, hasStoredBidRequest, nil) assert.Empty(t, errList, test.description) if err := jsonutil.UnmarshalValid(newRequest, req); err != nil { @@ -6616,3 +6617,362 @@ func TestProcessGDPR(t *testing.T) { }) } } + +func TestMergeWithArrayConcat(t *testing.T) { + tests := []struct { + name string + base string + patch string + expectedResult string + expectError bool + }{ + { + name: "empty base request should return patch unchanged", + base: ``, + patch: `{"bcat":["IAB1"]}`, + expectedResult: `{"bcat":["IAB1"]}`, + expectError: false, + }, + { + name: "empty patch request should return base unchanged", + base: `{"bcat":["IAB1"]}`, + patch: ``, + expectedResult: `{"bcat":["IAB1"]}`, + expectError: false, + }, + { + name: "empty array in patch request should clear base (RFC 7386 semantics)", + base: `{"bcat":["IAB1","IAB2"]}`, + patch: `{"bcat":[]}`, + expectedResult: `{ + "bcat":[] + }`, + expectError: false, + }, + { + name: "empty array in base request with non-empty patch should use patch", + base: `{"bcat":[]}`, + patch: `{"bcat":["IAB1"]}`, + expectedResult: `{ + "bcat":["IAB1"] + }`, + expectError: false, + }, + { + name: "only base request has bcat - should preserve base", + base: `{"bcat":["IAB1","IAB2"]}`, + patch: `{"id":"req1"}`, + expectedResult: `{ + "bcat":["IAB1","IAB2"], + "id":"req1" + }`, + expectError: false, + }, + { + name: "only patch request has bcat - should preserve patch", + base: `{"site":{"id":"1"}}`, + patch: `{"bcat":["IAB3"]}`, + expectedResult: `{ + "bcat":["IAB3"], + "site":{"id":"1"} + }`, + expectError: false, + }, + { + name: "array fields are concatenated", + base: `{"bcat":["IAB1"],"badv":["evil.com"]}`, + patch: `{"bcat":["IAB2"],"badv":["bad.com"]}`, + expectedResult: `{ + "bcat":["IAB1","IAB2"], + "badv":["evil.com","bad.com"] + }`, + expectError: false, + }, + { + name: "non-array fields should follow RFC 7386 merge", + base: `{"bcat":["IAB1"],"site":{"id":"1","name":"base"}}`, + patch: `{"bcat":["IAB2"],"site":{"name":"patch"},"id":"req1"}`, + expectedResult: `{ + "bcat":["IAB1","IAB2"], + "site":{"id":"1","name":"patch"}, + "id":"req1" + }`, + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := mergeWithArrayConcat([]byte(tt.base), []byte(tt.patch), []string{"bcat", "badv", "foo"}) + + if tt.expectError { + assert.Error(t, err) + return + } + + require.NoError(t, err) + + var expected, actual map[string]interface{} + err = json.Unmarshal([]byte(tt.expectedResult), &expected) + require.NoError(t, err, "test case has invalid expected JSON") + + err = json.Unmarshal(result, &actual) + require.NoError(t, err, "result is not valid JSON") + + assert.Equal(t, expected, actual) + }) + } +} + +func TestProcessStoredRequests_HasStoredBidRequestArrayMerge(t *testing.T) { + tests := []struct { + name string + incomingRequest string + storedBidRequestID string + storedBidRequest string + arrayMergeMode config.ArrayMergeMode + generateRequestID bool + storedHasUUID bool + expectedRequest string + hasStoredBidRequest bool + }{ + { + name: "concat mode - both have bcat and badv", + incomingRequest: `{"bcat":["IAB3"],"badv":["competitor.com"],"id":"req1","imp":[{"id":"imp1"}]}`, + storedBidRequestID: "stored1", + storedBidRequest: `{"bcat":["IAB1"],"badv":["evil.com"],"id":"storereq2"}`, + arrayMergeMode: config.ArrayMergeConcat, + expectedRequest: ` + { + "id":"req1", + "imp":[{"id":"imp1"}], + "bcat":["IAB1","IAB3"], + "badv":["evil.com","competitor.com"] + }`, + hasStoredBidRequest: true, + }, + { + name: "replace mode - incoming replaces stored", + incomingRequest: `{"bcat":["IAB3"],"id":"req1","imp":[{"id":"imp1"}]}`, + storedBidRequestID: "stored1", + storedBidRequest: `{"bcat":["IAB1","IAB2"]}`, + arrayMergeMode: config.ArrayMergeReplace, + expectedRequest: ` + { + "id":"req1", + "imp":[{"id":"imp1"}], + "bcat":["IAB3"] + }`, + hasStoredBidRequest: true, + }, + { + name: "default mode - should replace", + incomingRequest: `{"bcat":["IAB3"],"id":"req1","imp":[{"id":"imp1"}]}`, + storedBidRequestID: "stored1", + storedBidRequest: `{"bcat":["IAB1","IAB2"]}`, + arrayMergeMode: "", + expectedRequest: ` + { + "id":"req1", + "imp":[{"id":"imp1"}], + "bcat":["IAB3"] + }`, + hasStoredBidRequest: true, + }, + { + name: "concat mode - empty incoming array clears stored", + incomingRequest: `{"bcat":[],"id":"req1","imp":[{"id":"imp1"}]}`, + storedBidRequestID: "stored1", + storedBidRequest: `{"bcat":["IAB1","IAB2"]}`, + arrayMergeMode: config.ArrayMergeConcat, + expectedRequest: ` + { + "id":"req1", + "imp":[{"id":"imp1"}], + "bcat":[] + }`, + hasStoredBidRequest: true, + }, + { + name: "no stored request - incoming unchanged", + incomingRequest: `{"bcat":["IAB1"],"id":"req1","imp":[{"id":"imp1"}]}`, + arrayMergeMode: config.ArrayMergeConcat, + expectedRequest: ` + { + "id":"req1", + "imp":[{"id":"imp1"}], + "bcat":["IAB1"] + }`, + hasStoredBidRequest: false, + }, + { + name: "UUID generation with concat mode", + incomingRequest: `{"bcat":["IAB3"],"imp":[{"id":"imp1"}]}`, + storedBidRequestID: "stored1", + storedBidRequest: `{"bcat":["IAB1","IAB2"],"site":{"id":"site1"},"id":"{{UUID}}"}`, + arrayMergeMode: config.ArrayMergeConcat, + storedHasUUID: true, + expectedRequest: ` + { + "id":"test-generated-uuid-12345", + "imp":[{"id":"imp1"}], + "bcat":["IAB3","IAB1","IAB2"], + "site":{"id":"site1"} + }`, + hasStoredBidRequest: true, + }, + { + name: "UUID generation with replace mode - stored request wins", + incomingRequest: `{"bcat":["IAB3"],"imp":[{"id":"imp1"}]}`, + storedBidRequestID: "stored1", + storedBidRequest: `{"bcat":["IAB1","IAB2"],"site":{"id":"site1"},"id":"{{UUID}}"}`, + arrayMergeMode: config.ArrayMergeReplace, + storedHasUUID: true, + expectedRequest: ` + { + "id":"test-generated-uuid-12345", + "imp":[{"id":"imp1"}], + "bcat":["IAB1","IAB2"], + "site":{"id":"site1"} + }`, + hasStoredBidRequest: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + deps := &endpointDeps{ + cfg: &config.Configuration{ + GenerateRequestID: tt.generateRequestID, + }, + uuidGenerator: fakeUUIDGenerator{id: "test-generated-uuid-12345"}, + } + account := &config.Account{ + StoredRequest: config.AccountStoredRequest{ + ArrayMerge: tt.arrayMergeMode, + }, + } + + storedRequests := map[string]json.RawMessage{} + if tt.hasStoredBidRequest { + storedRequests[tt.storedBidRequestID] = json.RawMessage(tt.storedBidRequest) + } + + impInfo, errs := parseImpInfo([]byte(tt.incomingRequest)) + require.Empty(t, errs) + + result, _, errs := deps.processStoredRequests( + []byte(tt.incomingRequest), + impInfo, + storedRequests, + nil, + tt.storedBidRequestID, + tt.hasStoredBidRequest, + account, + ) + + require.Empty(t, errs) + assert.JSONEq(t, tt.expectedRequest, string(result)) + }) + } +} + +func TestProcessStoredRequests_DefaultRequestArrayMerge(t *testing.T) { + tests := []struct { + name string + incomingRequest string + defaultRequest string + arrayMergeMode config.ArrayMergeMode + expectedRequest string + }{ + { + name: "concat mode - both default and incoming have bcat", + incomingRequest: `{"bcat":["IAB3"],"id":"req1","imp":[{"id":"imp1"}]}`, + defaultRequest: `{"bcat":["IAB1","IAB2"]}`, + arrayMergeMode: config.ArrayMergeConcat, + expectedRequest: ` + { + "id":"req1", + "imp":[{"id":"imp1"}], + "bcat":["IAB1","IAB2","IAB3"] + }`, + }, + { + name: "replace mode - incoming wins", + incomingRequest: `{"bcat":["IAB3"],"id":"req1","imp":[{"id":"imp1"}]}`, + defaultRequest: `{"bcat":["IAB1","IAB2"]}`, + arrayMergeMode: config.ArrayMergeReplace, + expectedRequest: ` + { + "id":"req1", + "imp":[{"id":"imp1"}], + "bcat":["IAB3"] + }`, + }, + { + name: "default mode - should replace", + incomingRequest: `{"bcat":["IAB3"],"id":"req1","imp":[{"id":"imp1"}]}`, + defaultRequest: `{"bcat":["IAB1","IAB2"]}`, + arrayMergeMode: "", + expectedRequest: ` + { + "id":"req1", + "imp":[{"id":"imp1"}], + "bcat":["IAB3"] + }`, + }, + { + name: "concat mode - empty incoming array clears stored", + incomingRequest: `{"bcat":[],"id":"req1","imp":[{"id":"imp1"}]}`, + defaultRequest: `{"bcat":["IAB1","IAB2"]}`, + arrayMergeMode: config.ArrayMergeConcat, + expectedRequest: ` + { + "id":"req1", + "imp":[{"id":"imp1"}], + "bcat":[] + }`, + }, + { + name: "no default stored request - incoming unchanged", + incomingRequest: `{"bcat":["IAB1"],"id":"req1","imp":[{"id":"imp1"}]}`, + arrayMergeMode: config.ArrayMergeConcat, + expectedRequest: ` + { + "id":"req1", + "imp":[{"id":"imp1"}], + "bcat":["IAB1"] + }`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + deps := &endpointDeps{ + defaultRequest: true, + defReqJSON: json.RawMessage(tt.defaultRequest), + } + account := &config.Account{ + StoredRequest: config.AccountStoredRequest{ + ArrayMerge: tt.arrayMergeMode, + }, + } + + impInfo, errs := parseImpInfo([]byte(tt.incomingRequest)) + require.Empty(t, errs) + + result, _, errs := deps.processStoredRequests( + []byte(tt.incomingRequest), + impInfo, + nil, + nil, + "", + false, + account, + ) + + require.Empty(t, errs) + assert.JSONEq(t, tt.expectedRequest, string(result)) + }) + } +}