Repository navigation
pubmatic: return error for banner with no sizes and no format - #4643
Aditya-9-6 wants to merge 3 commits into
Conversation
Port of Go fix from prebid-server#5008 (prebid/prebid-server#5008). In Go, assignBannerSize() returns a BadInput error when banner.Format is empty and no explicit W/H are present, preventing a panic. The Java implementation had the same logical gap: assignSizesIfMissing() silently returned the banner unchanged when format was empty, forwarding a dimensionless banner to PubMatic. This commit: - Renames assignSizesIfMissing -> fillBannerSizeFromFormat to clarify its role (only copies dims from Format[0]; does not validate) - Adds a post-enrichment check in modifyImp() that throws PreBidException when the resulting banner still has no W/H after both fillBannerSizeFromFormat and enrichWithAdSlotParameters have run - This preserves the valid case where adSlot (e.g. slot@300x250) supplies dimensions even when format is empty Closes #5002 (see also prebid/prebid-server#5002)
Per OpenRTB 2.6 spec, banner sizes are optional for interstitial impressions (imp.instl=1), as interstitials fill the full screen. The previous fix incorrectly discarded these legitimate impressions. Skip the no-sizes PreBidException when the imp has instl=1, even if both W/H are absent and format is empty.
|
The new check only rejects when both w and h are missing. With w=300, h=null, no format, and an adSlot without size, the banner still passes through. Could the check reject either missing dimension? |
Update banner dimensions validation to check if either width or height is missing ((w == null || h == null)), preventing banners with only one dimension set from passing through.
|
Good catch! Updated the check to (resultBanner.getW() == null || resultBanner.getH() == null)\ so it rejects if either dimension is missing when no format/adSlot dimensions are available, and added a unit test for \w=300, h=null. |
| final Imp result = impBuilder.build(); | ||
| final Banner resultBanner = result.getBanner(); | ||
| // Per OpenRTB 2.6 spec, banner sizes are optional for interstitial impressions. | ||
| final boolean isInterstitial = Objects.equals(result.getInstl(), 1); | ||
| if (resultBanner != null && (resultBanner.getW() == null || resultBanner.getH() == null) && !isInterstitial) { | ||
| throw new PreBidException("No sizes provided for Banner"); | ||
| } | ||
|
|
||
| return result; |
There was a problem hiding this comment.
@Aditya-9-6 I doubt this check is needed in PBS-Java. It replicates ImpValidator.validateBanner, so imps with invalid values will be dropped before reaching makeHttpRequests. PBS-Go might have something similar in request_validator_banner.go
There was a problem hiding this comment.
Confirmed: ImpValidator.validateBanner rejects the w=300, h=null, format=[] case for non-interstitial impressions before the adapter runs. My earlier example tested the adapter directly, so I withdraw it as a reason to add this guard.
Summary
This is a Java port of the Go fix from prebid/prebid-server#5008, as requested in issue prebid/prebid-server#5002 (comment by @osulzhenko).
Problem
PubmaticBidder.assignSizesIfMissing()silently returned the banner unchanged whenformatwas empty and no explicitW/Hwere set, forwarding a dimensionless banner to PubMatic — leading to invalid requests.Go's equivalent (
assignBannerSize) was already fixed to return aBadInputerror in such cases. Java was missing the same guard.What is different in Java vs Go
The call order in Java differs from Go:
fillBannerSizeFromFormat()(formerlyassignSizesIfMissing) is called beforeenrichWithAdSlotParameters()adSlotcontains@WxH(e.g.slot@300x250),enrichWithAdSlotParameterssetsW/Hon the imp builder after the format-fill stepSo we cannot throw eagerly in
fillBannerSizeFromFormat— a valid imp with emptyformatbutadSlot-derived dimensions would be incorrectly rejected.Fix
assignSizesIfMissing→fillBannerSizeFromFormatto clarify its single responsibility (copying dims fromformat[0]when W/H are absent)modifyImp(): if the resulting banner still has noW/Hafter both steps, throwPreBidException("No sizes provided for Banner")Tests added
makeHttpRequestsShouldReturnErrorIfBannerHasNoSizesAndNoFormat— banner with empty format and adSlot without@→ BadInput errormakeHttpRequestsShouldAllowBannerWithNoFormatWhenAdSlotProvidesSize— banner with empty format butadSlot = slot@300x250→ W=300, H=250 from adSlot, no errorFixes #5002 (cross-repo issue from prebid/prebid-server#5002)