Fix prebid#3592. Bad Input Error if pbjs s2s config contains alias co…

…nfiguration for a disabled adapter
zxPhoenix · Dec 24, 2024 · ff61c81 · ff61c81
1 parent 19d25f3
commit ff61c81
Show file tree

Hide file tree

Showing 14 changed files with 240 additions and 116 deletions.
diff --git a/src/main/java/org/prebid/server/auction/requestfactory/AmpRequestFactory.java b/src/main/java/org/prebid/server/auction/requestfactory/AmpRequestFactory.java
@@ -415,6 +415,7 @@ private Future<BidRequest> updateBidRequest(AuctionContext auctionContext) {
                 .map(bidRequest -> paramsResolver.resolve(bidRequest, auctionContext, ENDPOINT, true))
                 .map(bidRequest -> ortb2RequestFactory.removeEmptyEids(bidRequest, auctionContext.getDebugWarnings()))
                 .compose(resolvedBidRequest -> ortb2RequestFactory.validateRequest(
+                        account,
                         resolvedBidRequest,
                         auctionContext.getHttpRequest(),
                         auctionContext.getDebugWarnings()));

diff --git a/src/main/java/org/prebid/server/auction/requestfactory/AuctionRequestFactory.java b/src/main/java/org/prebid/server/auction/requestfactory/AuctionRequestFactory.java
@@ -241,7 +241,8 @@ private Future<BidRequest> updateAndValidateBidRequest(AuctionContext auctionCon
 
         return storedRequestProcessor.processAuctionRequest(account.getId(), auctionContext.getBidRequest())
                 .compose(auctionStoredResult -> updateBidRequest(auctionStoredResult, auctionContext))
-                .compose(bidRequest -> ortb2RequestFactory.validateRequest(bidRequest, httpRequest, debugWarnings))
+                .compose(bidRequest -> ortb2RequestFactory.validateRequest(
+                        account, bidRequest, httpRequest, debugWarnings))
                 .map(interstitialProcessor::process);
     }
 

diff --git a/src/main/java/org/prebid/server/auction/requestfactory/Ortb2RequestFactory.java b/src/main/java/org/prebid/server/auction/requestfactory/Ortb2RequestFactory.java
@@ -192,11 +192,11 @@ public Future<ActivityInfrastructure> activityInfrastructureFrom(AuctionContext
                 auctionContext.getDebugContext().getTraceLevel()));
     }
 
