Skip to content

Commit

Permalink
Fix prebid#3592. Bad Input Error if pbjs s2s config contains alias co…
Browse files Browse the repository at this point in the history
…nfiguration for a disabled adapter
  • Loading branch information
zxPhoenix committed Feb 4, 2025
1 parent f978b0e commit d9db6d0
Show file tree
Hide file tree
Showing 13 changed files with 218 additions and 104 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.getDebugContext(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ private Future<BidRequest> updateAndValidateBidRequest(AuctionContext auctionCon
return storedRequestProcessor.processAuctionRequest(account.getId(), auctionContext.getBidRequest())
.compose(auctionStoredResult -> updateBidRequest(auctionStoredResult, auctionContext))
.compose(bidRequest -> ortb2RequestFactory.validateRequest(
bidRequest, httpRequest, auctionContext.getDebugContext(), debugWarnings))
account, bidRequest, httpRequest, auctionContext.getDebugContext(), debugWarnings))
.map(interstitialProcessor::process);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,13 +192,13 @@ public Future<ActivityInfrastructure> activityInfrastructureFrom(AuctionContext
auctionContext.getDebugContext().getTraceLevel()));
}

public Future<BidRequest> validateRequest(BidRequest bidRequest,
public Future<BidRequest> validateRequest(Account account, BidRequest bidRequest,
HttpRequestContext httpRequestContext,
DebugContext debugContext,
List<String> warnings) {

final ValidationResult validationResult = requestValidator.validate(
bidRequest, httpRequestContext, debugContext);
account, bidRequest, httpRequestContext, debugContext);

if (validationResult.hasWarnings()) {
warnings.addAll(validationResult.getWarnings());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -120,6 +121,7 @@ public Future<WithPodErrors<AuctionContext>> fromRequest(RoutingContext routingC
.map(auctionContext -> auctionContext.with(debugResolver.debugContextFrom(auctionContext)))

.compose(auctionContext -> ortb2RequestFactory.validateRequest(
auctionContext.getAccount(),
auctionContext.getBidRequest(),
auctionContext.getHttpRequest(),
auctionContext.getDebugContext(),
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/org/prebid/server/metric/MetricName.java
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ public enum MetricName {
nobid,
gotbids,
badinput,
disabled_bidder,
unknown_bidder,
blocklisted_account,
blocklisted_app,
badserverresponse,
Expand Down
16 changes: 16 additions & 0 deletions src/main/java/org/prebid/server/metric/Metrics.java
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,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);
}
Expand Down
20 changes: 13 additions & 7 deletions src/main/java/org/prebid/server/validation/RequestValidator.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,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;

Expand Down Expand Up @@ -97,7 +98,8 @@ 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,
public ValidationResult validate(Account account,
BidRequest bidRequest,
HttpRequestContext httpRequestContext,
DebugContext debugContext) {

Expand Down Expand Up @@ -126,7 +128,7 @@ public ValidationResult validate(BidRequest bidRequest,
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, isDebugEnabled, warnings);
Expand Down Expand Up @@ -505,18 +507,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. \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1753,7 +1753,7 @@ private void givenBidRequest(

given(ortb2ImplicitParametersResolver.resolve(any(), any(), any(), anyBoolean())).willAnswer(
answerWithFirstArgument());
given(ortb2RequestFactory.validateRequest(any(), any(), any(), any()))
given(ortb2RequestFactory.validateRequest(any(), any(), any(), any(), any()))
.willAnswer(invocation -> Future.succeededFuture((BidRequest) invocation.getArgument(0)));

given(ortb2RequestFactory.enrichBidRequestWithAccountAndPrivacyData(any()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ public void setUp() {
given(ortb2RequestFactory.executeRawAuctionRequestHooks(any()))
.willAnswer(invocation -> Future.succeededFuture(
((AuctionContext) invocation.getArgument(0)).getBidRequest()));
given(ortb2RequestFactory.validateRequest(any(), any(), any(), any()))
given(ortb2RequestFactory.validateRequest(any(), any(), any(), any(), any()))
.willAnswer(invocationOnMock -> Future.succeededFuture((BidRequest) invocationOnMock.getArgument(0)));
given(ortb2RequestFactory.removeEmptyEids(any(), any()))
.willAnswer(invocationOnMock -> invocationOnMock.getArgument(0));
Expand Down Expand Up @@ -662,7 +662,7 @@ public void shouldReturnFailedFutureIfRequestValidationFailed() {
// given
givenValidBidRequest();

given(ortb2RequestFactory.validateRequest(any(), any(), any(), any()))
given(ortb2RequestFactory.validateRequest(any(), any(), any()))
.willReturn(Future.failedFuture(new InvalidRequestException("errors")));

// when
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -658,12 +659,13 @@ public void enrichAuctionContextShouldSetDebugOff() {
@Test
public void validateRequestShouldThrowInvalidRequestExceptionIfRequestIsInvalid() {
// given
given(requestValidator.validate(any(), any(), any())).willReturn(ValidationResult.error("error"));
given(requestValidator.validate(any(), 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(),
DebugContext.empty(),
Expand All @@ -675,25 +677,26 @@ public void validateRequestShouldThrowInvalidRequestExceptionIfRequestIsInvalid(
.isInstanceOf(InvalidRequestException.class)
.hasMessage("error");

verify(requestValidator).validate(eq(bidRequest), any(), any());
verify(requestValidator).validate(any(), eq(bidRequest), any(), any());
}

@Test
public void validateRequestShouldReturnSameBidRequest() {
// given
given(requestValidator.validate(any(), any(), any())).willReturn(ValidationResult.success());
given(requestValidator.validate(any(), 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(),
DebugContext.empty(),
new ArrayList<>()).result();

// then
verify(requestValidator).validate(eq(bidRequest), any(), any());
verify(requestValidator).validate(any(), eq(bidRequest), any(), any());

assertThat(result).isSameAs(bidRequest);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(), any());
verify(ortb2RequestFactory).validateRequest(any(), eq(bidRequest), any(), any(), any());
verify(paramsResolver)
.resolve(eq(bidRequest), any(), eq(Endpoint.openrtb2_video.value()), eq(false));
verify(ortb2RequestFactory).enrichBidRequestWithAccountAndPrivacyData(
Expand Down Expand Up @@ -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(), any()))
given(ortb2RequestFactory.validateRequest(any(), any(), any(), any(), any()))
.willAnswer(invocation -> Future.succeededFuture((BidRequest) invocation.getArgument(0)));

given(paramsResolver.resolve(any(), any(), any(), anyBoolean()))
Expand Down
Loading

0 comments on commit d9db6d0

Please sign in to comment.