Add Support For SkAdN + Refactor Split Imps (prebid#1741)

endpoints/openrtb2/auction.go

@@ -830,19 +830,15 @@ func (deps *endpointDeps) validateImpExt(imp *openrtb.Imp, aliases map[string]st
 		return []error{err}
 	}
 
-	// Also accept bidder exts within imp[...].ext.prebid.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 {
-		var prebidExt openrtb_ext.ExtImpPrebid
-		if err := json.Unmarshal(rawPrebidExt, &prebidExt); err == nil && prebidExt.Bidder != nil {
-			for bidder, ext := range prebidExt.Bidder {
+	// Prefer bidder params from request.imp.ext.prebid.bidder.BIDDER over request.imp.ext.BIDDER
+	// to avoid confusion beteween prebid specific adapter config and other ext protocols.
+	if extPrebidJSON, ok := bidderExts[openrtb_ext.PrebidExtKey]; ok {
+		var extPrebid openrtb_ext.ExtImpPrebid
+		if err := json.Unmarshal(extPrebidJSON, &extPrebid); err == nil && extPrebid.Bidder != nil {
+			for bidder, ext := range extPrebid.Bidder {
 				if ext == nil {
 					continue
 				}
-
 				bidderExts[bidder] = ext
 			}
 		}
@@ -893,13 +889,18 @@ func (deps *endpointDeps) validateImpExt(imp *openrtb.Imp, aliases map[string]st
 	return errL
 }
 
+// isBidderToValidate determines if the bidder name in request.imp[].prebid should be validated.
 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
+	switch openrtb_ext.BidderName(bidder) {
+	case openrtb_ext.BidderReservedContext:
+		return false
+	case openrtb_ext.BidderReservedPrebid:
+		return false
+	case openrtb_ext.BidderReservedSKAdN:
+		return false
+	default:
+		return true
+	}
 }
 
 func (deps *endpointDeps) parseBidExt(ext json.RawMessage) (*openrtb_ext.ExtRequest, error) {

endpoints/openrtb2/sample-requests/valid-whole/exemplary/skadn.json

@@ -0,0 +1,48 @@
+{
+    "description": "The imp.ext.skadn field is valid for Apple's SKAdNetwork and should be exempt from bidder name validation.",
+
+    "mockBidRequest": {
+        "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
+                        }
+                    }
+                },
+                "skadn": {
+                    "anyMember": "anyValue"
+                }
+            }
+        }]
+    },
+    "expectedBidResponse": {
+        "id": "some-request-id",
+        "seatbid": [{
+            "bid": [{
+                "id": "appnexus-bid",
+                "impid": "",
+                "price": 0
+            }],
+            "seat": "appnexus-bids"
+        }],
+        "bidid": "test bid id",
+        "nbr": 0
+    },
+    "expectedReturnCode": 200
+}

exchange/exchangetest/firstpartydata-imp-ext-multiple-prebid-bidders.json → exchange/exchangetest/firstpartydata-imp-ext-multiple-prebid-bidders.json

@@ -61,7 +61,6 @@
                             "bidder": {
                                 "placementId": 1
                             },