-    public Future<BidRequest> validateRequest(BidRequest bidRequest,
+    public Future<BidRequest> validateRequest(Account account, BidRequest bidRequest,
                                               HttpRequestContext httpRequestContext,
                                               List<String> warnings) {
 
-        final ValidationResult validationResult = requestValidator.validate(bidRequest, httpRequestContext);
+        final ValidationResult validationResult = requestValidator.validate(account, bidRequest, httpRequestContext);
 
         if (validationResult.hasWarnings()) {
             warnings.addAll(validationResult.getWarnings());

diff --git a/src/main/java/org/prebid/server/auction/requestfactory/VideoRequestFactory.java b/src/main/java/org/prebid/server/auction/requestfactory/VideoRequestFactory.java
@@ -33,6 +33,7 @@
 import org.prebid.server.model.HttpRequestContext;
 import org.prebid.server.proto.openrtb.ext.request.ExtPublisher;
 import org.prebid.server.proto.openrtb.ext.request.ExtPublisherPrebid;
+import org.prebid.server.settings.model.Account;
 import org.prebid.server.util.HttpUtil;
 import org.prebid.server.util.ObjectUtil;
 
@@ -111,6 +112,7 @@ public Future<WithPodErrors<AuctionContext>> fromRequest(RoutingContext routingC
                 .compose(httpRequest -> createBidRequest(httpRequest)
                         .map(bidRequest -> removeEmptyEids(bidRequest, initialAuctionContext.getDebugWarnings()))
                         .compose(bidRequest -> validateRequest(
+                                initialAuctionContext.getAccount(),
                                 bidRequest,
                                 httpRequest,
                                 initialAuctionContext.getDebugWarnings()))
@@ -324,11 +326,13 @@ private WithPodErrors<BidRequest> fillImplicitParameters(HttpRequestContext http
         return WithPodErrors.of(updatedWithDebugBidRequest, bidRequestToErrors.getPodErrors());
     }
 
-    private Future<WithPodErrors<BidRequest>> validateRequest(WithPodErrors<BidRequest> requestWithPodErrors,
+    private Future<WithPodErrors<BidRequest>> validateRequest(Account account,
+                                                              WithPodErrors<BidRequest> requestWithPodErrors,
                                                               HttpRequestContext httpRequestContext,
                                                               List<String> warnings) {
 
-        return ortb2RequestFactory.validateRequest(requestWithPodErrors.getData(), httpRequestContext, warnings)
+        return ortb2RequestFactory.validateRequest(
+                account, requestWithPodErrors.getData(), httpRequestContext, warnings)
                 .map(bidRequest -> requestWithPodErrors);
     }
 }
diff --git a/src/main/java/org/prebid/server/metric/MetricName.java b/src/main/java/org/prebid/server/metric/MetricName.java
@@ -63,6 +63,8 @@ public enum MetricName {
     nobid,
     gotbids,
     badinput,
+    disabled_bidder,
+    unknown_bidder,
     blocklisted_account,
     blocklisted_app,
     badserverresponse,

diff --git a/src/main/java/org/prebid/server/metric/Metrics.java b/src/main/java/org/prebid/server/metric/Metrics.java
@@ -329,6 +329,22 @@ public void updateAdapterRequestErrorMetric(String bidder, MetricName errorMetri
         forAdapter(bidder).request().incCounter(errorMetric);
     }
 
+    public void updateAdapterRequestDisabledBidderMetric(String bidder, Account account) {
+        forAdapter(bidder).request().incCounter(MetricName.disabled_bidder);
+        if (accountMetricsVerbosityResolver.forAccount(account)
+                .isAtLeast(AccountMetricsVerbosityLevel.detailed)) {
+            forAccount(account.getId()).adapter().forAdapter(bidder).request().incCounter(MetricName.disabled_bidder);
+        }
+    }
+
+    public void updateAdapterRequestUnknownBidderMetric(String bidder, Account account) {
+        forAdapter(bidder).request().incCounter(MetricName.unknown_bidder);
+        if (accountMetricsVerbosityResolver.forAccount(account)
+                .isAtLeast(AccountMetricsVerbosityLevel.detailed)) {
+            forAccount(account.getId()).adapter().forAdapter(bidder).request().incCounter(MetricName.unknown_bidder);
+        }
+    }
+
     public void updateAnalyticEventMetric(String analyticCode, MetricName eventType, MetricName result) {
         forAnalyticReporter(analyticCode).forEventType(eventType).incCounter(result);
     }

diff --git a/src/main/java/org/prebid/server/validation/RequestValidator.java b/src/main/java/org/prebid/server/validation/RequestValidator.java
@@ -40,6 +40,7 @@
 import org.prebid.server.proto.openrtb.ext.request.ExtUserPrebid;
 import org.prebid.server.proto.openrtb.ext.request.ImpMediaType;
 import org.prebid.server.proto.openrtb.ext.response.BidType;
+import org.prebid.server.settings.model.Account;
 import org.prebid.server.util.HttpUtil;
 import org.prebid.server.validation.model.ValidationResult;
 
@@ -96,7 +97,7 @@ public RequestValidator(BidderCatalog bidderCatalog,
      * Validates the {@link BidRequest} against a list of validation checks, however, reports only one problem
      * at a time.
      */
-    public ValidationResult validate(BidRequest bidRequest, HttpRequestContext httpRequestContext) {
+    public ValidationResult validate(Account account, BidRequest bidRequest, HttpRequestContext httpRequestContext) {
         final List<String> warnings = new ArrayList<>();
         try {
             if (StringUtils.isBlank(bidRequest.getId())) {
@@ -121,7 +122,7 @@ public ValidationResult validate(BidRequest bidRequest, HttpRequestContext httpR
                     validateTargeting(targeting);
                 }
                 aliases = ObjectUtils.defaultIfNull(extRequestPrebid.getAliases(), Collections.emptyMap());
-                validateAliases(aliases);
+                validateAliases(aliases, warnings, metrics, account);
                 validateAliasesGvlIds(extRequestPrebid, aliases);
                 validateBidAdjustmentFactors(extRequestPrebid.getBidadjustmentfactors(), aliases);
                 validateExtBidPrebidData(extRequestPrebid.getData(), aliases);
@@ -491,18 +492,22 @@ private static void validateGranularityRangeIncrement(ExtGranularityRange range)
      * Validates aliases. Throws {@link ValidationException} in cases when alias points to invalid bidder or when alias
      * is equals to itself.
      */
-    private void validateAliases(Map<String, String> aliases) throws ValidationException {
+    private void validateAliases(Map<String, String> aliases, List<String> warnings,
+                                 Metrics metrics, Account account) throws ValidationException {
+
         for (final Map.Entry<String, String> aliasToBidder : aliases.entrySet()) {
             final String alias = aliasToBidder.getKey();
             final String coreBidder = aliasToBidder.getValue();
             if (!bidderCatalog.isValidName(coreBidder)) {
-                throw new ValidationException(
+                warnings.add(
                         "request.ext.prebid.aliases.%s refers to unknown bidder: %s".formatted(alias, coreBidder));
-            }
-            if (!bidderCatalog.isActive(coreBidder)) {
-                throw new ValidationException(
+                metrics.updateAdapterRequestUnknownBidderMetric(coreBidder, account);
+            } else if (!bidderCatalog.isActive(coreBidder)) {
+                warnings.add(
                         "request.ext.prebid.aliases.%s refers to disabled bidder: %s".formatted(alias, coreBidder));
+                metrics.updateAdapterRequestDisabledBidderMetric(coreBidder, account);
             }
+
             if (alias.equals(coreBidder)) {
                 throw new ValidationException("""
                         request.ext.prebid.aliases.%s defines a no-op alias. \

diff --git a/src/test/groovy/org/prebid/server/functional/tests/AliasSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/AliasSpec.groovy
@@ -10,6 +10,7 @@ import static org.prebid.server.functional.model.bidder.BidderName.ALIAS
 import static org.prebid.server.functional.model.bidder.BidderName.BOGUS
 import static org.prebid.server.functional.model.bidder.BidderName.GENERIC
 import static org.prebid.server.functional.model.bidder.CompressionType.GZIP
+import static org.prebid.server.functional.model.response.auction.ErrorType.PREBID
 import static org.prebid.server.functional.testcontainers.Dependencies.getNetworkServiceContainer
 import static org.prebid.server.functional.util.HttpUtil.CONTENT_ENCODING_HEADER
 
@@ -111,21 +112,28 @@ class AliasSpec extends BaseSpec {
         invalidId << [PBSUtils.randomNegativeNumber, 0]
     }
 
-    def "PBS should emit error when alias didn't participate in request"() {
+    def "PBS should emit warning when alias didn't participate in request"() {
         given: "Default bid request with alias"
         def randomString = PBSUtils.randomString
         def bidRequest = BidRequest.defaultBidRequest.tap {
             ext.prebid.aliases = [(randomString): BOGUS]
         }
 
         when: "PBS processes auction request"
-        defaultPbsService.sendAuctionRequest(bidRequest)
+        def response = defaultPbsService.sendAuctionRequest(bidRequest)
 
-        then: "Request should fail with an error"
-        def exception = thrown(PrebidServerException)
-        assert exception.statusCode == BAD_REQUEST.code()
-        assert exception.responseBody == "Invalid request format: request.ext.prebid.aliases.$randomString " +
-                "refers to unknown bidder: $BOGUS.value"
+        then: "Request shouldn't fail with an error"
+        def bidderRequests = bidder.getBidderRequests(bidRequest.id)
+        assert bidderRequests.size() == 1
+
+        and: "Bid response shouldn't contain errors"
+        assert !response.ext.errors
+
+        and: "Response should contain warnings"
+        def auctionWarnings = response.ext?.warnings?.get(PREBID)
+        assert auctionWarnings.size() == 1
+        assert auctionWarnings[0].code == 999
+        assert auctionWarnings[0].message == "request.ext.prebid.aliases.$randomString refers to unknown bidder: bogus"
     }
 
     def "PBS aliased bidder config should be independently from parent"() {

diff --git a/src/test/java/org/prebid/server/auction/requestfactory/AmpRequestFactoryTest.java b/src/test/java/org/prebid/server/auction/requestfactory/AmpRequestFactoryTest.java
@@ -1753,8 +1753,8 @@ private void givenBidRequest(
 
         given(ortb2ImplicitParametersResolver.resolve(any(), any(), any(), anyBoolean())).willAnswer(
                 answerWithFirstArgument());
-        given(ortb2RequestFactory.validateRequest(any(), any(), any()))
-                .willAnswer(invocation -> Future.succeededFuture((BidRequest) invocation.getArgument(0)));
+        given(ortb2RequestFactory.validateRequest(any(), any(), any(), any()))
+                .willAnswer(invocation -> Future.succeededFuture((BidRequest) invocation.getArgument(1)));
 
         given(ortb2RequestFactory.enrichBidRequestWithAccountAndPrivacyData(any()))
                 .willAnswer(invocation -> Future.succeededFuture(((AuctionContext) invocation.getArgument(0))

diff --git a/src/test/java/org/prebid/server/auction/requestfactory/AuctionRequestFactoryTest.java b/src/test/java/org/prebid/server/auction/requestfactory/AuctionRequestFactoryTest.java
@@ -165,8 +165,8 @@ public void setUp() {
         given(ortb2RequestFactory.executeRawAuctionRequestHooks(any()))
                 .willAnswer(invocation -> Future.succeededFuture(
                         ((AuctionContext) invocation.getArgument(0)).getBidRequest()));
-        given(ortb2RequestFactory.validateRequest(any(), any(), any()))
-                .willAnswer(invocationOnMock -> Future.succeededFuture((BidRequest) invocationOnMock.getArgument(0)));
+        given(ortb2RequestFactory.validateRequest(any(), any(), any(), any()))
+                .willAnswer(invocationOnMock -> Future.succeededFuture((BidRequest) invocationOnMock.getArgument(1)));
         given(ortb2RequestFactory.removeEmptyEids(any(), any()))
                 .willAnswer(invocationOnMock -> invocationOnMock.getArgument(0));
         given(ortb2RequestFactory.updateTimeout(any())).willAnswer(invocation -> invocation.getArgument(0));
@@ -662,7 +662,7 @@ public void shouldReturnFailedFutureIfRequestValidationFailed() {
         // given
         givenValidBidRequest();
 
-        given(ortb2RequestFactory.validateRequest(any(), any(), any()))
+        given(ortb2RequestFactory.validateRequest(any(), any(), any(), any()))
                 .willReturn(Future.failedFuture(new InvalidRequestException("errors")));
 
         // when

diff --git a/src/test/java/org/prebid/server/auction/requestfactory/Ortb2RequestFactoryTest.java b/src/test/java/org/prebid/server/auction/requestfactory/Ortb2RequestFactoryTest.java
@@ -104,6 +104,7 @@
 public class Ortb2RequestFactoryTest extends VertxTest {
 
     private static final List<String> BLOCKLISTED_ACCOUNTS = singletonList("bad_acc");
+    private static final String ACCOUNT_ID = "accountId";
 
     @Mock
     private UidsCookieService uidsCookieService;
@@ -658,12 +659,13 @@ public void enrichAuctionContextShouldSetDebugOff() {
     @Test
     public void validateRequestShouldThrowInvalidRequestExceptionIfRequestIsInvalid() {
         // given
-        given(requestValidator.validate(any(), any())).willReturn(ValidationResult.error("error"));
+        given(requestValidator.validate(any(), any(), any())).willReturn(ValidationResult.error("error"));
 
         final BidRequest bidRequest = givenBidRequest(identity());
 
         // when
         final Future<BidRequest> result = target.validateRequest(
+                Account.empty(ACCOUNT_ID),
                 bidRequest,
                 HttpRequestContext.builder().build(),
                 new ArrayList<>());
@@ -674,24 +676,25 @@ public void validateRequestShouldThrowInvalidRequestExceptionIfRequestIsInvalid(
                 .isInstanceOf(InvalidRequestException.class)
                 .hasMessage("error");
 
-        verify(requestValidator).validate(eq(bidRequest), any());
+        verify(requestValidator).validate(any(), eq(bidRequest), any());
     }
 
     @Test
     public void validateRequestShouldReturnSameBidRequest() {
         // given
-        given(requestValidator.validate(any(), any())).willReturn(ValidationResult.success());
+        given(requestValidator.validate(any(), any(), any())).willReturn(ValidationResult.success());
 
         final BidRequest bidRequest = givenBidRequest(identity());
 
         // when
         final BidRequest result = target.validateRequest(
+                Account.empty(ACCOUNT_ID),
                 bidRequest,
                 HttpRequestContext.builder().build(),
                 new ArrayList<>()).result();
 
         // then
-        verify(requestValidator).validate(eq(bidRequest), any());
+        verify(requestValidator).validate(any(), eq(bidRequest), any());
 
         assertThat(result).isSameAs(bidRequest);
     }

diff --git a/src/test/java/org/prebid/server/auction/requestfactory/VideoRequestFactoryTest.java b/src/test/java/org/prebid/server/auction/requestfactory/VideoRequestFactoryTest.java
@@ -345,7 +345,7 @@ public void shouldReturnExpectedResultAndReturnErrors() throws JsonProcessingExc
         verify(ortb2RequestFactory).createAuctionContext(any(), eq(MetricName.video));
         verify(ortb2RequestFactory).enrichAuctionContext(any(), any(), eq(bidRequest), eq(0L));
         verify(ortb2RequestFactory).fetchAccountWithoutStoredRequestLookup(any());
-        verify(ortb2RequestFactory).validateRequest(eq(bidRequest), any(), any());
+        verify(ortb2RequestFactory).validateRequest(any(), eq(bidRequest), any(), any());
         verify(paramsResolver)
                 .resolve(eq(bidRequest), any(), eq(Endpoint.openrtb2_video.value()), eq(false));
         verify(ortb2RequestFactory).enrichBidRequestWithAccountAndPrivacyData(
@@ -404,7 +404,7 @@ private void givenBidRequest(BidRequest bidRequest, List<PodError> podErrors) {
                         .build());
         given(ortb2RequestFactory.fetchAccountWithoutStoredRequestLookup(any())).willReturn(Future.succeededFuture());
 
-        given(ortb2RequestFactory.validateRequest(any(), any(), any()))
+        given(ortb2RequestFactory.validateRequest(any(), any(), any(), any()))
                 .willAnswer(invocation -> Future.succeededFuture((BidRequest) invocation.getArgument(0)));
 
         given(paramsResolver.resolve(any(), any(), any(), anyBoolean()))