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.

Sign up for GitHub

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

Silvermob: host validation (us, eu, apac) #3110

Merged
merged 2 commits into from
Sep 26, 2023
Merged

Silvermob: host validation (us, eu, apac) #3110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a52cec6
host validation (us, eu, apac)
freemmy Sep 15, 2023
0ffb7cc
Code optimizations
freemmy Sep 20, 2023
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
14 changes: 12 additions & 2 deletions adapters/silvermob/silvermob.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ type SilverMobAdapter struct {
endpoint *template.Template
}

func isValidHost(host string) bool {
return host == "eu" || host == "us" || host == "apac"
}

// Builder builds a new instance of the SilverMob adapter for the given bidder with the given config.
func Builder(bidderName openrtb_ext.BidderName, config config.Adapter, server config.Server) (adapters.Bidder, error) {
template, err := template.New("endpointTemplate").Parse(config.Endpoint)
Expand Down Expand Up @@ -121,8 +125,14 @@ func (a *SilverMobAdapter) getImpressionExt(imp *openrtb2.Imp) (*openrtb_ext.Ext
}

func (a *SilverMobAdapter) buildEndpointURL(params *openrtb_ext.ExtSilverMob) (string, error) {
endpointParams := macros.EndpointTemplateParams{ZoneID: params.ZoneID, Host: params.Host}
return macros.ResolveMacros(a.endpoint, endpointParams)
if isValidHost(params.Host) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please be mindful that your adapter has been flagged as it is currently using dynamic hosts. Please refer to the github issue here - #2612

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's why we added this validation. Is it enough?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should strive to render the host non-dynamic by excluding the host parameter from the domain and use it as a query parameter instead. The domain name should remain constant.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, we will select a vendor for geo DNS and publish update within a couple of weeks

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@freemmy Please advise if you would like to make changes for geo DNS in this PR or a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thing the next PR since it takes some time

endpointParams := macros.EndpointTemplateParams{ZoneID: params.ZoneID, Host: params.Host}
return macros.ResolveMacros(a.endpoint, endpointParams)
} else {
return "", &errortypes.BadInput{
Message: fmt.Sprintf("invalid host %s", params.Host),
}
}
}

func (a *SilverMobAdapter) MakeBids(
Expand Down
53 changes: 53 additions & 0 deletions adapters/silvermob/silvermobtest/supplemental/invalid-host.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
{
"mockBidRequest": {
"id": "some-request-id",
"device": {
"ua": "test-user-agent",
"ip": "123.123.123.123",
"language": "en",
"dnt": 0
},
"tmax": 1000,
"user": {
"buyeruid": "awesome-user"
},
"app": {
"publisher": {
"id": "123456789"
},
"cat": [
"IAB22-1"
],
"bundle": "com.app.awesome",
"name": "Awesome App",
"domain": "awesomeapp.com",
"id": "123456789"
},
"imp": [
{
"id": "some-impression-id",
"tagid": "ogTAGID",
"banner": {
"w":320,
"h":50
},
"ext": {
"bidder": {
"host": "some_host",
"zoneid": "0"
}
}
}
]
},
"httpCalls": [
],
"expectedBidResponses": [
],
"expectedMakeRequestsErrors": [
{
"value": "invalid host some_host",
"comparison": "literal"
}
]
}
Loading