Skip to content

Commit

Permalink
Merge pull request #1 from prebid/master
Browse files Browse the repository at this point in the history
Merge upstream
  • Loading branch information
Bill Newman committed Sep 11, 2020
2 parents 908477b + fa23f5c commit ce2973b
Show file tree
Hide file tree
Showing 24 changed files with 939 additions and 261 deletions.
1 change: 1 addition & 0 deletions adapters/rubicon/rubicon.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ var rubiSizeMap = map[rubiSize]int{
{w: 800, h: 250}: 125,
{w: 200, h: 600}: 126,
{w: 640, h: 320}: 156,
{w: 640, h: 360}: 198,
}

// defines the contract for bidrequest.user.ext.eids[i].ext
Expand Down
16 changes: 12 additions & 4 deletions endpoints/openrtb2/auction.go
Original file line number Diff line number Diff line change
Expand Up @@ -765,8 +765,8 @@ func (deps *endpointDeps) validateImpExt(imp *openrtb.Imp, aliases map[string]st
}

// Also accept bidder exts within imp[...].ext.prebid.bidder
// NOTE: This is not part of the official API, we are not expecting clients
// migrate from imp[...].ext.${BIDDER} to imp[...].ext.prebid.bidder.${BIDDER}
// NOTE: This is not part of the official API yet, so we are not expecting clients
// to migrate from imp[...].ext.${BIDDER} to imp[...].ext.prebid.bidder.${BIDDER}
// at this time
// https://github.com/prebid/prebid-server/pull/846#issuecomment-476352224
if rawPrebidExt, ok := bidderExts[openrtb_ext.PrebidExtKey]; ok {
Expand All @@ -785,7 +785,7 @@ func (deps *endpointDeps) validateImpExt(imp *openrtb.Imp, aliases map[string]st
/* Process all the bidder exts in the request */
disabledBidders := []string{}
for bidder, ext := range bidderExts {
if bidder != openrtb_ext.PrebidExtKey {
if isBidderToValidate(bidder) {
coreBidder := bidder
if tmp, isAlias := aliases[bidder]; isAlias {
coreBidder = tmp
Expand Down Expand Up @@ -820,12 +820,20 @@ func (deps *endpointDeps) validateImpExt(imp *openrtb.Imp, aliases map[string]st
// TODO #713 Fix this here
if len(bidderExts) < 1 {
errL = append(errL, fmt.Errorf("request.imp[%d].ext must contain at least one bidder", impIndex))
return errL
}

return errL
}

func isBidderToValidate(bidder string) bool {
// PrebidExtKey is a special case for the prebid config section and is not considered a bidder.

// FirstPartyDataContextExtKey is a special case for the first party data context section
// and is not considered a bidder.

return bidder != openrtb_ext.PrebidExtKey && bidder != openrtb_ext.FirstPartyDataContextExtKey
}

func (deps *endpointDeps) parseBidExt(ext json.RawMessage) (*openrtb_ext.ExtRequest, error) {
if len(ext) < 1 {
return nil, nil
Expand Down
95 changes: 89 additions & 6 deletions endpoints/openrtb2/auction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,16 @@ func TestGoodRequests(t *testing.T) {
supplementary.assert(t)
}

func TestFirstPartyDataRequests(t *testing.T) {
validRequests := &getResponseFromDirectory{
dir: "sample-requests/first-party-data",
payloadGetter: getRequestPayload,
messageGetter: nilReturner,
expectedCode: http.StatusOK,
}
validRequests.assert(t)
}

// TestGoodNativeRequests makes sure we return 200s on well-formed Native requests.
func TestGoodNativeRequests(t *testing.T) {
tests := &getResponseFromDirectory{
Expand Down Expand Up @@ -1127,10 +1137,73 @@ func TestDisabledBidder(t *testing.T) {
}
}

func TestValidateImpExtDisabledBidder(t *testing.T) {
imp := &openrtb.Imp{
Ext: json.RawMessage(`{"appnexus":{"placement_id":555},"unknownbidder":{"foo":"bar"}}`),
func TestValidateImpExt(t *testing.T) {
testCases := []struct {
description string
impExt json.RawMessage
expectedImpExt string
expectedErrs []error
}{
{
description: "Empty",
impExt: nil,
expectedImpExt: "",
expectedErrs: []error{errors.New("request.imp[0].ext is required")},
},
{
description: "Valid Bidder",
impExt: json.RawMessage(`{"appnexus":{"placement_id":555}}`),
expectedImpExt: `{"appnexus":{"placement_id":555}}`,
expectedErrs: []error{},
},
{
description: "Valid Bidder + Disabled Bidder",
impExt: json.RawMessage(`{"appnexus":{"placement_id":555},"unknownbidder":{"foo":"bar"}}`),
expectedImpExt: `{"appnexus":{"placement_id":555}}`,
expectedErrs: []error{&errortypes.BidderTemporarilyDisabled{Message: "The bidder 'unknownbidder' has been disabled."}},
},
{
description: "Valid Bidder + Disabled Bidder + First Party Data Context",
impExt: json.RawMessage(`{"appnexus":{"placement_id":555},"unknownbidder":{"foo":"bar"},"context":{"data":{"keywords":"prebid server example"}}}`),
expectedImpExt: `{"appnexus":{"placement_id":555},"context":{"data":{"keywords":"prebid server example"}}}`,
expectedErrs: []error{&errortypes.BidderTemporarilyDisabled{Message: "The bidder 'unknownbidder' has been disabled."}},
},
{
description: "Valid Bidder + First Party Data Context",
impExt: json.RawMessage(`{"appnexus":{"placement_id":555},"context":{"data":{"keywords":"prebid server example"}}}`),
expectedImpExt: `{"appnexus":{"placement_id":555},"context":{"data":{"keywords":"prebid server example"}}}`,
expectedErrs: []error{},
},
{
description: "Valid Prebid Ext Bidder",
impExt: json.RawMessage(`{"prebid":{"bidder":{"appnexus":{"placement_id":555}}}}`),
expectedImpExt: `{"prebid":{"bidder":{"appnexus":{"placement_id":555}}}}`,
expectedErrs: []error{},
// request.imp[x].ext.prebid.bidder.{biddername} is only promoted/copied to request.ext.{biddername} if there is at least one disabled bidder.
},
{
description: "Valid Prebid Ext Bidder + First Party Data Context",
impExt: json.RawMessage(`{"prebid":{"bidder":{"appnexus":{"placement_id":555}}} ,"context":{"data":{"keywords":"prebid server example"}}}`),
expectedImpExt: `{"prebid":{"bidder":{"appnexus":{"placement_id":555}}},"context":{"data":{"keywords":"prebid server example"}}}`,
expectedErrs: []error{},
// request.imp[x].ext.prebid.bidder.{biddername} is only promoted/copied to request.ext.{biddername} if there is at least one disabled bidder.
},
{
description: "Valid Prebid Ext Bidder + Disabled Bidder",
impExt: json.RawMessage(`{"prebid":{"bidder":{"appnexus":{"placement_id":555},"unknownbidder":{"foo":"bar"}}}}`),
expectedImpExt: `{"prebid":{"bidder":{"appnexus":{"placement_id": 555},"unknownbidder":{"foo":"bar"}}},"appnexus":{"placement_id":555}}`,
expectedErrs: []error{&errortypes.BidderTemporarilyDisabled{Message: "The bidder 'unknownbidder' has been disabled."}},
// request.imp[x].ext.prebid.bidder.{biddername} disabled bidders are not removed. if there is a disabled bidder, the valid ones are promoted/copied to request.ext.{biddername}.
},
{
description: "Valid Prebid Ext Bidder + Disabled Bidder + First Party Data Context",
impExt: json.RawMessage(`{"prebid":{"bidder":{"appnexus":{"placement_id":555},"unknownbidder":{"foo":"bar"}}},"context":{"data":{"keywords":"prebid server example"}}}`),
expectedImpExt: `{"prebid":{"bidder":{"appnexus":{"placement_id": 555},"unknownbidder":{"foo":"bar"}}},"appnexus":{"placement_id":555},"context":{"data":{"keywords":"prebid server example"}}}`,
expectedErrs: []error{&errortypes.BidderTemporarilyDisabled{Message: "The bidder 'unknownbidder' has been disabled."}},
// request.imp[x].ext.prebid.bidder.{biddername} disabled bidders are not removed. if there is a disabled bidder, the valid ones are promoted/copied to request.ext.{biddername}.
},
}

deps := &endpointDeps{
&nobidExchange{},
newParamsValidator(t),
Expand All @@ -1149,9 +1222,19 @@ func TestValidateImpExtDisabledBidder(t *testing.T) {
nil,
hardcodedResponseIPValidator{response: true},
}
errs := deps.validateImpExt(imp, nil, 0)
assert.JSONEq(t, `{"appnexus":{"placement_id":555}}`, string(imp.Ext))
assert.Equal(t, []error{&errortypes.BidderTemporarilyDisabled{Message: "The bidder 'unknownbidder' has been disabled."}}, errs)

for _, test := range testCases {
imp := &openrtb.Imp{Ext: test.impExt}

errs := deps.validateImpExt(imp, nil, 0)

if len(test.expectedImpExt) > 0 {
assert.JSONEq(t, test.expectedImpExt, string(imp.Ext))
} else {
assert.Empty(t, imp.Ext)
}
assert.Equal(t, test.expectedErrs, errs)
}
}

func validRequest(t *testing.T, filename string) string {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
{
"description": "The imp.ext.context field is valid for First Party Data and should be exempted from bidder name validation.",

"requestPayload": {
"id": "some-request-id",
"site": {
"page": "test.somepage.com"
},
"imp": [{
"id": "some-imp-id",
"banner": {
"format": [{
"w": 600,
"h": 500
}, {
"w": 300,
"h": 600
}]
},
"ext": {
"appnexus": {
"placementId": 12883451
},
"context": {
"data": {
"keywords": "prebid server example"
}
}
}
}]
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
{
"description": "The imp.ext.context field is valid for First Party Data and should be exempted from bidder name validation.",

"requestPayload": {
"id": "some-request-id",
"site": {
"page": "test.somepage.com"
},
"imp": [{
"id": "some-imp-id",
"banner": {
"format": [{
"w": 600,
"h": 500
}, {
"w": 300,
"h": 600
}]
},
"ext": {
"prebid": {
"bidder": {
"appnexus": {
"placementId": 12883451
}
}
},
"context": {
"data": {
"keywords": "prebid server example"
}
}
}
}]
}
}
Loading

0 comments on commit ce2973b

Please sign in to comment.