ORTB Blocking Module: battr media type strictness

[postindustria-code]

Fix ORTB Blocking Module: Prevent Creation of Empty Media Type Objects

Issues: #4006, #3913

Problem Description

The ORTB blocking module was incorrectly adding battr (blocked attributes) to media type objects (imp[].banner/video/audio.battr) even when the media type object didn't exist in the original request.

The module would create empty banner, video, or audio objects solely to inject battr values, resulting in "bogus" media types in impressions that could confuse SSPs and lead to invalid bid requests.

Example of Problematic Behavior

Before Fix:

{
  "imp": [
    {
      "id": "DLP_Desk_Top",
      "banner": {
        "format": [{"w": 728, "h": 90}],
        "battr": [1, 2, 3]
      }
    },
    {
      "id": "imp-video-1",
      "banner": {
        "battr": [1, 2, 3]  // Empty banner object created unnecessarily
      },
      "video": {
        "mimes": ["video/mp4"],
        "minduration": 5,
        "maxduration": 30,
        "protocols": [2, 3],
        "w": 640,
        "h": 480
      }
    }
  ]
}

Root Cause Analysis

1. Unconditional Object Creation: The mutationForImp function was creating empty media type objects:

   if imp.Banner == nil {
       imp.Banner = &openrtb2.Banner{}  // Creates empty object
   }

2. Incorrect Existence Check: The attribute existence check was validating populated BAttr arrays instead of checking if the media type object existed:

   bannerCheckAttrExistence := func(imp openrtb2.Imp) bool {
       return imp.Banner != nil && len(imp.Banner.BAttr) > 0  // Wrong logic
   }

Solution Implementation

1. Enhanced Configuration Support

Added comprehensive video and audio support to the configuration structure:

type Battr struct {
    ActionOverrides             BattrActionOverride `json:"action_overrides"`
    AllowedBannerAttrForDeals   []int               `json:"allowed_banner_attr_for_deals"`
    AllowedVideoAttrForDeals    []int               `json:"allowed_video_attr_for_deals"`
    AllowedAudioAttrForDeals    []int               `json:"allowed_audio_attr_for_deals"`
    BlockedBannerAttr           []int               `json:"blocked_banner_attr"`
    BlockedVideoAttr            []int               `json:"blocked_video_attr"`
    BlockedAudioAttr            []int               `json:"blocked_audio_attr"`
    EnforceBlocks               bool                `json:"enforce_blocks"`
}

2. Media Type Validation

Implemented proper media type existence validation before applying battr:

// Apply video battr only to impressions that have Video objects
if len(videoOverrides) > 0 {
    filteredVideoOverrides := filterByMediaType(payload, videoOverrides, func(imp openrtb2.Imp) bool {
        return imp.Video != nil  // Check object existence first
    })
    if len(filteredVideoOverrides) > 0 {
        mutation := createBAttrMutation(filteredVideoOverrides, "video")
        changeSet.AddMutation(mutation, hookstage.MutationUpdate, "bidrequest", "imp", "video", "battr")
    }
}

3. Safe Media Type Filtering

Added utility function to ensure battr is only applied to existing media type objects:

func filterByMediaType(
    payload hookstage.BidderRequestPayload,
    overrides map[string][]int,
    mediaTypeExists func(imp openrtb2.Imp) bool,
) map[string][]int {
    filtered := make(map[string][]int)
    
    for _, imp := range payload.Request.Imp {
        if values, exists := overrides[imp.ID]; exists && mediaTypeExists(imp) {
            filtered[imp.ID] = values  // Only include if media type exists
        }
    }
    
    return filtered
}

4. Type-Safe Mutation Creation

Implemented secure mutation functions that verify object existence before modification:

func createBAttrMutation(bAttrByImp map[string][]int, mediaType string) hookstage.MutationFunc[hookstage.BidderRequestPayload] {
    return func(payload hookstage.BidderRequestPayload) (hookstage.BidderRequestPayload, error) {
        for i, imp := range payload.Request.Imp {
            if values, ok := bAttrByImp[imp.ID]; ok && len(values) > 0 {
                switch mediaType {
                case "banner":
                    if imp.Banner != nil {  // Verify existence before modification
                        imp.Banner.BAttr = make([]adcom1.CreativeAttribute, len(values))
                        for j, attr := range values {
                            imp.Banner.BAttr[j] = adcom1.CreativeAttribute(attr)
                        }
                    }
                case "video":
                    if imp.Video != nil {  // Verify existence before modification
                        imp.Video.BAttr = make([]adcom1.CreativeAttribute, len(values))
                        for j, attr := range values {
                            imp.Video.BAttr[j] = adcom1.CreativeAttribute(attr)
                        }
                    }
                case "audio":
                    if imp.Audio != nil {  // Verify existence before modification
                        imp.Audio.BAttr = make([]adcom1.CreativeAttribute, len(values))
                        for j, attr := range values {
                            imp.Audio.BAttr[j] = adcom1.CreativeAttribute(attr)
                        }
                    }
                }
                payload.Request.Imp[i] = imp
            }
        }
        return payload, nil
    }
}

[pm-nikhil-vaidya]

We don't need to check again for banner object, as filterByMediaType already filters imps without banner.

[postindustria-code]

You're absolutely right.
I removed the redundant check.

[pm-nikhil-vaidya]

We can remove mediatype specific checks from here, as its already taken care by filterByMediaType or we can remove filterByMediaType function and keep these checks

[postindustria-code]

I removed the redundant check.

[pm-nikhil-vaidya]

Same as comment on line 509

[postindustria-code]

I removed the redundant check.

[pm-nikhil-vaidya]

Same as comment on line 509

[postindustria-code]

I removed the redundant check.