From efd916ad35232db3fa0f3c6e1cbaa8bec5ba23e7 Mon Sep 17 00:00:00 2001 From: Dave Bemiller <27972385+dbemiller@users.noreply.github.com> Date: Tue, 9 Oct 2018 15:09:02 -0400 Subject: [PATCH 1/6] Started adding some code to validate the amp ext, and some unit tests which dont pass yet. --- endpoints/openrtb2/amp_auction.go | 5 +++ endpoints/openrtb2/auction.go | 23 ++++++++-- endpoints/openrtb2/auction_test.go | 22 ++++++++++ .../valid-whole/supplementary/site-amp.json | 44 +++++++++++++++++++ openrtb_ext/site.go | 34 ++++++++++++++ openrtb_ext/site_test.go | 24 ++++++++++ 6 files changed, 148 insertions(+), 4 deletions(-) create mode 100644 endpoints/openrtb2/sample-requests/valid-whole/supplementary/site-amp.json create mode 100644 openrtb_ext/site.go create mode 100644 openrtb_ext/site_test.go diff --git a/endpoints/openrtb2/amp_auction.go b/endpoints/openrtb2/amp_auction.go index 2dbc8ac0016..107325138c9 100644 --- a/endpoints/openrtb2/amp_auction.go +++ b/endpoints/openrtb2/amp_auction.go @@ -325,6 +325,11 @@ func (deps *endpointDeps) overrideWithParams(httpRequest *http.Request, req *ope req.Site.Domain = domain } } + if data, err := json.Marshal(openrtb_ext.ExtSite{ + AMP: 1, + }); err != nil { + req.Site.Ext = data + } slot := httpRequest.FormValue("slot") if slot != "" { diff --git a/endpoints/openrtb2/auction.go b/endpoints/openrtb2/auction.go index 51ecaf8e994..5ff9544d3ad 100644 --- a/endpoints/openrtb2/auction.go +++ b/endpoints/openrtb2/auction.go @@ -684,6 +684,12 @@ func (deps *endpointDeps) validateSite(site *openrtb.Site) error { if site != nil && site.ID == "" && site.Page == "" { return errors.New("request.site should include at least one of request.site.id or request.site.page.") } + if len(site.Ext) > 0 { + var s openrtb_ext.ExtSite + if err := json.Unmarshal(site.Ext, &s); err != nil { + return err + } + } return nil } @@ -768,13 +774,13 @@ func setAuctionTypeImplicitly(bidReq *openrtb.BidRequest) { // setSiteImplicitly uses implicit info from httpReq to populate bidReq.Site func setSiteImplicitly(httpReq *http.Request, bidReq *openrtb.BidRequest) { - if bidReq.Site == nil || bidReq.Site.Page == "" || bidReq.Site.Domain == "" { + if bidReq.Site == nil { + bidReq.Site = &openrtb.Site{} + } + if bidReq.Site.Page == "" || bidReq.Site.Domain == "" { referrerCandidate := httpReq.Referer() if parsedUrl, err := url.Parse(referrerCandidate); err == nil { if domain, err := publicsuffix.EffectiveTLDPlusOne(parsedUrl.Host); err == nil { - if bidReq.Site == nil { - bidReq.Site = &openrtb.Site{} - } if bidReq.Site.Domain == "" { bidReq.Site.Domain = domain } @@ -787,6 +793,15 @@ func setSiteImplicitly(httpReq *http.Request, bidReq *openrtb.BidRequest) { } } } + if len(bidReq.Site.Ext) > 0 { + if _, dataType, _, _ := jsonparser.Get(bidReq.Site.Ext, "amp"); dataType == jsonparser.NotExist { + if val, err := jsonparser.Set(bidReq.Site.Ext, []byte("0"), "amp"); err == nil { + bidReq.Site.Ext = val + } + } + } else { + bidReq.Site.Ext = json.RawMessage(`{"amp":0}`) + } } func setImpsImplicitly(httpReq *http.Request, imps []openrtb.Imp) { diff --git a/endpoints/openrtb2/auction_test.go b/endpoints/openrtb2/auction_test.go index 920065405f2..54791c28ce4 100644 --- a/endpoints/openrtb2/auction_test.go +++ b/endpoints/openrtb2/auction_test.go @@ -491,6 +491,28 @@ func TestTimeoutParser(t *testing.T) { } } +func TestImplicitAMP(t *testing.T) { + httpReq, err := http.NewRequest("POST", "/openrtb2/auction", strings.NewReader(validRequest(t, "site.json"))) + if !assert.NoError(t, err) { + return + } + + var bidReq openrtb.BidRequest + setSiteImplicitly(httpReq, &bidReq) + assert.JSONEq(t, `{"amp":0}`, string(bidReq.Site.Ext)) +} + +func TestExplicitAMP(t *testing.T) { + httpReq, err := http.NewRequest("POST", "/openrtb2/auction", strings.NewReader(validRequest(t, "site-amp.json"))) + if !assert.NoError(t, err) { + return + } + + var bidReq openrtb.BidRequest + setSiteImplicitly(httpReq, &bidReq) + assert.JSONEq(t, `{"amp":1}`, string(bidReq.Site.Ext)) +} + // TestContentType prevents #328 func TestContentType(t *testing.T) { endpoint, _ := NewEndpoint( diff --git a/endpoints/openrtb2/sample-requests/valid-whole/supplementary/site-amp.json b/endpoints/openrtb2/sample-requests/valid-whole/supplementary/site-amp.json new file mode 100644 index 00000000000..c0dbb355fea --- /dev/null +++ b/endpoints/openrtb2/sample-requests/valid-whole/supplementary/site-amp.json @@ -0,0 +1,44 @@ +{ + "id": "some-request-id", + "site": { + "page": "test.somepage.com", + "ext": { + "amp": 1 + } + }, + "imp": [ + { + "id": "my-imp-id", + "banner": { + "format": [ + { + "w": 300, + "h": 600 + } + ] + }, + "pmp": { + "deals": [ + { + "id": "some-deal-id" + } + ] + }, + "ext": { + "appnexus": { + "placementId": 10433394 + } + } + } + ], + "ext": { + "prebid": { + "targeting": { + "pricegranularity": "low" + }, + "cache": { + "bids": {} + } + } + } +} diff --git a/openrtb_ext/site.go b/openrtb_ext/site.go new file mode 100644 index 00000000000..88b43305205 --- /dev/null +++ b/openrtb_ext/site.go @@ -0,0 +1,34 @@ +package openrtb_ext + +import ( + "errors" + + "github.com/buger/jsonparser" +) + +// ExtSite defines the contract for bidrequest.site.ext +type ExtSite struct { + // AMP should be 1 if the request comes from an AMP page, and 0 if not. + AMP int8 `json:"amp"` +} + +func (es *ExtSite) UnmarshalJSON(b []byte) error { + if len(b) == 0 { + return errors.New("request.site.ext must have some data in it") + } + if value, dataType, _, _ := jsonparser.Get(b, "amp"); dataType != jsonparser.NotExist && dataType != jsonparser.Number { + return errors.New(`request.site.ext.amp must be either 1, 0, or undefined`) + } else if len(value) != 1 { + return errors.New(`request.site.ext.amp must be either 1, 0, or undefined`) + } else { + switch value[0] { + case byte(48): // 0 + es.AMP = 0 + case byte(49): // 1 + es.AMP = 1 + default: + return errors.New(`request.site.ext.amp must be either 1, 0, or undefined`) + } + } + return nil +} diff --git a/openrtb_ext/site_test.go b/openrtb_ext/site_test.go new file mode 100644 index 00000000000..681ee1c6b1a --- /dev/null +++ b/openrtb_ext/site_test.go @@ -0,0 +1,24 @@ +package openrtb_ext_test + +import ( + "encoding/json" + "testing" + + "github.com/prebid/prebid-server/openrtb_ext" + "github.com/stretchr/testify/assert" +) + +func TestInvalidSiteExt(t *testing.T) { + var s openrtb_ext.ExtSite + assert.EqualError(t, json.Unmarshal([]byte(`{"amp":-1}`), &s), "request.site.ext.amp must be either 1, 0, or undefined") + assert.EqualError(t, json.Unmarshal([]byte(`{"amp":2}`), &s), "request.site.ext.amp must be either 1, 0, or undefined") + assert.EqualError(t, json.Unmarshal([]byte(`{"amp":true}`), &s), "request.site.ext.amp must be either 1, 0, or undefined") + assert.EqualError(t, json.Unmarshal([]byte(`{"amp":null}`), &s), "request.site.ext.amp must be either 1, 0, or undefined") + assert.EqualError(t, json.Unmarshal([]byte(`{"amp":"1"}`), &s), "request.site.ext.amp must be either 1, 0, or undefined") +} + +func TestValidSiteExt(t *testing.T) { + var s openrtb_ext.ExtSite + assert.NoError(t, json.Unmarshal([]byte(`{"amp":0}`), &s)) + assert.NoError(t, json.Unmarshal([]byte(`{"amp":1}`), &s)) +} From f6b705972a8e59d65d54e923c42b3726a4db075b Mon Sep 17 00:00:00 2001 From: Dave Bemiller <27972385+dbemiller@users.noreply.github.com> Date: Wed, 10 Oct 2018 09:42:29 -0400 Subject: [PATCH 2/6] Fixed some bugs. Added more tests. --- endpoints/openrtb2/amp_auction.go | 21 ++++----- endpoints/openrtb2/amp_auction_test.go | 25 ++++++++++- endpoints/openrtb2/auction.go | 14 +++--- endpoints/openrtb2/auction_test.go | 23 +++++++++- .../invalid-whole/site-ext-amp.json | 44 +++++++++++++++++++ openrtb_ext/site_test.go | 4 ++ 6 files changed, 112 insertions(+), 19 deletions(-) create mode 100644 endpoints/openrtb2/sample-requests/invalid-whole/site-ext-amp.json diff --git a/endpoints/openrtb2/amp_auction.go b/endpoints/openrtb2/amp_auction.go index 107325138c9..998b0f50622 100644 --- a/endpoints/openrtb2/amp_auction.go +++ b/endpoints/openrtb2/amp_auction.go @@ -280,6 +280,11 @@ func (deps *endpointDeps) loadRequestJSONForAmp(httpRequest *http.Request) (req return } + if req.App != nil { + errs = []error{errors.New("request.app must not exist in AMP stored requests.")} + return + } + // Force HTTPS as AMP requires it, but pubs can forget to set it. if req.Imp[0].Secure == nil { secure := int8(1) @@ -294,6 +299,9 @@ func (deps *endpointDeps) loadRequestJSONForAmp(httpRequest *http.Request) (req } func (deps *endpointDeps) overrideWithParams(httpRequest *http.Request, req *openrtb.BidRequest) { + if req.Site == nil { + req.Site = &openrtb.Site{} + } // Override the stored request sizes with AMP ones, if they exist. if req.Imp[0].Banner != nil { width := parseFormInt(httpRequest, "w", 0) @@ -311,11 +319,7 @@ func (deps *endpointDeps) overrideWithParams(httpRequest *http.Request, req *ope canonicalURL := httpRequest.FormValue("curl") if canonicalURL != "" { - if req.Site == nil { - req.Site = &openrtb.Site{Page: canonicalURL} - } else { - req.Site.Page = canonicalURL - } + req.Site.Page = canonicalURL // Fixes #683 if parsedURL, err := url.Parse(canonicalURL); err == nil { domain := parsedURL.Host @@ -325,11 +329,8 @@ func (deps *endpointDeps) overrideWithParams(httpRequest *http.Request, req *ope req.Site.Domain = domain } } - if data, err := json.Marshal(openrtb_ext.ExtSite{ - AMP: 1, - }); err != nil { - req.Site.Ext = data - } + + setAmpExt(req.Site, "1") slot := httpRequest.FormValue("slot") if slot != "" { diff --git a/endpoints/openrtb2/amp_auction_test.go b/endpoints/openrtb2/amp_auction_test.go index ee39f5a3809..1eb4f6d2139 100644 --- a/endpoints/openrtb2/amp_auction_test.go +++ b/endpoints/openrtb2/amp_auction_test.go @@ -30,12 +30,10 @@ func TestGoodAmpRequests(t *testing.T) { goodRequests := map[string]json.RawMessage{ "1": json.RawMessage(validRequest(t, "aliased-buyeruids.json")), "2": json.RawMessage(validRequest(t, "aliases.json")), - "3": json.RawMessage(validRequest(t, "app.json")), "4": json.RawMessage(validRequest(t, "digitrust.json")), "5": json.RawMessage(validRequest(t, "gdpr-no-consentstring.json")), "6": json.RawMessage(validRequest(t, "gdpr.json")), "7": json.RawMessage(validRequest(t, "site.json")), - "8": json.RawMessage(validRequest(t, "timeout.json")), "9": json.RawMessage(validRequest(t, "user.json")), } @@ -99,6 +97,29 @@ func TestAMPPageInfo(t *testing.T) { assert.Equal(t, "test.somepage.co.uk", exchange.lastRequest.Site.Domain) } +func TestAMPSiteExt(t *testing.T) { + stored := map[string]json.RawMessage{ + "1": json.RawMessage(validRequest(t, "site.json")), + } + theMetrics := pbsmetrics.NewMetrics(metrics.NewRegistry(), openrtb_ext.BidderList()) + exchange := &mockAmpExchange{} + endpoint, _ := NewAmpEndpoint(exchange, newParamsValidator(t), &mockAmpStoredReqFetcher{stored}, &config.Configuration{MaxRequestSize: maxSize}, theMetrics, analyticsConf.NewPBSAnalytics(&config.Analytics{})) + request, err := http.NewRequest("GET", "/openrtb2/auction/amp?tag_id=1", nil) + if !assert.NoError(t, err) { + return + } + recorder := httptest.NewRecorder() + endpoint(recorder, request, nil) + + if !assert.NotNil(t, exchange.lastRequest, "Endpoint responded with %d: %s", recorder.Code, recorder.Body.String()) { + return + } + if !assert.NotNil(t, exchange.lastRequest.Site) { + return + } + assert.JSONEq(t, `{"amp":1}`, string(exchange.lastRequest.Site.Ext)) +} + // TestBadRequests makes sure we return 400's on bad requests. func TestAmpBadRequests(t *testing.T) { files := fetchFiles(t, "sample-requests/invalid-whole") diff --git a/endpoints/openrtb2/auction.go b/endpoints/openrtb2/auction.go index 5ff9544d3ad..e6957fe813d 100644 --- a/endpoints/openrtb2/auction.go +++ b/endpoints/openrtb2/auction.go @@ -793,14 +793,18 @@ func setSiteImplicitly(httpReq *http.Request, bidReq *openrtb.BidRequest) { } } } - if len(bidReq.Site.Ext) > 0 { - if _, dataType, _, _ := jsonparser.Get(bidReq.Site.Ext, "amp"); dataType == jsonparser.NotExist { - if val, err := jsonparser.Set(bidReq.Site.Ext, []byte("0"), "amp"); err == nil { - bidReq.Site.Ext = val + setAmpExt(bidReq.Site, "0") +} + +func setAmpExt(site *openrtb.Site, value string) { + if len(site.Ext) > 0 { + if _, dataType, _, _ := jsonparser.Get(site.Ext, "amp"); dataType == jsonparser.NotExist { + if val, err := jsonparser.Set(site.Ext, []byte(value), "amp"); err == nil { + site.Ext = val } } } else { - bidReq.Site.Ext = json.RawMessage(`{"amp":0}`) + site.Ext = json.RawMessage(`{"amp":` + value + `}`) } } diff --git a/endpoints/openrtb2/auction_test.go b/endpoints/openrtb2/auction_test.go index 54791c28ce4..e22462cdc44 100644 --- a/endpoints/openrtb2/auction_test.go +++ b/endpoints/openrtb2/auction_test.go @@ -491,7 +491,7 @@ func TestTimeoutParser(t *testing.T) { } } -func TestImplicitAMP(t *testing.T) { +func TestImplicitAMPNoExt(t *testing.T) { httpReq, err := http.NewRequest("POST", "/openrtb2/auction", strings.NewReader(validRequest(t, "site.json"))) if !assert.NoError(t, err) { return @@ -502,13 +502,32 @@ func TestImplicitAMP(t *testing.T) { assert.JSONEq(t, `{"amp":0}`, string(bidReq.Site.Ext)) } +func TestImplicitAMPOtherExt(t *testing.T) { + httpReq, err := http.NewRequest("POST", "/openrtb2/auction", strings.NewReader(validRequest(t, "site.json"))) + if !assert.NoError(t, err) { + return + } + + bidReq := openrtb.BidRequest{ + Site: &openrtb.Site{ + Ext: json.RawMessage(`{"other":true}`), + }, + } + setSiteImplicitly(httpReq, &bidReq) + assert.JSONEq(t, `{"amp":0,"other":true}`, string(bidReq.Site.Ext)) +} + func TestExplicitAMP(t *testing.T) { httpReq, err := http.NewRequest("POST", "/openrtb2/auction", strings.NewReader(validRequest(t, "site-amp.json"))) if !assert.NoError(t, err) { return } - var bidReq openrtb.BidRequest + bidReq := openrtb.BidRequest{ + Site: &openrtb.Site{ + Ext: json.RawMessage(`{"amp":1}`), + }, + } setSiteImplicitly(httpReq, &bidReq) assert.JSONEq(t, `{"amp":1}`, string(bidReq.Site.Ext)) } diff --git a/endpoints/openrtb2/sample-requests/invalid-whole/site-ext-amp.json b/endpoints/openrtb2/sample-requests/invalid-whole/site-ext-amp.json new file mode 100644 index 00000000000..b769f41e233 --- /dev/null +++ b/endpoints/openrtb2/sample-requests/invalid-whole/site-ext-amp.json @@ -0,0 +1,44 @@ +{ + "id": "some-request-id", + "site": { + "page": "test.somepage.com", + "ext": { + "amp": 2 + } + }, + "imp": [ + { + "id": "my-imp-id", + "banner": { + "format": [ + { + "w": 300, + "h": 600 + } + ] + }, + "pmp": { + "deals": [ + { + "id": "some-deal-id" + } + ] + }, + "ext": { + "appnexus": { + "placementId": 10433394 + } + } + } + ], + "ext": { + "prebid": { + "targeting": { + "pricegranularity": "low" + }, + "cache": { + "bids": {} + } + } + } +} diff --git a/openrtb_ext/site_test.go b/openrtb_ext/site_test.go index 681ee1c6b1a..67ec6cc4f99 100644 --- a/openrtb_ext/site_test.go +++ b/openrtb_ext/site_test.go @@ -20,5 +20,9 @@ func TestInvalidSiteExt(t *testing.T) { func TestValidSiteExt(t *testing.T) { var s openrtb_ext.ExtSite assert.NoError(t, json.Unmarshal([]byte(`{"amp":0}`), &s)) + assert.EqualValues(t, 0, s.AMP) assert.NoError(t, json.Unmarshal([]byte(`{"amp":1}`), &s)) + assert.EqualValues(t, 1, s.AMP) + assert.NoError(t, json.Unmarshal([]byte(`{"amp": 1 }`), &s)) + assert.EqualValues(t, 1, s.AMP) } From 6489fdddbab2c11cd32df3e13f6b6e58390c6aff Mon Sep 17 00:00:00 2001 From: Dave Bemiller <27972385+dbemiller@users.noreply.github.com> Date: Wed, 10 Oct 2018 09:50:43 -0400 Subject: [PATCH 3/6] Updated the docs. --- docs/endpoints/openrtb2/auction.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/docs/endpoints/openrtb2/auction.md b/docs/endpoints/openrtb2/auction.md index 81c8c03d371..ed816a11876 100644 --- a/docs/endpoints/openrtb2/auction.md +++ b/docs/endpoints/openrtb2/auction.md @@ -80,7 +80,11 @@ The only exception here is the top-level `BidResponse`, because it's bidder-inde `ext.{anyBidderCode}` and `ext.bidder` extensions are defined by bidders. `ext.prebid` extensions are defined by Prebid Server. -Exceptions are made for DigiTrust and GDPR, so that we define `ext` according to the official recommendations. +Exceptions are made for extensions with "standard" recommendations: + +- `request.user.ext.digitrust` -- To support Digitrust support +- `request.regs.ext.gdpr` and `request.user.ext.consent` -- To support GDPR +- `request.site.ext.amp` -- To identify AMP as the request source #### Bid Adjustments @@ -254,7 +258,7 @@ For example, a request may return this in `response.ext` ], "rubicon": [ { - "code": 1, + "code": 1, "message": "The request exceeded the timeout allocated" } ] From 75c43e1a2faba1f1caf57dda70f17f0e29ca405c Mon Sep 17 00:00:00 2001 From: Dave Bemiller <27972385+dbemiller@users.noreply.github.com> Date: Wed, 10 Oct 2018 10:07:04 -0400 Subject: [PATCH 4/6] Fixed some tests... but not all. --- endpoints/openrtb2/amp_auction_test.go | 1 - endpoints/openrtb2/auction.go | 6 +- .../invalid-whole/site-ext-amp.json | 75 ++++++++++--------- 3 files changed, 44 insertions(+), 38 deletions(-) diff --git a/endpoints/openrtb2/amp_auction_test.go b/endpoints/openrtb2/amp_auction_test.go index 1eb4f6d2139..b1cca1039d5 100644 --- a/endpoints/openrtb2/amp_auction_test.go +++ b/endpoints/openrtb2/amp_auction_test.go @@ -147,7 +147,6 @@ func TestAmpBadRequests(t *testing.T) { // TestAmpDebug makes sure we get debug information back when requested func TestAmpDebug(t *testing.T) { requests := map[string]json.RawMessage{ - "1": json.RawMessage(validRequest(t, "app.json")), "2": json.RawMessage(validRequest(t, "site.json")), } diff --git a/endpoints/openrtb2/auction.go b/endpoints/openrtb2/auction.go index e6957fe813d..63238ba2afc 100644 --- a/endpoints/openrtb2/auction.go +++ b/endpoints/openrtb2/auction.go @@ -681,7 +681,11 @@ func (deps *endpointDeps) validateAliases(aliases map[string]string) error { } func (deps *endpointDeps) validateSite(site *openrtb.Site) error { - if site != nil && site.ID == "" && site.Page == "" { + if site == nil { + return nil + } + + if site.ID == "" && site.Page == "" { return errors.New("request.site should include at least one of request.site.id or request.site.page.") } if len(site.Ext) > 0 { diff --git a/endpoints/openrtb2/sample-requests/invalid-whole/site-ext-amp.json b/endpoints/openrtb2/sample-requests/invalid-whole/site-ext-amp.json index b769f41e233..17cd68b9232 100644 --- a/endpoints/openrtb2/sample-requests/invalid-whole/site-ext-amp.json +++ b/endpoints/openrtb2/sample-requests/invalid-whole/site-ext-amp.json @@ -1,43 +1,46 @@ { - "id": "some-request-id", - "site": { - "page": "test.somepage.com", - "ext": { - "amp": 2 - } - }, - "imp": [ - { - "id": "my-imp-id", - "banner": { - "format": [ - { - "w": 300, - "h": 600 - } - ] - }, - "pmp": { - "deals": [ - { - "id": "some-deal-id" - } - ] - }, + "message": "Invalid request: request.site.ext.amp must be either 1, 0, or undefined\n", + "requestPayload": { + "id": "some-request-id", + "site": { + "page": "test.somepage.com", "ext": { - "appnexus": { - "placementId": 10433394 + "amp": 2 + } + }, + "imp": [ + { + "id": "my-imp-id", + "banner": { + "format": [ + { + "w": 300, + "h": 600 + } + ] + }, + "pmp": { + "deals": [ + { + "id": "some-deal-id" + } + ] + }, + "ext": { + "appnexus": { + "placementId": 10433394 + } } } - } - ], - "ext": { - "prebid": { - "targeting": { - "pricegranularity": "low" - }, - "cache": { - "bids": {} + ], + "ext": { + "prebid": { + "targeting": { + "pricegranularity": "low" + }, + "cache": { + "bids": {} + } } } } From 2fc6ec58b86107406f0164254f210d22f4e39ea4 Mon Sep 17 00:00:00 2001 From: Dave Bemiller <27972385+dbemiller@users.noreply.github.com> Date: Wed, 10 Oct 2018 10:50:21 -0400 Subject: [PATCH 5/6] Fixed last remaining bug. --- endpoints/openrtb2/auction.go | 12 +++++++----- endpoints/openrtb2/auction_test.go | 4 +++- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/endpoints/openrtb2/auction.go b/endpoints/openrtb2/auction.go index 63238ba2afc..b15a23aa32c 100644 --- a/endpoints/openrtb2/auction.go +++ b/endpoints/openrtb2/auction.go @@ -778,13 +778,13 @@ func setAuctionTypeImplicitly(bidReq *openrtb.BidRequest) { // setSiteImplicitly uses implicit info from httpReq to populate bidReq.Site func setSiteImplicitly(httpReq *http.Request, bidReq *openrtb.BidRequest) { - if bidReq.Site == nil { - bidReq.Site = &openrtb.Site{} - } - if bidReq.Site.Page == "" || bidReq.Site.Domain == "" { + if bidReq.Site == nil || bidReq.Site.Page == "" || bidReq.Site.Domain == "" { referrerCandidate := httpReq.Referer() if parsedUrl, err := url.Parse(referrerCandidate); err == nil { if domain, err := publicsuffix.EffectiveTLDPlusOne(parsedUrl.Host); err == nil { + if bidReq.Site == nil { + bidReq.Site = &openrtb.Site{} + } if bidReq.Site.Domain == "" { bidReq.Site.Domain = domain } @@ -797,7 +797,9 @@ func setSiteImplicitly(httpReq *http.Request, bidReq *openrtb.BidRequest) { } } } - setAmpExt(bidReq.Site, "0") + if bidReq.Site != nil { + setAmpExt(bidReq.Site, "0") + } } func setAmpExt(site *openrtb.Site, value string) { diff --git a/endpoints/openrtb2/auction_test.go b/endpoints/openrtb2/auction_test.go index e22462cdc44..e8ddb0180f8 100644 --- a/endpoints/openrtb2/auction_test.go +++ b/endpoints/openrtb2/auction_test.go @@ -497,7 +497,9 @@ func TestImplicitAMPNoExt(t *testing.T) { return } - var bidReq openrtb.BidRequest + bidReq := openrtb.BidRequest{ + Site: &openrtb.Site{}, + } setSiteImplicitly(httpReq, &bidReq) assert.JSONEq(t, `{"amp":0}`, string(bidReq.Site.Ext)) } From 29bbf818445ded33d895f3d6dec957447e46ff09 Mon Sep 17 00:00:00 2001 From: Dave Bemiller <27972385+dbemiller@users.noreply.github.com> Date: Wed, 10 Oct 2018 14:51:57 -0400 Subject: [PATCH 6/6] Moved setAmpExt to the amp_auction file. --- endpoints/openrtb2/amp_auction.go | 13 +++++++++++++ endpoints/openrtb2/auction.go | 12 ------------ 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/endpoints/openrtb2/amp_auction.go b/endpoints/openrtb2/amp_auction.go index 998b0f50622..c93c03e85e3 100644 --- a/endpoints/openrtb2/amp_auction.go +++ b/endpoints/openrtb2/amp_auction.go @@ -12,6 +12,7 @@ import ( "strings" "time" + "github.com/buger/jsonparser" "github.com/golang/glog" "github.com/julienschmidt/httprouter" "github.com/mxmCherry/openrtb" @@ -461,3 +462,15 @@ func defaultRequestExt(req *openrtb.BidRequest) (errs []error) { return } + +func setAmpExt(site *openrtb.Site, value string) { + if len(site.Ext) > 0 { + if _, dataType, _, _ := jsonparser.Get(site.Ext, "amp"); dataType == jsonparser.NotExist { + if val, err := jsonparser.Set(site.Ext, []byte(value), "amp"); err == nil { + site.Ext = val + } + } + } else { + site.Ext = json.RawMessage(`{"amp":` + value + `}`) + } +} diff --git a/endpoints/openrtb2/auction.go b/endpoints/openrtb2/auction.go index b15a23aa32c..3aa122a6715 100644 --- a/endpoints/openrtb2/auction.go +++ b/endpoints/openrtb2/auction.go @@ -802,18 +802,6 @@ func setSiteImplicitly(httpReq *http.Request, bidReq *openrtb.BidRequest) { } } -func setAmpExt(site *openrtb.Site, value string) { - if len(site.Ext) > 0 { - if _, dataType, _, _ := jsonparser.Get(site.Ext, "amp"); dataType == jsonparser.NotExist { - if val, err := jsonparser.Set(site.Ext, []byte(value), "amp"); err == nil { - site.Ext = val - } - } - } else { - site.Ext = json.RawMessage(`{"amp":` + value + `}`) - } -} - func setImpsImplicitly(httpReq *http.Request, imps []openrtb.Imp) { secure := int8(1) for i := 0; i < len(imps); i++ {