From 32fdbc4be35375912ac886ca05f7318c06995e0c Mon Sep 17 00:00:00 2001 From: Brian Sardo <1168933+bsardo@users.noreply.github.com> Date: Tue, 4 Jun 2024 21:07:45 -0400 Subject: [PATCH] Imp FPD: Skip bidder params and native validation (#3720) --- endpoints/openrtb2/auction.go | 2 +- .../invalid-whole/invalid-bidder-params.json | 38 +++++ exchange/exchange_test.go | 2 +- .../exchangetest/imp-fpd-multiple-imp.json | 4 +- .../imp-fpd-skipped-validation.json | 137 ++++++++++++++++++ exchange/utils.go | 10 +- ortb/request_validator.go | 25 +++- ortb/request_validator_test.go | 47 +++++- 8 files changed, 241 insertions(+), 24 deletions(-) create mode 100644 endpoints/openrtb2/sample-requests/invalid-whole/invalid-bidder-params.json create mode 100644 exchange/exchangetest/imp-fpd-skipped-validation.json diff --git a/endpoints/openrtb2/auction.go b/endpoints/openrtb2/auction.go index d6ee5303604..57d9e73a27d 100644 --- a/endpoints/openrtb2/auction.go +++ b/endpoints/openrtb2/auction.go @@ -918,7 +918,7 @@ func (deps *endpointDeps) validateRequest(account *config.Account, httpReq *http } impIDs[imp.ID] = i - errs := deps.requestValidator.ValidateImp(imp, i, requestAliases, hasStoredAuctionResponses, storedBidResp) + errs := deps.requestValidator.ValidateImp(imp, ortb.ValidationConfig{}, i, requestAliases, hasStoredAuctionResponses, storedBidResp) if len(errs) > 0 { errL = append(errL, errs...) } diff --git a/endpoints/openrtb2/sample-requests/invalid-whole/invalid-bidder-params.json b/endpoints/openrtb2/sample-requests/invalid-whole/invalid-bidder-params.json new file mode 100644 index 00000000000..0107ef9420e --- /dev/null +++ b/endpoints/openrtb2/sample-requests/invalid-whole/invalid-bidder-params.json @@ -0,0 +1,38 @@ +{ + "description": "Required appnexus bidder param is provided but the type is invalid", + "config": { + "realParamsValidator": true + }, + "mockBidRequest": { + "id": "some-request-id", + "site": { + "page": "prebid.org" + }, + "imp": [ + { + "id": "some-impression-id", + "banner": { + "format": [ + { + "w": 300, + "h": 250 + }, + { + "w": 300, + "h": 600 + } + ] + }, + "ext": { + "appnexus": { + "placementId": [] + } + } + } + ], + "tmax": 500, + "ext": {} + }, + "expectedReturnCode": 400, + "expectedErrorMessage": "Invalid request: request.imp[0].ext.prebid.bidder.appnexus failed validation.\nplacementId: Invalid type. Expected: [integer,string], given: array\n" +} \ No newline at end of file diff --git a/exchange/exchange_test.go b/exchange/exchange_test.go index 162f425587e..cdd2fc1889f 100644 --- a/exchange/exchange_test.go +++ b/exchange/exchange_test.go @@ -6349,6 +6349,6 @@ 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 { +func (mrv *mockRequestValidator) ValidateImp(imp *openrtb_ext.ImpWrapper, cfg ortb.ValidationConfig, index int, aliases map[string]string, hasStoredResponses bool, storedBidResponses stored_responses.ImpBidderStoredResp) []error { return mrv.errors } diff --git a/exchange/exchangetest/imp-fpd-multiple-imp.json b/exchange/exchangetest/imp-fpd-multiple-imp.json index fa095126eed..11261cd1349 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,\"data\":{\"type\":6}},{\"id\":5,\"data\":{\"type\":7}},{\"id\":6,\"data\":{\"type\":1,\"len\":20}}]}", + "request": "{\"ver\":\"1.1\",\"layout\":1,\"adunit\":2,\"plcmttype\":4,\"assets\":[{\"id\":1,\"required\":1,\"title\":{\"len\":75}}]}", "ver": "1.1" }, "ext": { @@ -173,7 +173,7 @@ "poddur": 40 }, "native": { - "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}}]}", + "request": "{\"ver\":\"1.1\",\"layout\":1,\"adunit\":2,\"plcmttype\":4,\"assets\":[{\"id\":1,\"required\":1,\"title\":{\"len\":75}}]}", "ver": "2.2", "ctype": 1 }, diff --git a/exchange/exchangetest/imp-fpd-skipped-validation.json b/exchange/exchangetest/imp-fpd-skipped-validation.json new file mode 100644 index 00000000000..d6285a02fbe --- /dev/null +++ b/exchange/exchangetest/imp-fpd-skipped-validation.json @@ -0,0 +1,137 @@ +{ + "requestType": "openrtb2-web", + "incomingRequest": { + "ortbRequest": { + "id": "some-request-id", + "site": { + "page": "test.somepage.com" + }, + "imp": [ + { + "id": "imp-id-1", + "native": {}, + "ext": { + "prebid": { + "bidder": { + "appnexus": { + "placementId": [] + } + }, + "imp": { + "appnexus": { + "id": "imp-id-1-appnexus" + } + } + } + } + }, + { + "id": "imp-id-2", + "banner": { + "format": [ + { + "w": 300, + "h": 600 + } + ] + }, + "ext": { + "prebid": { + "bidder": { + "appnexus": { + "placementId": "123" + } + } + } + } + } + ] + } + }, + "outgoingRequests": { + "appnexus": { + "expectRequest": { + "ortbRequest": { + "id": "some-request-id", + "site": { + "page": "test.somepage.com" + }, + "imp": [ + { + "id": "imp-id-1-appnexus", + "native": {}, + "ext": { + "bidder": { + "placementId": [] + } + } + }, + { + "id": "imp-id-2", + "banner": { + "format": [ + { + "w": 300, + "h": 600 + } + ] + }, + "ext": { + "bidder": { + "placementId": "123" + } + } + } + ] + } + }, + "mockResponse": { + "pbsSeatBids": [ + { + "pbsBids": [ + { + "ortbBid": { + "id": "apn-bid", + "impid": "imp-id-2", + "price": 0.3, + "w": 300, + "h": 600, + "crid": "creative-1" + }, + "bidType": "banner" + } + ], + "seat": "appnexus" + } + ] + } + } + }, + "response": { + "bids": { + "id": "some-request-id", + "seatbid": [ + { + "seat": "appnexus", + "bid": [ + { + "id": "apn-bid", + "impid": "imp-id-2", + "price": 0.3, + "w": 300, + "h": 600, + "crid": "creative-1", + "ext": { + "origbidcpm": 0.3, + "prebid": { + "meta": {}, + "type": "banner" + } + } + } + ] + } + ] + } + } +} \ No newline at end of file diff --git a/exchange/utils.go b/exchange/utils.go index 2074d7f2941..9b4ffd21f3e 100644 --- a/exchange/utils.go +++ b/exchange/utils.go @@ -9,8 +9,6 @@ import ( "math/rand" "strings" - "github.com/prebid/prebid-server/v2/ortb" - "github.com/prebid/go-gdpr/vendorconsent" gpplib "github.com/prebid/go-gpp" gppConstants "github.com/prebid/go-gpp/constants" @@ -22,7 +20,7 @@ import ( "github.com/prebid/prebid-server/v2/gdpr" "github.com/prebid/prebid-server/v2/metrics" "github.com/prebid/prebid-server/v2/openrtb_ext" - // "github.com/prebid/prebid-server/v2/ortb/validation" + "github.com/prebid/prebid-server/v2/ortb" "github.com/prebid/prebid-server/v2/privacy" "github.com/prebid/prebid-server/v2/privacy/ccpa" "github.com/prebid/prebid-server/v2/privacy/lmt" @@ -609,7 +607,11 @@ func splitImps(imps []openrtb2.Imp, requestValidator ortb.RequestValidator, requ return nil, err } impWrapper := openrtb_ext.ImpWrapper{Imp: &impCopy} - if err := requestValidator.ValidateImp(&impWrapper, i, requestAliases, hasStoredAuctionResponses, storedBidResponses); err != nil { + cfg := ortb.ValidationConfig{ + SkipBidderParams: true, + SkipNative: true, + } + if err := requestValidator.ValidateImp(&impWrapper, cfg, i, requestAliases, hasStoredAuctionResponses, 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), } diff --git a/ortb/request_validator.go b/ortb/request_validator.go index e38b8da2c55..efe047f986f 100644 --- a/ortb/request_validator.go +++ b/ortb/request_validator.go @@ -9,8 +9,13 @@ import ( "github.com/prebid/prebid-server/v2/stored_responses" ) +type ValidationConfig struct { + SkipBidderParams bool + SkipNative bool +} + type RequestValidator interface { - ValidateImp(imp *openrtb_ext.ImpWrapper, index int, aliases map[string]string, hasStoredAuctionResponses bool, storedBidResponses stored_responses.ImpBidderStoredResp) []error + ValidateImp(imp *openrtb_ext.ImpWrapper, cfg ValidationConfig, index int, aliases map[string]string, hasStoredAuctionResponses bool, storedBidResponses stored_responses.ImpBidderStoredResp) []error } func NewRequestValidator(bidderMap map[string]openrtb_ext.BidderName, disabledBidders map[string]string, paramsValidator openrtb_ext.BidderParamValidator) RequestValidator { @@ -27,7 +32,7 @@ type standardRequestValidator struct { paramsValidator openrtb_ext.BidderParamValidator } -func (srv *standardRequestValidator) ValidateImp(imp *openrtb_ext.ImpWrapper, index int, aliases map[string]string, hasStoredAuctionResponses bool, storedBidResponses stored_responses.ImpBidderStoredResp) []error { +func (srv *standardRequestValidator) ValidateImp(imp *openrtb_ext.ImpWrapper, cfg ValidationConfig, index int, aliases map[string]string, hasStoredAuctionResponses bool, storedBidResponses stored_responses.ImpBidderStoredResp) []error { if imp.ID == "" { return []error{fmt.Errorf("request.imp[%d] missing required field: \"id\"", index)} } @@ -52,15 +57,17 @@ func (srv *standardRequestValidator) ValidateImp(imp *openrtb_ext.ImpWrapper, in return []error{err} } - if err := fillAndValidateNative(imp.Native, index); err != nil { - return []error{err} + if !cfg.SkipNative { + if err := fillAndValidateNative(imp.Native, index); err != nil { + return []error{err} + } } if err := validatePmp(imp.PMP, index); err != nil { return []error{err} } - errL := srv.validateImpExt(imp, aliases, index, hasStoredAuctionResponses, storedBidResponses) + errL := srv.validateImpExt(imp, cfg, aliases, index, hasStoredAuctionResponses, storedBidResponses) if len(errL) != 0 { return errL } @@ -68,7 +75,7 @@ func (srv *standardRequestValidator) ValidateImp(imp *openrtb_ext.ImpWrapper, in return nil } -func (srv *standardRequestValidator) validateImpExt(imp *openrtb_ext.ImpWrapper, aliases map[string]string, impIndex int, hasStoredAuctionResponses bool, storedBidResp stored_responses.ImpBidderStoredResp) []error { +func (srv *standardRequestValidator) validateImpExt(imp *openrtb_ext.ImpWrapper, cfg ValidationConfig, aliases map[string]string, impIndex int, hasStoredAuctionResponses bool, storedBidResp stored_responses.ImpBidderStoredResp) []error { if len(imp.Ext) == 0 { return []error{fmt.Errorf("request.imp[%d].ext is required", impIndex)} } @@ -117,8 +124,10 @@ func (srv *standardRequestValidator) validateImpExt(imp *openrtb_ext.ImpWrapper, } if coreBidderNormalized, isValid := srv.bidderMap[coreBidder.String()]; isValid { - if err := srv.paramsValidator.Validate(coreBidderNormalized, ext); err != nil { - return []error{fmt.Errorf("request.imp[%d].ext.prebid.bidder.%s failed validation.\n%v", impIndex, bidder, err)} + if !cfg.SkipBidderParams { + if err := srv.paramsValidator.Validate(coreBidderNormalized, ext); err != nil { + return []error{fmt.Errorf("request.imp[%d].ext.prebid.bidder.%s failed validation.\n%v", impIndex, bidder, err)} + } } } else { if msg, isDisabled := srv.disabledBidders[bidder]; isDisabled { diff --git a/ortb/request_validator_test.go b/ortb/request_validator_test.go index 1c84a6ca1cb..1eb0708fcc8 100644 --- a/ortb/request_validator_test.go +++ b/ortb/request_validator_test.go @@ -13,10 +13,12 @@ import ( func TestValidateImpExt(t *testing.T) { type testCase struct { - description string - impExt json.RawMessage - expectedImpExt string - expectedErrs []error + description string + impExt json.RawMessage + cfg ValidationConfig + paramValidatorError error + expectedImpExt string + expectedErrs []error } testGroups := []struct { description string @@ -200,6 +202,31 @@ func TestValidateImpExt(t *testing.T) { }, }, }, + { + "Config tests", + []testCase{ + { + description: "Invalid Params", + impExt: json.RawMessage(`{"appnexus":{"placement_id_wrong_format":[]}}`), + cfg: ValidationConfig{ + SkipBidderParams: false, + }, + paramValidatorError: errors.New("params error"), + expectedImpExt: `{"appnexus":{"placement_id_wrong_format":[]}}`, + expectedErrs: []error{errors.New("request.imp[0].ext.prebid.bidder.appnexus failed validation.\nparams error")}, + }, + { + description: "Invalid Params - Skip Params Validation", + impExt: json.RawMessage(`{"appnexus":{"placement_id_wrong_format":[]}}`), + cfg: ValidationConfig{ + SkipBidderParams: true, + }, + paramValidatorError: errors.New("params error"), + expectedImpExt: `{"prebid":{"bidder":{"appnexus":{"placement_id_wrong_format":[]}}}}`, + expectedErrs: []error{}, + }, + }, + }, } for _, group := range testGroups { @@ -212,9 +239,11 @@ func TestValidateImpExt(t *testing.T) { rv := standardRequestValidator{ bidderMap: openrtb_ext.BuildBidderMap(), disabledBidders: disabledBidders, - paramsValidator: mockBidderParamValidator{}, + paramsValidator: mockBidderParamValidator{ + Error: test.paramValidatorError, + }, } - errs := rv.validateImpExt(impWrapper, nil, 0, false, nil) + errs := rv.validateImpExt(impWrapper, test.cfg, nil, 0, false, nil) assert.NoError(t, impWrapper.RebuildImp(), test.description+":rebuild_imp") @@ -229,9 +258,11 @@ func TestValidateImpExt(t *testing.T) { } } -type mockBidderParamValidator struct{} +type mockBidderParamValidator struct { + Error error +} func (v mockBidderParamValidator) Validate(name openrtb_ext.BidderName, ext json.RawMessage) error { - return nil + return v.Error } func (v mockBidderParamValidator) Schema(name openrtb_ext.BidderName) string { return "" }