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 Dec 23, 2024
1 parent 19d25f3 commit 22c0aa7
Show file tree
Hide file tree
Showing 13 changed files with 225 additions and 109 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.getDebugWarnings()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
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 @@ -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()))
Expand Down Expand Up @@ -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);
}
}
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 @@ -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);
}
Expand Down
19 changes: 12 additions & 7 deletions src/main/java/org/prebid/server/validation/RequestValidator.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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())) {
Expand All @@ -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);
Expand Down Expand Up @@ -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. \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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
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())).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<>());
Expand All @@ -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);
}
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());
verify(ortb2RequestFactory).validateRequest(any(), eq(bidRequest), 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()))
given(ortb2RequestFactory.validateRequest(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 22c0aa7

Please sign in to comment.