From c5d367490a3b0593bc075c364009f388f3c4e87b Mon Sep 17 00:00:00 2001 From: Oleksandr Zhevedenko <720803+Net-burst@users.noreply.github.com> Date: Fri, 28 Mar 2025 14:15:46 -0400 Subject: [PATCH 1/2] Validate actual presence of uuid query param value --- .../prebid/cache/handlers/ErrorHandler.java | 2 +- .../cache/handlers/cache/GetCacheHandler.java | 18 +-- .../cache/handlers/CacheHandlerTests.java | 2 +- .../cache/handlers/GetCacheHandlerTests.java | 103 ++++++++++++------ .../cache/handlers/PostCacheHandlerTests.java | 14 +-- .../handlers/PostStorageHandlerTests.java | 2 +- .../cache/metrics/MetricsRecorderTest.java | 6 +- 7 files changed, 92 insertions(+), 55 deletions(-) diff --git a/src/main/java/org/prebid/cache/handlers/ErrorHandler.java b/src/main/java/org/prebid/cache/handlers/ErrorHandler.java index 429c3b5..4652719 100644 --- a/src/main/java/org/prebid/cache/handlers/ErrorHandler.java +++ b/src/main/java/org/prebid/cache/handlers/ErrorHandler.java @@ -16,7 +16,7 @@ public class ErrorHandler extends MetricsHandler { private static final String RESOURCE_NOT_FOUND_BAD_URL = "Resource Not Found - Bad URL."; private static final String RESOURCE_NOT_FOUND = "Resource Not Found: uuid %s"; - private static final String INVALID_PARAMETERS = "Invalid Parameter(s): uuid not found."; + private static final String INVALID_PARAMETERS = "Invalid Parameter(s): uuid not found or is empty."; private static final String NO_ELEMENTS_FOUND = "No Elements Found."; @Autowired diff --git a/src/main/java/org/prebid/cache/handlers/cache/GetCacheHandler.java b/src/main/java/org/prebid/cache/handlers/cache/GetCacheHandler.java index 37371b7..f49aa6d 100644 --- a/src/main/java/org/prebid/cache/handlers/cache/GetCacheHandler.java +++ b/src/main/java/org/prebid/cache/handlers/cache/GetCacheHandler.java @@ -77,14 +77,16 @@ private static Map createClientsCache(final int ttl, final in } public Mono fetch(ServerRequest request) { - // metrics - metricsRecorder.markMeterForTag(this.metricTagPrefix, MeasurementTag.REQUEST); - final var timerContext = metricsRecorder.createRequestTimerForServiceType(this.type); - - return request.queryParam(ID_KEY).map(id -> fetch(request, id, timerContext)).orElseGet(() -> { - final var responseMono = ErrorHandler.createInvalidParameters(); - return finalizeResult(responseMono, request, timerContext); - }); + metricsRecorder.markMeterForTag(metricTagPrefix, MeasurementTag.REQUEST); + final var timerContext = metricsRecorder.createRequestTimerForServiceType(type); + + return request.queryParam(ID_KEY) + .filter(StringUtils::isNotBlank) + .map(id -> fetch(request, id, timerContext)) + .orElseGet(() -> { + final var responseMono = ErrorHandler.createInvalidParameters(); + return finalizeResult(responseMono, request, timerContext); + }); } private Mono fetch(final ServerRequest request, diff --git a/src/test/java/org/prebid/cache/handlers/CacheHandlerTests.java b/src/test/java/org/prebid/cache/handlers/CacheHandlerTests.java index dd94868..bddafeb 100644 --- a/src/test/java/org/prebid/cache/handlers/CacheHandlerTests.java +++ b/src/test/java/org/prebid/cache/handlers/CacheHandlerTests.java @@ -1,7 +1,7 @@ package org.prebid.cache.handlers; -import org.prebid.cache.exceptions.RequestParsingException; import org.prebid.cache.exceptions.RepositoryException; +import org.prebid.cache.exceptions.RequestParsingException; import org.prebid.cache.handlers.cache.CacheHandler; import org.prebid.cache.model.Payload; import org.prebid.cache.model.PayloadTransfer; diff --git a/src/test/java/org/prebid/cache/handlers/GetCacheHandlerTests.java b/src/test/java/org/prebid/cache/handlers/GetCacheHandlerTests.java index 62a0837..c8e37be 100644 --- a/src/test/java/org/prebid/cache/handlers/GetCacheHandlerTests.java +++ b/src/test/java/org/prebid/cache/handlers/GetCacheHandlerTests.java @@ -47,14 +47,14 @@ @ExtendWith(SpringExtension.class) @ContextConfiguration(classes = { - GetCacheHandler.class, - PrebidServerResponseBuilder.class, - CacheConfig.class, - CacheConfig.class, - MetricsRecorderTest.class, - MetricsRecorder.class, - ApiConfig.class, - CircuitBreakerPropertyConfiguration.class + GetCacheHandler.class, + PrebidServerResponseBuilder.class, + CacheConfig.class, + CacheConfig.class, + MetricsRecorderTest.class, + MetricsRecorder.class, + ApiConfig.class, + CircuitBreakerPropertyConfiguration.class }) @EnableConfigurationProperties @SpringBootTest @@ -110,18 +110,14 @@ void testVerifyError() { verifyRepositoryError(handler); } - private static Consumer assertNotFoundStatusCode() { - return response -> assertEquals(response.statusCode().value(), 404); - } - @Test void testVerifyFetch() { given(repository.findById("prebid_a8db2208-d085-444c-9721-c1161d7f09ce")).willReturn(Mono.just(PAYLOAD_WRAPPER)); final var requestMono = MockServerRequest.builder() - .method(HttpMethod.GET) - .queryParam("uuid", "a8db2208-d085-444c-9721-c1161d7f09ce") - .build(); + .method(HttpMethod.GET) + .queryParam("uuid", "a8db2208-d085-444c-9721-c1161d7f09ce") + .build(); final var responseMono = handler.fetch(requestMono); @@ -142,18 +138,18 @@ void testVerifyFetchWithCacheHostParam() { final var requestMono = MockServerRequest.builder() .method(HttpMethod.GET) .header(CONTENT_TYPE, MediaType.APPLICATION_JSON_UTF8_VALUE) - .queryParam("uuid", "a8db2208-d085-444c-9721-c1161d7f09ce") - .queryParam("ch", "localhost:8080") - .build(); + .queryParam("uuid", "a8db2208-d085-444c-9721-c1161d7f09ce") + .queryParam("ch", "localhost:8080") + .build(); final var responseMono = handler.fetch(requestMono); responseMono.doOnEach(assertSignalStatusCode(200)).subscribe(); StepVerifier.create(responseMono) - .expectSubscription() - .expectNextMatches(t -> true) - .expectComplete() - .verify(); + .expectSubscription() + .expectNextMatches(t -> true) + .expectComplete() + .verify(); verify(getRequestedFor(urlPathEqualTo("/cache")) .withQueryParam("uuid", equalTo("a8db2208-d085-444c-9721-c1161d7f09ce")) @@ -164,34 +160,34 @@ void testVerifyFetchWithCacheHostParam() { @Test void testVerifyFailForNotFoundResourceWithCacheHostParam() { final var requestMono = MockServerRequest.builder() - .method(HttpMethod.GET) - .queryParam("uuid", "a8db2208-d085-444c-9721-c1161d7f09ce") - .queryParam("ch", "localhost:8080") - .build(); + .method(HttpMethod.GET) + .queryParam("uuid", "a8db2208-d085-444c-9721-c1161d7f09ce") + .queryParam("ch", "localhost:8080") + .build(); final var responseMono = handler.fetch(requestMono); responseMono.doOnEach(assertSignalStatusCode(404)).subscribe(); StepVerifier.create(responseMono) .consumeNextWith(assertNotFoundStatusCode()) - .expectComplete() - .verify(); + .expectComplete() + .verify(); } @Test void testVerifyFetchReturnsBadRequestWhenResponseStatusIsNotOk() { serverMock.stubFor(get(urlPathEqualTo("/cache")) - .willReturn(aResponse().withHeader(HttpHeaders.CONTENT_TYPE, "application/json;charset=utf-8") - .withStatus(201) - .withBody("{\"uuid\":\"2be04ba5-8f9b-4a1e-8100-d573c40312f8\"}"))); + .willReturn(aResponse().withHeader(HttpHeaders.CONTENT_TYPE, "application/json;charset=utf-8") + .withStatus(201) + .withBody("{\"uuid\":\"2be04ba5-8f9b-4a1e-8100-d573c40312f8\"}"))); final var requestMono = MockServerRequest.builder() .method(HttpMethod.GET) .header(CONTENT_TYPE, MediaType.APPLICATION_JSON_UTF8_VALUE) - .queryParam("uuid", "a8db2208-d085-444c-9721-c1161d7f09ce") - .queryParam("ch", "localhost:8080") - .build(); + .queryParam("uuid", "a8db2208-d085-444c-9721-c1161d7f09ce") + .queryParam("ch", "localhost:8080") + .build(); final var responseMono = handler.fetch(requestMono); @@ -208,10 +204,49 @@ void testVerifyFetchReturnsBadRequestWhenResponseStatusIsNotOk() { ); } + @Test + void testVerifyFetchReturnsBadRequestWhenNoUuid() { + final var requestMono = MockServerRequest.builder() + .method(HttpMethod.GET) + .header(CONTENT_TYPE, MediaType.APPLICATION_JSON_UTF8_VALUE) + .build(); + + final var responseMono = handler.fetch(requestMono); + + StepVerifier.create(responseMono) + .consumeNextWith(assertBadRequestStatusCode()) + .expectComplete() + .verify(); + } + + @Test + void testVerifyFetchReturnsBadRequestWhenUuidIsEmpty() { + final var requestMono = MockServerRequest.builder() + .method(HttpMethod.GET) + .header(CONTENT_TYPE, MediaType.APPLICATION_JSON_UTF8_VALUE) + .queryParam("uuid", "") + .build(); + + final var responseMono = handler.fetch(requestMono); + + StepVerifier.create(responseMono) + .consumeNextWith(assertBadRequestStatusCode()) + .expectComplete() + .verify(); + } + private static Consumer> assertSignalStatusCode(int statusCode) { return signal -> { assertTrue(signal.isOnComplete()); assertEquals(signal.get().statusCode().value(), statusCode); }; } + + private static Consumer assertNotFoundStatusCode() { + return response -> assertEquals(response.statusCode().value(), 404); + } + + private static Consumer assertBadRequestStatusCode() { + return response -> assertEquals(response.statusCode().value(), 400); + } } diff --git a/src/test/java/org/prebid/cache/handlers/PostCacheHandlerTests.java b/src/test/java/org/prebid/cache/handlers/PostCacheHandlerTests.java index b32f292..d8b9ab0 100644 --- a/src/test/java/org/prebid/cache/handlers/PostCacheHandlerTests.java +++ b/src/test/java/org/prebid/cache/handlers/PostCacheHandlerTests.java @@ -51,13 +51,13 @@ @ExtendWith(SpringExtension.class) @ContextConfiguration(classes = { - PostCacheHandler.class, - PrebidServerResponseBuilder.class, - CacheConfig.class, - MetricsRecorderTest.class, - MetricsRecorder.class, - ApiConfig.class, - CircuitBreakerPropertyConfiguration.class + PostCacheHandler.class, + PrebidServerResponseBuilder.class, + CacheConfig.class, + MetricsRecorderTest.class, + MetricsRecorder.class, + ApiConfig.class, + CircuitBreakerPropertyConfiguration.class }) @EnableConfigurationProperties @SpringBootTest diff --git a/src/test/java/org/prebid/cache/handlers/PostStorageHandlerTests.java b/src/test/java/org/prebid/cache/handlers/PostStorageHandlerTests.java index fe83cc3..9c8428e 100644 --- a/src/test/java/org/prebid/cache/handlers/PostStorageHandlerTests.java +++ b/src/test/java/org/prebid/cache/handlers/PostStorageHandlerTests.java @@ -9,9 +9,9 @@ import org.prebid.cache.builders.PrebidServerResponseBuilder; import org.prebid.cache.config.StorageConfig; import org.prebid.cache.handlers.storage.PostStorageHandler; -import org.prebid.cache.model.StoragePayload; import org.prebid.cache.model.Payload; import org.prebid.cache.model.PayloadWrapper; +import org.prebid.cache.model.StoragePayload; import org.prebid.cache.repository.redis.module.storage.ModuleCompositeRepository; import org.prebid.cache.routers.ApiConfig; import org.springframework.beans.factory.annotation.Autowired; diff --git a/src/test/java/org/prebid/cache/metrics/MetricsRecorderTest.java b/src/test/java/org/prebid/cache/metrics/MetricsRecorderTest.java index 8d0a8ca..2f3eccd 100644 --- a/src/test/java/org/prebid/cache/metrics/MetricsRecorderTest.java +++ b/src/test/java/org/prebid/cache/metrics/MetricsRecorderTest.java @@ -1,11 +1,11 @@ package org.prebid.cache.metrics; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Primary; import io.micrometer.core.instrument.MeterRegistry; +import io.micrometer.core.instrument.MockClock; import io.micrometer.core.instrument.simple.SimpleConfig; import io.micrometer.core.instrument.simple.SimpleMeterRegistry; -import io.micrometer.core.instrument.MockClock; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Primary; public class MetricsRecorderTest { From ee371a534f08c692d88a456017514d96dc7d9269 Mon Sep 17 00:00:00 2001 From: Oleksandr Zhevedenko <720803+Net-burst@users.noreply.github.com> Date: Fri, 28 Mar 2025 16:50:58 -0400 Subject: [PATCH 2/2] Update tests --- .../org/prebid/cache/functional/GeneralCacheSpec.kt | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/test/kotlin/org/prebid/cache/functional/GeneralCacheSpec.kt b/src/test/kotlin/org/prebid/cache/functional/GeneralCacheSpec.kt index 5f96717..1a28a63 100644 --- a/src/test/kotlin/org/prebid/cache/functional/GeneralCacheSpec.kt +++ b/src/test/kotlin/org/prebid/cache/functional/GeneralCacheSpec.kt @@ -29,7 +29,18 @@ class GeneralCacheSpec : ShouldSpec({ // then: Bad Request exception is thrown assertSoftly { exception.statusCode shouldBe BAD_REQUEST.value() - exception.responseBody shouldContain "\"message\":\"Invalid Parameter(s): uuid not found.\"" + exception.responseBody shouldContain "\"message\":\"Invalid Parameter(s): uuid not found or is empty.\"" + } + } + + should("throw an exception when 'uuid' query parameter is empty") { + // when: GET cache endpoint is called with empty 'uuid' query parameter + val exception = shouldThrowExactly { BaseSpec.getPrebidCacheApi().getCache("") } + + // then: Bad Request exception is thrown + assertSoftly { + exception.statusCode shouldBe BAD_REQUEST.value() + exception.responseBody shouldContain "\"message\":\"Invalid Parameter(s): uuid not found or is empty.\"" } }