diff --git a/endpoints/openrtb2/auction_benchmark_test.go b/endpoints/openrtb2/auction_benchmark_test.go index b3e97b4d4e4..d3dbc518bd3 100644 --- a/endpoints/openrtb2/auction_benchmark_test.go +++ b/endpoints/openrtb2/auction_benchmark_test.go @@ -72,6 +72,7 @@ func BenchmarkOpenrtbEndpoint(b *testing.B) { if err != nil { return } + requestValidator := ortb.NewRequestValidator(nil, map[string]string{}, paramValidator) nilMetrics := &metricsConfig.NilMetricsEngine{} @@ -88,6 +89,7 @@ func BenchmarkOpenrtbEndpoint(b *testing.B) { adapters, nil, &config.Configuration{}, + requestValidator, map[string]usersync.Syncer{}, nilMetrics, infos, @@ -102,7 +104,7 @@ func BenchmarkOpenrtbEndpoint(b *testing.B) { endpoint, _ := NewEndpoint( fakeUUIDGenerator{}, exchange, - ortb.NewRequestValidator(nil, map[string]string{}, paramValidator), + requestValidator, empty_fetcher.EmptyFetcher{}, empty_fetcher.EmptyFetcher{}, &config.Configuration{MaxRequestSize: maxSize}, diff --git a/endpoints/openrtb2/test_utils.go b/endpoints/openrtb2/test_utils.go index 0521befa34e..b2f2826739f 100644 --- a/endpoints/openrtb2/test_utils.go +++ b/endpoints/openrtb2/test_utils.go @@ -1197,7 +1197,7 @@ func (te *exchangeTestWrapper) HoldAuction(ctx context.Context, r *exchange.Auct } // buildTestExchange returns an exchange with mock bidder servers and mock currency conversion server -func buildTestExchange(testCfg *testConfigValues, adapterMap map[openrtb_ext.BidderName]exchange.AdaptedBidder, mockBidServersArray []*httptest.Server, mockCurrencyRatesServer *httptest.Server, bidderInfos config.BidderInfos, cfg *config.Configuration, met metrics.MetricsEngine, mockFetcher stored_requests.CategoryFetcher) (exchange.Exchange, []*httptest.Server) { +func buildTestExchange(testCfg *testConfigValues, adapterMap map[openrtb_ext.BidderName]exchange.AdaptedBidder, mockBidServersArray []*httptest.Server, mockCurrencyRatesServer *httptest.Server, bidderInfos config.BidderInfos, cfg *config.Configuration, met metrics.MetricsEngine, mockFetcher stored_requests.CategoryFetcher, requestValidator ortb.RequestValidator) (exchange.Exchange, []*httptest.Server) { if len(testCfg.MockBidders) == 0 { testCfg.MockBidders = append(testCfg.MockBidders, mockBidderHandler{BidderName: "appnexus", Currency: "USD", Price: 0.00}) } @@ -1220,6 +1220,7 @@ func buildTestExchange(testCfg *testConfigValues, adapterMap map[openrtb_ext.Bid testExchange := exchange.NewExchange(adapterMap, &wellBehavedCache{}, cfg, + requestValidator, nil, met, bidderInfos, @@ -1276,7 +1277,7 @@ func buildTestEndpoint(test testCase, cfg *config.Configuration) (httprouter.Han } mockCurrencyRatesServer := httptest.NewServer(http.HandlerFunc(mockCurrencyConversionService.handle)) - testExchange, mockBidServersArray := buildTestExchange(test.Config, adapterMap, mockBidServersArray, mockCurrencyRatesServer, bidderInfos, cfg, met, mockFetcher) + testExchange, mockBidServersArray := buildTestExchange(test.Config, adapterMap, mockBidServersArray, mockCurrencyRatesServer, bidderInfos, cfg, met, mockFetcher, requestValidator) var storedRequestFetcher stored_requests.Fetcher if len(test.StoredRequest) > 0 { diff --git a/errortypes/code.go b/errortypes/code.go index a56df67c9c9..552beffbcb8 100644 --- a/errortypes/code.go +++ b/errortypes/code.go @@ -17,6 +17,7 @@ const ( TmaxTimeoutErrorCode FailedToMarshalErrorCode FailedToUnmarshalErrorCode + InvalidImpFirstPartyDataErrorCode ) // Defines numeric codes for well-known warnings. diff --git a/errortypes/errortypes.go b/errortypes/errortypes.go index 7ca5668c290..f23e1423977 100644 --- a/errortypes/errortypes.go +++ b/errortypes/errortypes.go @@ -274,3 +274,21 @@ func (err *DebugWarning) Severity() Severity { func (err *DebugWarning) Scope() Scope { return ScopeDebug } + +// InvalidImpFirstPartyData should be used when the retrieved account config cannot be unmarshaled +// These errors will be written to http.ResponseWriter before canceling execution +type InvalidImpFirstPartyData struct { + Message string +} + +func (err *InvalidImpFirstPartyData) Error() string { + return err.Message +} + +func (err *InvalidImpFirstPartyData) Code() int { + return InvalidImpFirstPartyDataErrorCode +} + +func (err *InvalidImpFirstPartyData) Severity() Severity { + return SeverityFatal +} diff --git a/exchange/exchange.go b/exchange/exchange.go index 01abe37d4d7..5c27b0d3c5a 100644 --- a/exchange/exchange.go +++ b/exchange/exchange.go @@ -14,6 +14,7 @@ import ( "strings" "time" + "github.com/prebid/prebid-server/v2/ortb" "github.com/prebid/prebid-server/v2/privacy" "github.com/prebid/prebid-server/v2/adapters" @@ -132,7 +133,7 @@ func (randomDeduplicateBidBooleanGenerator) Generate() bool { return rand.Intn(100) < 50 } -func NewExchange(adapters map[openrtb_ext.BidderName]AdaptedBidder, cache prebid_cache_client.Client, cfg *config.Configuration, syncersByBidder map[string]usersync.Syncer, metricsEngine metrics.MetricsEngine, infos config.BidderInfos, gdprPermsBuilder gdpr.PermissionsBuilder, currencyConverter *currency.RateConverter, categoriesFetcher stored_requests.CategoryFetcher, adsCertSigner adscert.Signer, macroReplacer macros.Replacer, priceFloorFetcher floors.FloorFetcher) Exchange { +func NewExchange(adapters map[openrtb_ext.BidderName]AdaptedBidder, cache prebid_cache_client.Client, cfg *config.Configuration, requestValidator ortb.RequestValidator, syncersByBidder map[string]usersync.Syncer, metricsEngine metrics.MetricsEngine, infos config.BidderInfos, gdprPermsBuilder gdpr.PermissionsBuilder, currencyConverter *currency.RateConverter, categoriesFetcher stored_requests.CategoryFetcher, adsCertSigner adscert.Signer, macroReplacer macros.Replacer, priceFloorFetcher floors.FloorFetcher) Exchange { bidderToSyncerKey := map[string]string{} for bidder, syncer := range syncersByBidder { bidderToSyncerKey[bidder] = syncer.Key() @@ -155,6 +156,7 @@ func NewExchange(adapters map[openrtb_ext.BidderName]AdaptedBidder, cache prebid gdprPermsBuilder: gdprPermsBuilder, hostSChainNode: cfg.HostSChainNode, bidderInfo: infos, + requestValidator: requestValidator, } return &exchange{ @@ -338,6 +340,11 @@ func (e *exchange) HoldAuction(ctx context.Context, r *AuctionRequest, debugLog SChain: requestExt.GetSChain(), } bidderRequests, privacyLabels, errs := e.requestSplitter.cleanOpenRTBRequests(ctx, *r, requestExtLegacy, gdprSignal, gdprEnforced, bidAdjustmentFactors) + for _, err := range errs { + if errortypes.ReadCode(err) == errortypes.InvalidImpFirstPartyDataErrorCode { + return nil, err + } + } errs = append(errs, floorErrs...) mergedBidAdj, err := bidadjustment.Merge(r.BidRequestWrapper, r.Account.BidAdjustments) diff --git a/exchange/exchange_test.go b/exchange/exchange_test.go index 5849dbc3f25..162f425587e 100644 --- a/exchange/exchange_test.go +++ b/exchange/exchange_test.go @@ -35,10 +35,12 @@ import ( metricsConf "github.com/prebid/prebid-server/v2/metrics/config" metricsConfig "github.com/prebid/prebid-server/v2/metrics/config" "github.com/prebid/prebid-server/v2/openrtb_ext" + "github.com/prebid/prebid-server/v2/ortb" pbc "github.com/prebid/prebid-server/v2/prebid_cache_client" "github.com/prebid/prebid-server/v2/privacy" "github.com/prebid/prebid-server/v2/stored_requests" "github.com/prebid/prebid-server/v2/stored_requests/backends/file_fetcher" + "github.com/prebid/prebid-server/v2/stored_responses" "github.com/prebid/prebid-server/v2/usersync" "github.com/prebid/prebid-server/v2/util/jsonutil" "github.com/prebid/prebid-server/v2/util/ptrutil" @@ -82,7 +84,7 @@ func TestNewExchange(t *testing.T) { }, }.Builder - e := NewExchange(adapters, nil, cfg, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, currencyConverter, nilCategoryFetcher{}, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), nil).(*exchange) + e := NewExchange(adapters, nil, cfg, &mockRequestValidator{}, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, currencyConverter, nilCategoryFetcher{}, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), nil).(*exchange) for _, bidderName := range knownAdapters { if _, ok := e.adapterMap[bidderName]; !ok { if biddersInfo[string(bidderName)].IsEnabled() { @@ -132,7 +134,7 @@ func TestCharacterEscape(t *testing.T) { }, }.Builder - e := NewExchange(adapters, nil, cfg, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, currencyConverter, nilCategoryFetcher{}, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), nil).(*exchange) + e := NewExchange(adapters, nil, cfg, &mockRequestValidator{}, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, currencyConverter, nilCategoryFetcher{}, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), nil).(*exchange) // 3) Build all the parameters e.buildBidResponse(ctx.Background(), liveA... ) needs //liveAdapters []openrtb_ext.BidderName, @@ -1235,7 +1237,7 @@ func TestGetBidCacheInfoEndToEnd(t *testing.T) { }, }.Builder - e := NewExchange(adapters, pbc, cfg, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, currencyConverter, nilCategoryFetcher{}, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), nil).(*exchange) + e := NewExchange(adapters, pbc, cfg, &mockRequestValidator{}, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, currencyConverter, nilCategoryFetcher{}, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), nil).(*exchange) // 3) Build all the parameters e.buildBidResponse(ctx.Background(), liveA... ) needs liveAdapters := []openrtb_ext.BidderName{bidderName} @@ -1594,7 +1596,7 @@ func TestBidResponseCurrency(t *testing.T) { }, }.Builder - e := NewExchange(adapters, nil, cfg, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, currencyConverter, nilCategoryFetcher{}, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), nil).(*exchange) + e := NewExchange(adapters, nil, cfg, &mockRequestValidator{}, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, currencyConverter, nilCategoryFetcher{}, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), nil).(*exchange) liveAdapters := make([]openrtb_ext.BidderName, 1) liveAdapters[0] = "appnexus" @@ -1742,7 +1744,7 @@ func TestBidResponseImpExtInfo(t *testing.T) { t.Fatalf("Error intializing adapters: %v", adaptersErr) } - e := NewExchange(adapters, nil, cfg, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, nil, gdprPermsBuilder, nil, nilCategoryFetcher{}, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), nil).(*exchange) + e := NewExchange(adapters, nil, cfg, &mockRequestValidator{}, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, nil, gdprPermsBuilder, nil, nilCategoryFetcher{}, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), nil).(*exchange) liveAdapters := make([]openrtb_ext.BidderName, 1) liveAdapters[0] = "appnexus" @@ -1836,7 +1838,7 @@ func TestRaceIntegration(t *testing.T) { }, }.Builder - ex := NewExchange(adapters, &wellBehavedCache{}, cfg, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, currencyConverter, &nilCategoryFetcher{}, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), nil).(*exchange) + ex := NewExchange(adapters, &wellBehavedCache{}, cfg, &mockRequestValidator{}, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, currencyConverter, &nilCategoryFetcher{}, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), nil).(*exchange) _, err = ex.HoldAuction(context.Background(), auctionRequest, &debugLog) if err != nil { t.Errorf("HoldAuction returned unexpected error: %v", err) @@ -1934,7 +1936,7 @@ func TestPanicRecovery(t *testing.T) { }, }.Builder - e := NewExchange(adapters, nil, cfg, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, currencyConverter, nilCategoryFetcher{}, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), nil).(*exchange) + e := NewExchange(adapters, nil, cfg, &mockRequestValidator{}, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, currencyConverter, nilCategoryFetcher{}, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), nil).(*exchange) chBids := make(chan *bidResponseWrapper, 1) panicker := func(bidderRequest BidderRequest, conversions currency.Conversions) { @@ -2004,7 +2006,7 @@ func TestPanicRecoveryHighLevel(t *testing.T) { allowAllBidders: true, }, }.Builder - e := NewExchange(adapters, &mockCache{}, cfg, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, currencyConverter, categoriesFetcher, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), nil).(*exchange) + e := NewExchange(adapters, &mockCache{}, cfg, &mockRequestValidator{}, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, currencyConverter, categoriesFetcher, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), nil).(*exchange) e.adapterMap[openrtb_ext.BidderBeachfront] = panicingAdapter{} e.adapterMap[openrtb_ext.BidderAppnexus] = panicingAdapter{} @@ -2335,7 +2337,8 @@ func runSpec(t *testing.T, filename string, spec *exchangeSpec) { } func findBiddersInAuction(t *testing.T, context string, req *openrtb2.BidRequest) []string { - if splitImps, err := splitImps(req.Imp); err != nil { + + if splitImps, err := splitImps(req.Imp, &mockRequestValidator{}, nil, false, nil); err != nil { t.Errorf("%s: Failed to parse Bidders from request: %v", context, err) return nil } else { @@ -2384,6 +2387,8 @@ func newExchangeForTests(t *testing.T, filename string, expectations map[string] mockResponses: map[string]bidderResponse{string(bidderName): spec.MockResponse}, } bidderInfos[string(bidderName)] = config.BidderInfo{ModifyingVastXmlAllowed: spec.ModifyingVastXmlAllowed} + } else { + bidderInfos[string(bidderName)] = config.BidderInfo{} } } @@ -2433,6 +2438,12 @@ func newExchangeForTests(t *testing.T, filename string, expectations map[string] } metricsEngine := metricsConf.NewMetricsEngine(&config.Configuration{}, openrtb_ext.CoreBidderNames(), nil, nil) + paramValidator, err := openrtb_ext.NewBidderParamsValidator("../static/bidder-params") + if err != nil { + t.Fatalf("Failed to create params validator: %v", error) + } + bidderMap := GetActiveBidders(bidderInfos) + requestValidator := ortb.NewRequestValidator(bidderMap, map[string]string{}, paramValidator) requestSplitter := requestSplitter{ bidderToSyncerKey: bidderToSyncerKey, me: metricsEngine, @@ -2440,6 +2451,7 @@ func newExchangeForTests(t *testing.T, filename string, expectations map[string] gdprPermsBuilder: gdprPermsBuilder, hostSChainNode: hostSChainNode, bidderInfo: bidderInfos, + requestValidator: requestValidator, } return &exchange{ @@ -4565,7 +4577,7 @@ func TestPassExperimentConfigsToHoldAuction(t *testing.T) { }, }.Builder - e := NewExchange(adapters, nil, cfg, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, currencyConverter, nilCategoryFetcher{}, &signer, macros.NewStringIndexBasedReplacer(), nil).(*exchange) + e := NewExchange(adapters, nil, cfg, &mockRequestValidator{}, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, currencyConverter, nilCategoryFetcher{}, &signer, macros.NewStringIndexBasedReplacer(), nil).(*exchange) // Define mock incoming bid requeset mockBidRequest := &openrtb2.BidRequest{ @@ -6332,3 +6344,11 @@ func TestBidsToUpdate(t *testing.T) { }) } } + +type mockRequestValidator struct { + errors []error +} + +func (mrv *mockRequestValidator) ValidateImp(imp *openrtb_ext.ImpWrapper, index int, aliases map[string]string, hasStoredResponses bool, storedBidResponses stored_responses.ImpBidderStoredResp) []error { + return mrv.errors +} diff --git a/exchange/exchangetest/imp-fpd-invalid.json b/exchange/exchangetest/imp-fpd-invalid.json new file mode 100644 index 00000000000..6d0bd9b9f76 --- /dev/null +++ b/exchange/exchangetest/imp-fpd-invalid.json @@ -0,0 +1,55 @@ +{ + "requestType": "openrtb2-web", + "incomingRequest": { + "ortbRequest": { + "id": "some-request-id", + "site": { + "page": "test.somepage.com" + }, + "imp": [ + { + "id": "imp-id-1", + "banner": { + "format": [ + { + "w": 300, + "h": 600 + } + ] + }, + "ext": { + "prebid": { + "bidder": { + "appnexus": { + "placementId": 1 + }, + "audienceNetwork": { + "placementId": "some-placement" + } + }, + "imp": { + "appnexus": { + "id": "imp-id-1-appnexus" + }, + "audienceNetwork": { + "banner": { + "format": [ + { + "w": -1, + "h": -1 + } + ] + } + } + } + } + } + } + ] + } + }, + "outgoingRequests": {}, + "response": { + "error": "merging bidder imp first party data for imp imp-id-1 results in an invalid imp: [request.imp[0].banner.format[0].w must be a positive number]" + } +} \ No newline at end of file diff --git a/exchange/exchangetest/imp-fpd-multiple-imp.json b/exchange/exchangetest/imp-fpd-multiple-imp.json index c49e0b685cf..902a071b606 100644 --- a/exchange/exchangetest/imp-fpd-multiple-imp.json +++ b/exchange/exchangetest/imp-fpd-multiple-imp.json @@ -83,7 +83,7 @@ }] }, "native": { - "request": "{\"ver\":\"1.1\",\"layout\":1,\"adunit\":2,\"plcmtcnt\":6,\"plcmttype\":4,\"assets\":[{\"id\":1,\"required\":1,\"title\":{\"len\":75}},{\"id\":2,\"required\":1,\"img\":{\"wmin\":492,\"hmin\":328,\"type\":3,\"mimes\":[\"image/jpeg\",\"image/jpg\",\"image/png\"]}},{\"id\":4,\"required\":0,\"data\":{\"type\":6}},{\"id\":5,\"required\":0,\"data\":{\"type\":7}},{\"id\":6,\"required\":0,\"data\":{\"type\":1,\"len\":20}}]}", + "request": "{\"ver\":\"1.1\",\"layout\":1,\"adunit\":2,\"plcmtcnt\":6,\"plcmttype\":4,\"assets\":[{\"id\":1,\"required\":1,\"title\":{\"len\":75}},{\"id\":2,\"required\":1,\"img\":{\"wmin\":492,\"hmin\":328,\"type\":3,\"mimes\":[\"image/jpeg\",\"image/jpg\",\"image/png\"]}},{\"id\":4,\"data\":{\"type\":6}},{\"id\":5,\"data\":{\"type\":7}},{\"id\":6,\"data\":{\"type\":1,\"len\":20}}]}", "ver": "1.1" }, "ext": { @@ -171,7 +171,7 @@ "poddur": 40 }, "native": { - "request": "{\"ver\":\"1.1\",\"layout\":1,\"adunit\":2,\"plcmtcnt\":6,\"plcmttype\":4,\"assets\":[{\"id\":1,\"required\":1,\"title\":{\"len\":75}},{\"id\":2,\"required\":1,\"img\":{\"wmin\":492,\"hmin\":328,\"type\":3,\"mimes\":[\"image/jpeg\",\"image/jpg\",\"image/png\"]}},{\"id\":4,\"required\":0,\"data\":{\"type\":6}},{\"id\":5,\"required\":0,\"data\":{\"type\":7}},{\"id\":6,\"required\":0,\"data\":{\"type\":1,\"len\":20}}]}", + "request": "{\"ver\":\"1.1\",\"layout\":1,\"adunit\":2,\"plcmttype\":4,\"plcmtcnt\":6,\"assets\":[{\"id\":1,\"required\":1,\"title\":{\"len\":75}},{\"id\":2,\"required\":1,\"img\":{\"type\":3,\"wmin\":492,\"hmin\":328,\"mimes\":[\"image/jpeg\",\"image/jpg\",\"image/png\"]}},{\"id\":4,\"data\":{\"type\":6}},{\"id\":5,\"data\":{\"type\":7}},{\"id\":6,\"data\":{\"type\":1,\"len\":20}}]}", "ver": "2.2", "ctype": 1 }, diff --git a/exchange/utils.go b/exchange/utils.go index a3a5bd13e84..acac61d3c49 100644 --- a/exchange/utils.go +++ b/exchange/utils.go @@ -51,6 +51,7 @@ type requestSplitter struct { gdprPermsBuilder gdpr.PermissionsBuilder hostSChainNode *openrtb2.SupplyChainNode bidderInfo config.BidderInfos + requestValidator ortb.RequestValidator } // cleanOpenRTBRequests splits the input request into requests which are sanitized for each bidder. Intended behavior is: @@ -76,7 +77,9 @@ func (rs *requestSplitter) cleanOpenRTBRequests(ctx context.Context, bidderImpWithBidResp := stored_responses.InitStoredBidResponses(req.BidRequest, auctionReq.StoredBidResponses) - impsByBidder, err := splitImps(req.BidRequest.Imp) + hasStoredResponses := len(auctionReq.StoredAuctionResponses) > 0 + + impsByBidder, err := splitImps(req.BidRequest.Imp, rs.requestValidator, requestAliases, hasStoredResponses, auctionReq.StoredBidResponses) if err != nil { errs = []error{err} return @@ -567,7 +570,7 @@ func extractBuyerUIDs(user *openrtb2.User) (map[string]string, error) { // The "imp.ext" value of the rubicon Imp will only contain the "prebid" values, and "rubicon" value at the "bidder" key. // // The goal here is so that Bidders only get Imps and Imp.Ext values which are intended for them. -func splitImps(imps []openrtb2.Imp) (map[string][]openrtb2.Imp, error) { +func splitImps(imps []openrtb2.Imp, requestValidator ortb.RequestValidator, requestAliases map[string]string, hasStoredResponses bool, storedBidResponses stored_responses.ImpBidderStoredResp) (map[string][]openrtb2.Imp, error) { bidderImps := make(map[string][]openrtb2.Imp) for i, imp := range imps { @@ -605,6 +608,12 @@ func splitImps(imps []openrtb2.Imp) (map[string][]openrtb2.Imp, error) { if err := mergeImpFPD(&impCopy, impBidderFPD, i); err != nil { return nil, err } + impWrapper := openrtb_ext.ImpWrapper{Imp: &impCopy} + if err := requestValidator.ValidateImp(&impWrapper, i, requestAliases, hasStoredResponses, storedBidResponses); err != nil { + return nil, &errortypes.InvalidImpFirstPartyData{ + Message: fmt.Sprintf("merging bidder imp first party data for imp %s results in an invalid imp: %v", imp.ID, err), + } + } } sanitizedImpExt[openrtb_ext.PrebidExtBidderKey] = bidderExt @@ -629,10 +638,6 @@ func mergeImpFPD(imp *openrtb2.Imp, fpd json.RawMessage, index int) error { } return fmt.Errorf("invalid first party data for imp[%d]", index) } - // impWrapper := openrtb_ext.ImpWrapper{Imp: imp} - // if err := validation.ValidateImp(&impWrapper, index); err != nil { - // return fmt.Errorf("merging bidder imp first party data for imp %s results in an invalid imp: %v", imp.ID, err) - // } return nil } diff --git a/exchange/utils_test.go b/exchange/utils_test.go index e2f2c51e584..c22fc091949 100644 --- a/exchange/utils_test.go +++ b/exchange/utils_test.go @@ -93,10 +93,11 @@ func assertReq(t *testing.T, bidderRequests []BidderRequest, func TestSplitImps(t *testing.T) { testCases := []struct { - description string - givenImps []openrtb2.Imp - expectedImps map[string][]openrtb2.Imp - expectedError string + description string + givenImps []openrtb2.Imp + validatorErrors []error + expectedImps map[string][]openrtb2.Imp + expectedError string }{ { description: "Nil", @@ -284,10 +285,30 @@ func TestSplitImps(t *testing.T) { }, expectedError: "", }, + { + description: "invalid FPD at imp.ext.prebid.imp for valid bidder", + givenImps: []openrtb2.Imp{ + { + ID: "imp1", + Banner: &openrtb2.Banner{ + Format: []openrtb2.Format{ + { + W: 10, + H: 20, + }, + }, + }, + Ext: json.RawMessage(`{"prebid":{"bidder":{"bidderA":{"imp1paramA":"imp1valueA"}},"imp":{"bidderA":{"id":"impFPD", "banner":{"format":[{"w":0,"h":0}]}}}}}`), + }, + }, + validatorErrors: []error{errors.New("some error")}, + expectedImps: nil, + expectedError: "merging bidder imp first party data for imp imp1 results in an invalid imp: [some error]", + }, } for _, test := range testCases { - imps, err := splitImps(test.givenImps) + imps, err := splitImps(test.givenImps, &mockRequestValidator{errors: test.validatorErrors}, nil, false, nil) if test.expectedError == "" { assert.NoError(t, err, test.description+":err") diff --git a/router/router.go b/router/router.go index 308f8d4fd2c..c402038317b 100644 --- a/router/router.go +++ b/router/router.go @@ -242,7 +242,7 @@ func New(cfg *config.Configuration, rateConvertor *currency.RateConverter) (r *R tmaxAdjustments := exchange.ProcessTMaxAdjustments(cfg.TmaxAdjustments) planBuilder := hooks.NewExecutionPlanBuilder(cfg.Hooks, repo) macroReplacer := macros.NewStringIndexBasedReplacer() - theExchange := exchange.NewExchange(adapters, cacheClient, cfg, syncersByBidder, r.MetricsEngine, cfg.BidderInfos, gdprPermsBuilder, rateConvertor, categoriesFetcher, adsCertSigner, macroReplacer, priceFloorFetcher) + theExchange := exchange.NewExchange(adapters, cacheClient, cfg, requestValidator, syncersByBidder, r.MetricsEngine, cfg.BidderInfos, gdprPermsBuilder, rateConvertor, categoriesFetcher, adsCertSigner, macroReplacer, priceFloorFetcher) var uuidGenerator uuidutil.UUIDRandomGenerator openrtbEndpoint, err := openrtb2.NewEndpoint(uuidGenerator, theExchange, requestValidator, fetcher, accounts, cfg, r.MetricsEngine, analyticsRunner, disabledBidders, defReqJSON, activeBidders, storedRespFetcher, planBuilder, tmaxAdjustments) if err != nil {