Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge upstream #1

Merged
merged 3 commits into from
Sep 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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