Skip to content

Commit

Permalink
Imp FPD: Skip bidder params and native validation (#3720)
Browse files Browse the repository at this point in the history
  • Loading branch information
bsardo authored Jun 5, 2024
1 parent ecf3171 commit 32fdbc4
Show file tree
Hide file tree
Showing 8 changed files with 241 additions and 24 deletions.
2 changes: 1 addition & 1 deletion endpoints/openrtb2/auction.go
Original file line number Diff line number Diff line change
Expand Up @@ -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...)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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"
}
2 changes: 1 addition & 1 deletion exchange/exchange_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
4 changes: 2 additions & 2 deletions exchange/exchangetest/imp-fpd-multiple-imp.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down Expand Up @@ -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
},
Expand Down
137 changes: 137 additions & 0 deletions exchange/exchangetest/imp-fpd-skipped-validation.json
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
]
}
]
}
}
}
10 changes: 6 additions & 4 deletions exchange/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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),
}
Expand Down
25 changes: 17 additions & 8 deletions ortb/request_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)}
}
Expand All @@ -52,23 +57,25 @@ 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
}

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)}
}
Expand Down Expand Up @@ -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 {
Expand Down
47 changes: 39 additions & 8 deletions ortb/request_validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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")

Expand All @@ -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 "" }

0 comments on commit 32fdbc4

Please sign in to comment.