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

Better native request validation #1132

Merged
merged 1 commit into from
Dec 4, 2019
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
25 changes: 9 additions & 16 deletions endpoints/openrtb2/auction.go
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,10 @@ func fillAndValidateNative(n *openrtb.Native, impIndex int) error {
}

func validateNativeContextTypes(cType native.ContextType, cSubtype native.ContextSubType, impIndex int) error {
if cType == 0 {
SyntaxNode marked this conversation as resolved.
Show resolved Hide resolved
// Context is only recommended, so none is a valid type.
return nil
guscarreon marked this conversation as resolved.
Show resolved Hide resolved
}
if cType < native.ContextTypeContent || cType > native.ContextTypeProduct {
return fmt.Errorf("request.imp[%d].native.request.context is invalid. See https://iabtechlab.com/wp-content/uploads/2016/07/OpenRTB-Native-Ads-Specification-Final-1.2.pdf#page=39", impIndex)
}
Expand Down Expand Up @@ -480,6 +484,10 @@ func validateNativeContextTypes(cType native.ContextType, cSubtype native.Contex
}

func validateNativePlacementType(pt native.PlacementType, impIndex int) error {
if pt == 0 {
SyntaxNode marked this conversation as resolved.
Show resolved Hide resolved
// Placement Type is only reccomended, not required.
return nil
}
if pt < native.PlacementTypeFeed || pt > native.PlacementTypeRecommendationWidget {
return fmt.Errorf("request.imp[%d].native.request.plcmttype is invalid. See https://iabtechlab.com/wp-content/uploads/2016/07/OpenRTB-Native-Ads-Specification-Final-1.2.pdf#page=40", impIndex)
}
Expand Down Expand Up @@ -523,9 +531,7 @@ func validateNativeAsset(asset nativeRequests.Asset, impIndex int, assetIndex in
return fmt.Errorf(assetErr, impIndex, assetIndex)
}
foundType = true
if err := validateNativeAssetImg(asset.Img, impIndex, assetIndex); err != nil {
return err
}
// It is technically valid to have neither w/h nor wmin/hmin, so no check
}

if asset.Video != nil {
Expand Down Expand Up @@ -587,19 +593,6 @@ func validateNativeEventTracker(tracker nativeRequests.EventTracker, impIndex in
return nil
}

func validateNativeAssetImg(image *nativeRequests.Image, impIndex int, assetIndex int) error {
// Note that w, wmin, h, and hmin cannot be negative because these variables use unsigned ints.
// Those fail during the standard json.Unmarshal() call.
if image.W == 0 && image.WMin == 0 {
return fmt.Errorf(`request.imp[%d].native.request.assets[%d].img must contain at least one of "w" or "wmin"`, impIndex, assetIndex)
}
if image.H == 0 && image.HMin == 0 {
return fmt.Errorf(`request.imp[%d].native.request.assets[%d].img must contain at least one of "h" or "hmin"`, impIndex, assetIndex)
}

return nil
}

func validateNativeAssetVideo(video *nativeRequests.Video, impIndex int, assetIndex int) error {
if len(video.MIMEs) < 1 {
return fmt.Errorf("request.imp[%d].native.request.assets[%d].video.mimes must be an array with at least one MIME type", impIndex, assetIndex)
Expand Down
7 changes: 3 additions & 4 deletions endpoints/openrtb2/auction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,6 @@ func TestRejectAccountRequired(t *testing.T) {
func (gr *getResponseFromDirectory) assert(t *testing.T) {
//t *testing.T, dir string, payloadGetter func(*testing.T, []byte) []byte, messageGetter func(*testing.T, []byte) []byte, expectedCode int, aliased bool) {
t.Helper()
var filename string
var filesToAssert []string
guscarreon marked this conversation as resolved.
Show resolved Hide resolved
if gr.file == "" {
// Append every file found in `gr.dir` to the `filesToAssert` array and test them all
Expand All @@ -300,14 +299,14 @@ func (gr *getResponseFromDirectory) assert(t *testing.T) {
fileData = readFile(t, testFile)
code, msg := gr.doRequest(t, gr.payloadGetter(t, fileData))
fmt.Printf("Processing %s\n", testFile)
assertResponseCode(t, filename, code, gr.expectedCode, msg)
assertResponseCode(t, testFile, code, gr.expectedCode, msg)

expectMsg := gr.messageGetter(t, fileData)
if gr.description != "" {
if len(expectMsg) > 0 {
assert.Equal(t, string(expectMsg), msg, "Test failed. %s. Filename: \n", gr.description, filename)
assert.Equal(t, string(expectMsg), msg, "Test failed. %s. Filename: \n", gr.description, testFile)
} else {
assert.Equal(t, string(expectMsg), msg, "file %s had bad response body", filename)
assert.Equal(t, string(expectMsg), msg, "file %s had bad response body", testFile)
}
}
}
Expand Down