-                            "prebid": {},
                             "context": {
                                 "data": {
                                     "keywords": "prebid server example"
@@ -112,7 +111,6 @@
                                 "siteId": 2,
                                 "zoneId": 3
                             },
-                            "prebid": {},
                             "context": {
                                 "data": {
                                     "keywords": "prebid server example"

exchange/exchangetest/firstpartydata-imp-ext-one-prebid-bidder.json

@@ -56,7 +56,6 @@
                             "bidder": {
                                 "placementId": 1
                             },
-                            "prebid": {},
                             "context": {
                                 "data": {
                                     "keywords": "prebid server example"

exchange/utils.go

@@ -59,8 +59,9 @@ func cleanOpenRTBRequests(ctx context.Context,
 	usersyncIfAmbiguous bool,
 	privacyConfig config.Privacy) (bidderRequests []BidderRequest, privacyLabels metrics.PrivacyLabels, errs []error) {
 
-	impsByBidder, errs := splitImps(req.BidRequest.Imp)
-	if len(errs) > 0 {
+	impsByBidder, err := splitImps(req.BidRequest.Imp)
+	if err != nil {
+		errs = []error{err}
 		return
 	}
 
@@ -327,106 +328,99 @@ func extractBuyerUIDs(user *openrtb.User) (map[string]string, error) {
 // The "imp.ext" value of the rubicon Imp will only contain the "prebid" values, and "rubicon" value at the "bidder" key.
 //
 // The goal here is so that Bidders only get Imps and Imp.Ext values which are intended for them.
-func splitImps(imps []openrtb.Imp) (map[string][]openrtb.Imp, []error) {
-	impExts, err := parseImpExts(imps)
-	if err != nil {
-		return nil, []error{err}
-	}
+func splitImps(imps []openrtb.Imp) (map[string][]openrtb.Imp, error) {
+	bidderImps := make(map[string][]openrtb.Imp)
 
-	splitImps := make(map[string][]openrtb.Imp, len(imps))
-	var errList []error
+	for i, imp := range imps {
+		var impExt map[string]json.RawMessage
+		if err := json.Unmarshal(imp.Ext, &impExt); err != nil {
+			return nil, fmt.Errorf("invalid json for imp[%d]: %v", i, err)
+		}
 
-	for i := 0; i < len(imps); i++ {
-		imp := imps[i]
-		impExt := impExts[i]
+		var impExtPrebid map[string]json.RawMessage
+		if impExtPrebidJSON, exists := impExt[openrtb_ext.PrebidExtKey]; exists {
+			// validation already performed by impExt unmarshal. no error is possible here, proven by tests.
+			json.Unmarshal(impExtPrebidJSON, &impExtPrebid)
+		}
 
-		var firstPartyDataContext json.RawMessage
-		if context, exists := impExt[openrtb_ext.FirstPartyDataContextExtKey]; exists {
-			firstPartyDataContext = context
+		var impExtPrebidBidder map[string]json.RawMessage
+		if impExtPrebidBidderJSON, exists := impExtPrebid[openrtb_ext.PrebidExtBidderKey]; exists {
+			// validation already performed by impExt unmarshal. no error is possible here, proven by tests.
+			json.Unmarshal(impExtPrebidBidderJSON, &impExtPrebidBidder)
 		}
 
-		rawPrebidExt, ok := impExt[openrtb_ext.PrebidExtKey]
+		sanitizedImpExt, err := createSanitizedImpExt(impExt, impExtPrebid)
+		if err != nil {
+			return nil, fmt.Errorf("unable to remove other bidder fields for imp[%d]: %v", i, err)
+		}
 
-		if ok {
-			var prebidExt openrtb_ext.ExtImpPrebid
+		for bidder, bidderExt := range extractBidderExts(impExt, impExtPrebidBidder) {
+			impCopy := imp
 
-			if err := json.Unmarshal(rawPrebidExt, &prebidExt); err == nil && prebidExt.Bidder != nil {
-				if errs := sanitizedImpCopy(&imp, prebidExt.Bidder, rawPrebidExt, firstPartyDataContext, &splitImps); errs != nil {
-					errList = append(errList, errs...)
-				}
+			sanitizedImpExt[openrtb_ext.PrebidExtBidderKey] = bidderExt
 
-				continue
+			impExtJSON, err := json.Marshal(sanitizedImpExt)
+			if err != nil {
+				return nil, fmt.Errorf("unable to remove other bidder fields for imp[%d]: cannot marshal ext: %v", i, err)
 			}
-		}
+			impCopy.Ext = impExtJSON
 
-		if errs := sanitizedImpCopy(&imp, impExt, rawPrebidExt, firstPartyDataContext, &splitImps); errs != nil {
-			errList = append(errList, errs...)
+			bidderImps[bidder] = append(bidderImps[bidder], impCopy)
 		}
 	}
 
-	return splitImps, nil
+	return bidderImps, nil
 }
 
-// sanitizedImpCopy returns a copy of imp with its ext filtered so that only "prebid", "context", and bidder params exist.
-// It will not mutate the input imp.
-// This function will write the new imps to the output map passed in
-func sanitizedImpCopy(imp *openrtb.Imp,
-	bidderExts map[string]json.RawMessage,
-	rawPrebidExt json.RawMessage,
-	firstPartyDataContext json.RawMessage,
-	out *map[string][]openrtb.Imp) []error {
-
-	var prebidExt map[string]json.RawMessage
-	var errs []error
-
-	if err := json.Unmarshal(rawPrebidExt, &prebidExt); err == nil {
-		// Remove the entire bidder field. We will already have the content we need in bidderExts. We
-		// don't want to include other demand partners' bidder params in the sanitized imp.
-		if _, hasBidderField := prebidExt["bidder"]; hasBidderField {
-			delete(prebidExt, "bidder")
+func createSanitizedImpExt(impExt, impExtPrebid map[string]json.RawMessage) (map[string]json.RawMessage, error) {
+	sanitizedImpExt := make(map[string]json.RawMessage, 3)
 
-			var err error
-			if rawPrebidExt, err = json.Marshal(prebidExt); err != nil {
-				errs = append(errs, err)
-			}
+	delete(impExtPrebid, openrtb_ext.PrebidExtBidderKey)
+	if len(impExtPrebid) > 0 {
+		if impExtPrebidJSON, err := json.Marshal(impExtPrebid); err == nil {
+			sanitizedImpExt[openrtb_ext.PrebidExtKey] = impExtPrebidJSON
+		} else {
+			return nil, fmt.Errorf("cannot marshal ext.prebid: %v", err)
 		}
 	}
 
-	for bidder, ext := range bidderExts {
-		if bidder == openrtb_ext.PrebidExtKey || bidder == openrtb_ext.FirstPartyDataContextExtKey {
-			continue
-		}
+	if v, exists := impExt[openrtb_ext.FirstPartyDataContextExtKey]; exists {
+		sanitizedImpExt[openrtb_ext.FirstPartyDataContextExtKey] = v
+	}
 
-		impCopy := *imp
-		newExt := make(map[string]json.RawMessage, 3)
+	if v, exists := impExt[openrtb_ext.SKAdNExtKey]; exists {
+		sanitizedImpExt[openrtb_ext.SKAdNExtKey] = v
+	}
 
-		newExt["bidder"] = ext
+	return sanitizedImpExt, nil
+}
 
-		if rawPrebidExt != nil {
-			newExt[openrtb_ext.PrebidExtKey] = rawPrebidExt
-		}
+func extractBidderExts(impExt, impExtPrebidBidders map[string]json.RawMessage) map[string]json.RawMessage {
+	bidderExts := make(map[string]json.RawMessage)
 
-		if len(firstPartyDataContext) > 0 {
-			newExt[openrtb_ext.FirstPartyDataContextExtKey] = firstPartyDataContext
-		}
+	// prefer imp.ext.prebid.bidder.BIDDER
+	for bidder, bidderExt := range impExtPrebidBidders {
+		bidderExts[bidder] = bidderExt
+	}
 
-		rawExt, err := json.Marshal(newExt)
-		if err != nil {
-			errs = append(errs, err)
+	// fallback to imp.BIDDER
+	for bidder, bidderExt := range impExt {
+		if isSpecialField(bidder) {
 			continue
 		}
 
-		impCopy.Ext = rawExt
-
-		otherImps, _ := (*out)[bidder]
-		(*out)[bidder] = append(otherImps, impCopy)
+		if _, exists := bidderExts[bidder]; !exists {
+			bidderExts[bidder] = bidderExt
+		}
 	}
 
-	if len(errs) > 0 {
-		return errs
-	}
+	return bidderExts
+}
 
-	return nil
+func isSpecialField(bidder string) bool {
+	return bidder == openrtb_ext.FirstPartyDataContextExtKey ||
+		bidder == openrtb_ext.SKAdNExtKey ||
+		bidder == openrtb_ext.PrebidExtKey
 }
 
 // prepareUser changes req.User so that it's ready for the given bidder.
@@ -564,21 +558,6 @@ func resolveBidder(bidder string, aliases map[string]string) openrtb_ext.BidderN
 	return openrtb_ext.BidderName(bidder)
 }
 
-// parseImpExts does a partial-unmarshal of the imp[].Ext field.
-// The keys in the returned map are expected to be "prebid", "context", CoreBidderNames, or Aliases for this request.
-func parseImpExts(imps []openrtb.Imp) ([]map[string]json.RawMessage, error) {
-	exts := make([]map[string]json.RawMessage, len(imps))
-	// Loop over every impression in the request
-	for i := 0; i < len(imps); i++ {
-		// Unpack each set of extensions found in the Imp array
-		err := json.Unmarshal(imps[i].Ext, &exts[i])
-		if err != nil {
-			return nil, fmt.Errorf("Error unpacking extensions for Imp[%d]: %s", i, err.Error())
-		}
-	}
-	return exts, nil
-}
-
 // parseAliases parses the aliases from the BidRequest
 func parseAliases(orig *openrtb.BidRequest) (map[string]string, []error) {
 	var aliases map[string]string