From 5e57ae563c2ed84c149550a048ca179e9fe3e1f1 Mon Sep 17 00:00:00 2001 From: cjmamo <823038+cjmamo@users.noreply.github.com> Date: Wed, 5 Feb 2025 07:18:40 +0100 Subject: [PATCH 01/23] feat(DHIS2-18568): allow response timeout to be customised perf: use non-blocking WebClient instead of RestTemplate --- .../main/java/org/hisp/dhis/route/Route.java | 2 + .../dhis-services/dhis-service-core/pom.xml | 24 ++-- .../org/hisp/dhis/route/RouteService.java | 69 +++++------ .../dhis/support/config/ServiceConfig.java | 7 ++ .../controller/RouteControllerTest.java | 114 ++++++++++-------- 5 files changed, 120 insertions(+), 96 deletions(-) diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/route/Route.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/route/Route.java index 82e8f32c44a9..00289f4067e8 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/route/Route.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/route/Route.java @@ -67,6 +67,8 @@ public class Route extends BaseIdentifiableObject implements MetadataObject { /** Optional. Required authorities for invoking the route. */ @JsonProperty private List authorities = new ArrayList<>(); + @JsonProperty private Integer responseTimeout; + /** * If the route url ends with /** return true. Otherwise return false. * diff --git a/dhis-2/dhis-services/dhis-service-core/pom.xml b/dhis-2/dhis-services/dhis-service-core/pom.xml index dae45dad2ee3..61ff85cd1821 100644 --- a/dhis-2/dhis-services/dhis-service-core/pom.xml +++ b/dhis-2/dhis-services/dhis-service-core/pom.xml @@ -69,10 +69,6 @@ dhis-support-sql - - org.apache.httpcomponents.core5 - httpcore5 - org.hisp.dhis.parser dhis-antlr-expression-parser @@ -97,6 +93,21 @@ org.springframework.security spring-security-oauth2-core + + org.springframework + spring-webflux + ${spring.version} + + + io.projectreactor + reactor-core + 3.6.9 + + + io.projectreactor.netty + reactor-netty-http + 1.2.2 + com.nimbusds nimbus-jose-jwt @@ -197,10 +208,6 @@ com.google.code.gson gson - - org.apache.httpcomponents.client5 - httpclient5 - @@ -290,6 +297,7 @@ org.springframework spring-webmvc + org.springframework.security spring-security-crypto diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java index caa3603e2c1d..faa1b8fc2feb 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java @@ -32,6 +32,7 @@ import jakarta.servlet.http.HttpServletRequest; import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.time.Duration; import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -49,27 +50,22 @@ import lombok.RequiredArgsConstructor; import lombok.Setter; import lombok.extern.slf4j.Slf4j; -import org.apache.hc.client5.http.config.ConnectionConfig; -import org.apache.hc.client5.http.config.RequestConfig; -import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager; -import org.apache.hc.core5.http.io.SocketConfig; -import org.apache.hc.core5.util.Timeout; import org.hibernate.Hibernate; import org.hisp.dhis.feedback.BadRequestException; import org.hisp.dhis.user.UserDetails; import org.jasypt.encryption.pbe.PBEStringCleanablePasswordEncryptor; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; -import org.springframework.http.HttpEntity; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.ResponseEntity; -import org.springframework.http.client.HttpComponentsClientHttpRequestFactory; +import org.springframework.http.client.reactive.ClientHttpConnector; import org.springframework.stereotype.Service; import org.springframework.util.StreamUtils; import org.springframework.util.StringUtils; -import org.springframework.web.client.RestTemplate; +import org.springframework.web.reactive.function.client.WebClient; import org.springframework.web.util.UriComponentsBuilder; +import reactor.netty.http.client.HttpClientRequest; /** * @author Morten Olav Hansen @@ -85,7 +81,7 @@ public class RouteService { @Qualifier(AES_128_STRING_ENCRYPTOR) private final PBEStringCleanablePasswordEncryptor encryptor; - @Autowired @Getter @Setter private RestTemplate restTemplate; + @Autowired @Getter @Setter private ClientHttpConnector clientHttpConnector; private static final Set ALLOWED_REQUEST_HEADERS = Set.of( @@ -117,32 +113,11 @@ public class RouteService { "cache-control", "last-modified", "etag"); + private WebClient webClient; @PostConstruct public void postConstruct() { - // Connect timeout - ConnectionConfig connectionConfig = - ConnectionConfig.custom().setConnectTimeout(Timeout.ofMilliseconds(5_000)).build(); - - // Socket timeout - SocketConfig socketConfig = - SocketConfig.custom().setSoTimeout(Timeout.ofMilliseconds(10_000)).build(); - - // Connection request timeout - RequestConfig requestConfig = - RequestConfig.custom().setConnectionRequestTimeout(Timeout.ofMilliseconds(1_000)).build(); - - PoolingHttpClientConnectionManager connectionManager = new PoolingHttpClientConnectionManager(); - connectionManager.setDefaultSocketConfig(socketConfig); - connectionManager.setDefaultConnectionConfig(connectionConfig); - - org.apache.hc.client5.http.classic.HttpClient httpClient = - org.apache.hc.client5.http.impl.classic.HttpClientBuilder.create() - .setConnectionManager(connectionManager) - .setDefaultRequestConfig(requestConfig) - .build(); - - restTemplate.setRequestFactory(new HttpComponentsClientHttpRequestFactory(httpClient)); + webClient = WebClient.builder().clientConnector(clientHttpConnector).build(); } /** @@ -224,12 +199,32 @@ public ResponseEntity execute( String body = StreamUtils.copyToString(request.getInputStream(), StandardCharsets.UTF_8); - HttpEntity entity = new HttpEntity<>(body, headers); - HttpMethod httpMethod = Objects.requireNonNullElse(HttpMethod.valueOf(request.getMethod()), HttpMethod.GET); String targetUri = uriComponentsBuilder.toUriString(); + WebClient.RequestHeadersSpec requestHeadersSpec = + webClient + .method(httpMethod) + .uri(targetUri) + .httpRequest( + clientHttpRequest -> { + Object nativeRequest = clientHttpRequest.getNativeRequest(); + if (nativeRequest instanceof HttpClientRequest httpClientRequest) { + if (route.getResponseTimeout() == null) { + httpClientRequest.responseTimeout(Duration.ofMillis(10_000)); + } else { + httpClientRequest.responseTimeout( + Duration.ofSeconds(route.getResponseTimeout())); + } + } + }) + .bodyValue(body); + for (Map.Entry> header : headers.entrySet()) { + requestHeadersSpec = + requestHeadersSpec.header(header.getKey(), header.getValue().toArray(new String[0])); + } + log.info( "Sending '{}' '{}' with route '{}' ('{}')", httpMethod, @@ -237,11 +232,9 @@ public ResponseEntity execute( route.getName(), route.getUid()); - ResponseEntity response = - restTemplate.exchange(targetUri, httpMethod, entity, String.class); - + WebClient.ResponseSpec responseSpec = requestHeadersSpec.retrieve(); + ResponseEntity response = responseSpec.toEntity(String.class).block(); HttpHeaders responseHeaders = filterResponseHeaders(response.getHeaders()); - String responseBody = response.getBody(); log.info( diff --git a/dhis-2/dhis-support/dhis-support-system/src/main/java/org/hisp/dhis/support/config/ServiceConfig.java b/dhis-2/dhis-support/dhis-support-system/src/main/java/org/hisp/dhis/support/config/ServiceConfig.java index 436fa502e221..c5606883fa5b 100644 --- a/dhis-2/dhis-support/dhis-support-system/src/main/java/org/hisp/dhis/support/config/ServiceConfig.java +++ b/dhis-2/dhis-support/dhis-support-system/src/main/java/org/hisp/dhis/support/config/ServiceConfig.java @@ -29,6 +29,8 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.http.client.reactive.ClientHttpConnector; +import org.springframework.http.client.reactive.ReactorClientHttpConnector; import org.springframework.web.client.RestTemplate; import org.springframework.web.util.UriComponentsBuilder; @@ -42,6 +44,11 @@ public RestTemplate restTemplate() { return new RestTemplate(); } + @Bean + public ClientHttpConnector clientHttpConnector() { + return new ReactorClientHttpConnector(); + } + @Bean public UriComponentsBuilder uriComponentsBuilder() { return UriComponentsBuilder.newInstance(); diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/RouteControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/RouteControllerTest.java index 2cf4072fb170..3f6a6812b815 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/RouteControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/RouteControllerTest.java @@ -31,15 +31,10 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; -import java.net.MalformedURLException; -import java.net.URL; +import java.util.Collections; import java.util.HashMap; import java.util.Map; import org.hisp.dhis.common.auth.ApiHeadersAuthScheme; @@ -50,40 +45,67 @@ import org.hisp.dhis.route.RouteService; import org.hisp.dhis.test.webapi.PostgresControllerIntegrationTestBase; import org.junit.jupiter.api.Test; -import org.mockito.ArgumentCaptor; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.http.HttpEntity; -import org.springframework.http.HttpHeaders; -import org.springframework.http.HttpMethod; -import org.springframework.http.HttpStatusCode; -import org.springframework.http.ResponseEntity; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.http.client.reactive.ClientHttpConnector; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.client.MockMvcHttpConnector; +import org.springframework.test.web.servlet.setup.MockMvcBuilders; import org.springframework.transaction.annotation.Transactional; -import org.springframework.web.client.RestTemplate; +import org.springframework.web.context.WebApplicationContext; @Transactional +@ContextConfiguration(classes = {RouteControllerTest.ClientHttpConnectorTestConfig.class}) class RouteControllerTest extends PostgresControllerIntegrationTestBase { @Autowired private RouteService service; @Autowired private ObjectMapper jsonMapper; - @Test - void testRunRouteGivenApiQueryParamsAuthScheme() - throws JsonProcessingException, MalformedURLException { - ArgumentCaptor urlArgumentCaptor = ArgumentCaptor.forClass(String.class); - - RestTemplate mockRestTemplate = mock(RestTemplate.class); - when(mockRestTemplate.exchange( - urlArgumentCaptor.capture(), - any(HttpMethod.class), - any(HttpEntity.class), - any(Class.class))) - .thenReturn( - new ResponseEntity<>( - jsonMapper.writeValueAsString(Map.of("name", "John Doe")), - HttpStatusCode.valueOf(200))); - service.setRestTemplate(mockRestTemplate); + @Autowired private ClientHttpConnector clientHttpConnector; + + @Configuration + public static class ClientHttpConnectorTestConfig { + @Autowired private ObjectMapper jsonMapper; + + @Autowired private WebApplicationContext webApplicationContext; + + @Bean + public ClientHttpConnector clientHttpConnector() { + MockMvc mockMvc = + MockMvcBuilders.webAppContextSetup(webApplicationContext) + .addFilter( + (request, response, chain) -> { + Map headers = new HashMap<>(); + for (String headerName : + Collections.list(((MockHttpServletRequest) request).getHeaderNames())) { + headers.put( + headerName, ((MockHttpServletRequest) request).getHeader(headerName)); + } + + String queryString = ((MockHttpServletRequest) request).getQueryString(); + response + .getWriter() + .write( + jsonMapper.writeValueAsString( + Map.of( + "name", + "John Doe", + "headers", + headers, + "queryString", + queryString != null ? queryString : ""))); + }) + .build(); + return new MockMvcHttpConnector(mockMvc); + } + } + @Test + void testRunRouteGivenApiQueryParamsAuthScheme() throws JsonProcessingException { Map route = new HashMap<>(); route.put("name", "route-under-test"); route.put("auth", Map.of("type", "api-query-params", "queryParams", Map.of("token", "foo"))); @@ -94,29 +116,15 @@ void testRunRouteGivenApiQueryParamsAuthScheme() GET( "/routes/{id}/run", postHttpResponse.content().get("response.uid").as(JsonString.class).string()); + assertStatus(HttpStatus.OK, runHttpResponse); assertEquals("John Doe", runHttpResponse.content().get("name").as(JsonString.class).string()); - - assertEquals("token=foo", new URL(urlArgumentCaptor.getValue()).getQuery()); + assertEquals( + "token=foo", runHttpResponse.content().get("queryString").as(JsonString.class).string()); } @Test void testRunRouteGivenApiHeadersAuthScheme() throws JsonProcessingException { - ArgumentCaptor> httpEntityArgumentCaptor = - ArgumentCaptor.forClass(HttpEntity.class); - - RestTemplate mockRestTemplate = mock(RestTemplate.class); - when(mockRestTemplate.exchange( - anyString(), - any(HttpMethod.class), - httpEntityArgumentCaptor.capture(), - any(Class.class))) - .thenReturn( - new ResponseEntity<>( - jsonMapper.writeValueAsString(Map.of("name", "John Doe")), - HttpStatusCode.valueOf(200))); - service.setRestTemplate(mockRestTemplate); - Map route = new HashMap<>(); route.put("name", "route-under-test"); route.put("auth", Map.of("type", "api-headers", "headers", Map.of("X-API-KEY", "foo"))); @@ -127,12 +135,18 @@ void testRunRouteGivenApiHeadersAuthScheme() throws JsonProcessingException { GET( "/routes/{id}/run", postHttpResponse.content().get("response.uid").as(JsonString.class).string()); + assertStatus(HttpStatus.OK, runHttpResponse); assertEquals("John Doe", runHttpResponse.content().get("name").as(JsonString.class).string()); - - HttpEntity capturedHttpEntity = httpEntityArgumentCaptor.getValue(); - HttpHeaders headers = capturedHttpEntity.getHeaders(); - assertEquals("foo", headers.get("X-API-KEY").get(0)); + assertEquals( + "foo", + runHttpResponse + .content() + .get("headers") + .asObject() + .get("X-API-KEY") + .as(JsonString.class) + .string()); } @Test From 88ff1b5b56b644b55a62feb8c8ab42b519ebb052 Mon Sep 17 00:00:00 2001 From: cjmamo <823038+cjmamo@users.noreply.github.com> Date: Thu, 6 Feb 2025 09:40:50 +0100 Subject: [PATCH 02/23] fix: persist response timeout tolerate HTTP errors in WebClient test: add test coverage for response timeout and HTTP errors --- .../main/java/org/hisp/dhis/route/Route.java | 3 +- .../dhis-services/dhis-service-core/pom.xml | 5 + .../org/hisp/dhis/route/RouteService.java | 19 +++- .../hisp/dhis/route/hibernate/Route.hbm.xml | 2 + .../V2_42_36__Add_responsetimeout_route.sql | 1 + dhis-2/dhis-test-web-api/pom.xml | 11 ++ .../controller/RouteControllerTest.java | 103 ++++++++++++++++++ dhis-2/pom.xml | 1 + 8 files changed, 142 insertions(+), 3 deletions(-) create mode 100644 dhis-2/dhis-support/dhis-support-db-migration/src/main/resources/org/hisp/dhis/db/migration/2.42/V2_42_36__Add_responsetimeout_route.sql diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/route/Route.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/route/Route.java index 00289f4067e8..c87af8e14a97 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/route/Route.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/route/Route.java @@ -67,7 +67,8 @@ public class Route extends BaseIdentifiableObject implements MetadataObject { /** Optional. Required authorities for invoking the route. */ @JsonProperty private List authorities = new ArrayList<>(); - @JsonProperty private Integer responseTimeout; + @JsonProperty(defaultValue = "10") + private Integer responseTimeout; /** * If the route url ends with /** return true. Otherwise return false. diff --git a/dhis-2/dhis-services/dhis-service-core/pom.xml b/dhis-2/dhis-services/dhis-service-core/pom.xml index 61ff85cd1821..478976e7809f 100644 --- a/dhis-2/dhis-services/dhis-service-core/pom.xml +++ b/dhis-2/dhis-services/dhis-service-core/pom.xml @@ -108,6 +108,11 @@ reactor-netty-http 1.2.2 + + io.netty + netty-handler + 4.1.116.Final + com.nimbusds nimbus-jose-jwt diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java index faa1b8fc2feb..2e32196c3eaf 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java @@ -29,6 +29,7 @@ import static org.hisp.dhis.config.HibernateEncryptionConfig.AES_128_STRING_ENCRYPTOR; +import io.netty.handler.timeout.ReadTimeoutException; import jakarta.servlet.http.HttpServletRequest; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -58,6 +59,7 @@ import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.http.client.reactive.ClientHttpConnector; import org.springframework.stereotype.Service; @@ -65,6 +67,7 @@ import org.springframework.util.StringUtils; import org.springframework.web.reactive.function.client.WebClient; import org.springframework.web.util.UriComponentsBuilder; +import reactor.core.publisher.Mono; import reactor.netty.http.client.HttpClientRequest; /** @@ -232,8 +235,20 @@ public ResponseEntity execute( route.getName(), route.getUid()); - WebClient.ResponseSpec responseSpec = requestHeadersSpec.retrieve(); - ResponseEntity response = responseSpec.toEntity(String.class).block(); + WebClient.ResponseSpec responseSpec = + requestHeadersSpec + .retrieve() + .onStatus(httpStatusCode -> true, clientResponse -> Mono.empty()); + + ResponseEntity response = + responseSpec + .toEntity(String.class) + .onErrorReturn( + throwable -> + throwable.getCause() != null + && throwable.getCause() instanceof ReadTimeoutException, + new ResponseEntity<>(HttpStatus.GATEWAY_TIMEOUT)) + .block(); HttpHeaders responseHeaders = filterResponseHeaders(response.getHeaders()); String responseBody = response.getBody(); diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/resources/org/hisp/dhis/route/hibernate/Route.hbm.xml b/dhis-2/dhis-services/dhis-service-core/src/main/resources/org/hisp/dhis/route/hibernate/Route.hbm.xml index 5a70e4152c75..e62a2c69015a 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/resources/org/hisp/dhis/route/hibernate/Route.hbm.xml +++ b/dhis-2/dhis-services/dhis-service-core/src/main/resources/org/hisp/dhis/route/hibernate/Route.hbm.xml @@ -39,5 +39,7 @@ + + diff --git a/dhis-2/dhis-support/dhis-support-db-migration/src/main/resources/org/hisp/dhis/db/migration/2.42/V2_42_36__Add_responsetimeout_route.sql b/dhis-2/dhis-support/dhis-support-db-migration/src/main/resources/org/hisp/dhis/db/migration/2.42/V2_42_36__Add_responsetimeout_route.sql new file mode 100644 index 000000000000..d7d395e5507b --- /dev/null +++ b/dhis-2/dhis-support/dhis-support-db-migration/src/main/resources/org/hisp/dhis/db/migration/2.42/V2_42_36__Add_responsetimeout_route.sql @@ -0,0 +1 @@ +ALTER TABLE route ADD COLUMN IF NOT EXISTS responsetimeout INTEGER; \ No newline at end of file diff --git a/dhis-2/dhis-test-web-api/pom.xml b/dhis-2/dhis-test-web-api/pom.xml index 5f1ea4868571..19798b76e655 100644 --- a/dhis-2/dhis-test-web-api/pom.xml +++ b/dhis-2/dhis-test-web-api/pom.xml @@ -361,6 +361,17 @@ + + org.testcontainers + testcontainers + test + + + org.mock-server + mockserver-client-java + 5.15.0 + test + diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/RouteControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/RouteControllerTest.java index 3f6a6812b815..fb50af3f2b8c 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/RouteControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/RouteControllerTest.java @@ -31,12 +31,14 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.mockserver.model.HttpRequest.request; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import java.util.Collections; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.TimeUnit; import org.hisp.dhis.common.auth.ApiHeadersAuthScheme; import org.hisp.dhis.common.auth.ApiQueryParamsAuthScheme; import org.hisp.dhis.http.HttpStatus; @@ -44,7 +46,12 @@ import org.hisp.dhis.jsontree.JsonString; import org.hisp.dhis.route.RouteService; import org.hisp.dhis.test.webapi.PostgresControllerIntegrationTestBase; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.mockserver.client.MockServerClient; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -56,6 +63,8 @@ import org.springframework.test.web.servlet.setup.MockMvcBuilders; import org.springframework.transaction.annotation.Transactional; import org.springframework.web.context.WebApplicationContext; +import org.testcontainers.containers.GenericContainer; +import org.testcontainers.containers.wait.strategy.HttpWaitStrategy; @Transactional @ContextConfiguration(classes = {RouteControllerTest.ClientHttpConnectorTestConfig.class}) @@ -104,6 +113,100 @@ public ClientHttpConnector clientHttpConnector() { } } + @Transactional + @Nested + public class IntegrationTest extends PostgresControllerIntegrationTestBase { + + private static GenericContainer mockServerContainer; + private MockServerClient mockServerClient; + + @BeforeAll + public static void beforeAll() { + mockServerContainer = + new GenericContainer<>("mockserver/mockserver") + .waitingFor(new HttpWaitStrategy().forStatusCode(404)) + .withExposedPorts(1080); + mockServerContainer.start(); + } + + @BeforeEach + public void beforeEach() { + mockServerClient = + new MockServerClient("localhost", mockServerContainer.getFirstMappedPort()); + } + + @AfterEach + public void afterEach() { + mockServerClient.reset(); + } + + @Test + public void testRunRouteWhenResponseDurationExceedsRouteResponseTimeout() + throws JsonProcessingException { + mockServerClient + .when(request().withPath("/")) + .respond( + org.mockserver.model.HttpResponse.response("{}").withDelay(TimeUnit.SECONDS, 10)); + + Map route = new HashMap<>(); + route.put("name", "route-under-test"); + route.put("url", "http://localhost:" + mockServerContainer.getFirstMappedPort()); + route.put("responseTimeout", 5); + + HttpResponse postHttpResponse = POST("/routes", jsonMapper.writeValueAsString(route)); + HttpResponse runHttpResponse = + GET( + "/routes/{id}/run", + postHttpResponse.content().get("response.uid").as(JsonString.class).string()); + + assertStatus(HttpStatus.GATEWAY_TIMEOUT, runHttpResponse); + } + + @Test + public void testRunRouteWhenResponseDurationDoesNotExceedRouteResponseTimeout() + throws JsonProcessingException { + mockServerClient + .when(request().withPath("/")) + .respond(org.mockserver.model.HttpResponse.response("{}")); + + Map route = new HashMap<>(); + route.put("name", "route-under-test"); + route.put("url", "http://localhost:" + mockServerContainer.getFirstMappedPort()); + route.put("responseTimeout", 5); + + HttpResponse postHttpResponse = POST("/routes", jsonMapper.writeValueAsString(route)); + HttpResponse runHttpResponse = + GET( + "/routes/{id}/run", + postHttpResponse.content().get("response.uid").as(JsonString.class).string()); + + assertStatus(HttpStatus.OK, runHttpResponse); + } + + @Test + public void testRunRouteWhenResponseIsHttpError() throws JsonProcessingException { + mockServerClient + .when(request().withPath("/")) + .respond( + org.mockserver.model.HttpResponse.response( + jsonMapper.writeValueAsString(Map.of("message", "not found"))) + .withStatusCode(404)); + + Map route = new HashMap<>(); + route.put("name", "route-under-test"); + route.put("url", "http://localhost:" + mockServerContainer.getFirstMappedPort()); + + HttpResponse postHttpResponse = POST("/routes", jsonMapper.writeValueAsString(route)); + HttpResponse runHttpResponse = + GET( + "/routes/{id}/run", + postHttpResponse.content().get("response.uid").as(JsonString.class).string()); + + assertStatus(HttpStatus.NOT_FOUND, runHttpResponse); + assertEquals("not found", runHttpResponse.error().getMessage()); + } + } + @Test void testRunRouteGivenApiQueryParamsAuthScheme() throws JsonProcessingException { Map route = new HashMap<>(); diff --git a/dhis-2/pom.xml b/dhis-2/pom.xml index 9cdb64c8be88..103dc320baf8 100644 --- a/dhis-2/pom.xml +++ b/dhis-2/pom.xml @@ -2152,6 +2152,7 @@ jasperreports.version=${jasperreports.version} org.springframework.session:spring-session-core jakarta.jms:jakarta.jms-api org.apache.activemq:artemis-server + org.mock-server:mockserver-core:jar From f83be8e4ef9077dccf3face2e50c282b3bf6c40c Mon Sep 17 00:00:00 2001 From: cjmamo <823038+cjmamo@users.noreply.github.com> Date: Thu, 6 Feb 2025 13:55:12 +0100 Subject: [PATCH 03/23] fix: validate response timeout --- .../controller/RouteControllerTest.java | 23 +++++++++++++++++++ .../webapi/controller/RouteController.java | 9 ++++++++ 2 files changed, 32 insertions(+) diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/RouteControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/RouteControllerTest.java index fb50af3f2b8c..b834246a9e31 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/RouteControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/RouteControllerTest.java @@ -38,6 +38,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.TimeUnit; import org.hisp.dhis.common.auth.ApiHeadersAuthScheme; import org.hisp.dhis.common.auth.ApiQueryParamsAuthScheme; @@ -316,4 +317,26 @@ void testGetRouteGivenApiQueryParamsAuthScheme() throws JsonProcessingException getHttpResponse.content().get("auth.type").as(JsonString.class).string()); assertFalse(getHttpResponse.content().get("auth").as(JsonObject.class).has("queryParams")); } + + @Test + void testAddRouteGivenResponseTimeoutGreaterThanMax() throws JsonProcessingException { + Map route = new HashMap<>(); + route.put("name", "route-under-test"); + route.put("url", "http://stub"); + route.put("responseTimeout", ThreadLocalRandom.current().nextInt(61, Integer.MAX_VALUE)); + + HttpResponse postHttpResponse = POST("/routes", jsonMapper.writeValueAsString(route)); + assertStatus(HttpStatus.CONFLICT, postHttpResponse); + } + + @Test + void testAddRouteGivenResponseTimeoutLessThanMin() throws JsonProcessingException { + Map route = new HashMap<>(); + route.put("name", "route-under-test"); + route.put("url", "http://stub"); + route.put("responseTimeout", ThreadLocalRandom.current().nextInt(Integer.MIN_VALUE, 1)); + + HttpResponse postHttpResponse = POST("/routes", jsonMapper.writeValueAsString(route)); + assertStatus(HttpStatus.CONFLICT, postHttpResponse); + } } diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/RouteController.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/RouteController.java index fdda5c0d2d13..66346ed58b19 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/RouteController.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/RouteController.java @@ -129,6 +129,15 @@ private Optional getSubPath(String path, String id) { return Optional.empty(); } + @Override + protected void preCreateEntity(Route route) throws ConflictException { + if (route.getResponseTimeout() != null + && (route.getResponseTimeout() < 1 || route.getResponseTimeout() > 60)) { + throw new ConflictException( + "Route response timeout must be greater than 0 seconds and less than or equal to 60 seconds"); + } + } + /** * Disable the collection API for /api/routes endpoint. This conflicts with sub-path based routes * and is not supported by the Route API (no identifiable object collections). From bf79d6c20e1775fff2741a3d3cbcf9670580649b Mon Sep 17 00:00:00 2001 From: cjmamo <823038+cjmamo@users.noreply.github.com> Date: Thu, 6 Feb 2025 14:10:19 +0100 Subject: [PATCH 04/23] fix: solve SonarQube issues --- .../src/main/java/org/hisp/dhis/route/RouteService.java | 3 +-- .../hisp/dhis/webapi/controller/RouteControllerTest.java | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java index 2e32196c3eaf..b9c9accac2f3 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java @@ -245,8 +245,7 @@ public ResponseEntity execute( .toEntity(String.class) .onErrorReturn( throwable -> - throwable.getCause() != null - && throwable.getCause() instanceof ReadTimeoutException, + throwable.getCause() instanceof ReadTimeoutException, new ResponseEntity<>(HttpStatus.GATEWAY_TIMEOUT)) .block(); HttpHeaders responseHeaders = filterResponseHeaders(response.getHeaders()); diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/RouteControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/RouteControllerTest.java index b834246a9e31..c5aa3545b947 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/RouteControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/RouteControllerTest.java @@ -142,7 +142,7 @@ public void afterEach() { } @Test - public void testRunRouteWhenResponseDurationExceedsRouteResponseTimeout() + void testRunRouteWhenResponseDurationExceedsRouteResponseTimeout() throws JsonProcessingException { mockServerClient .when(request().withPath("/")) @@ -164,7 +164,7 @@ public void testRunRouteWhenResponseDurationExceedsRouteResponseTimeout() } @Test - public void testRunRouteWhenResponseDurationDoesNotExceedRouteResponseTimeout() + void testRunRouteWhenResponseDurationDoesNotExceedRouteResponseTimeout() throws JsonProcessingException { mockServerClient .when(request().withPath("/")) @@ -185,7 +185,7 @@ public void testRunRouteWhenResponseDurationDoesNotExceedRouteResponseTimeout() } @Test - public void testRunRouteWhenResponseIsHttpError() throws JsonProcessingException { + void testRunRouteWhenResponseIsHttpError() throws JsonProcessingException { mockServerClient .when(request().withPath("/")) .respond( From e9cfe03c43cb23c6155dd95eb3cac4901882c86b Mon Sep 17 00:00:00 2001 From: cjmamo <823038+cjmamo@users.noreply.github.com> Date: Thu, 6 Feb 2025 14:15:18 +0100 Subject: [PATCH 05/23] style: format --- .../src/main/java/org/hisp/dhis/route/RouteService.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java index b9c9accac2f3..935140e7f37f 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java @@ -244,8 +244,7 @@ public ResponseEntity execute( responseSpec .toEntity(String.class) .onErrorReturn( - throwable -> - throwable.getCause() instanceof ReadTimeoutException, + throwable -> throwable.getCause() instanceof ReadTimeoutException, new ResponseEntity<>(HttpStatus.GATEWAY_TIMEOUT)) .block(); HttpHeaders responseHeaders = filterResponseHeaders(response.getHeaders()); From b40ee46c4807dec7e6deac6bff23bc4f212c02f1 Mon Sep 17 00:00:00 2001 From: cjmamo <823038+cjmamo@users.noreply.github.com> Date: Thu, 6 Feb 2025 14:30:48 +0100 Subject: [PATCH 06/23] fix: update to newer patch version of Spring to solve reported security issues --- dhis-2/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dhis-2/pom.xml b/dhis-2/pom.xml index fc6b86e1d45d..72d37b0d3035 100644 --- a/dhis-2/pom.xml +++ b/dhis-2/pom.xml @@ -103,7 +103,7 @@ 3.5.3 - 6.1.12 + 6.1.14 3.4.1 2.7.18 2.7.4 From 1d64c7920fb09f30d694b875d5571ddca15654fb Mon Sep 17 00:00:00 2001 From: cjmamo <823038+cjmamo@users.noreply.github.com> Date: Thu, 6 Feb 2025 14:52:15 +0100 Subject: [PATCH 07/23] fix: validate Route response time update --- .../controller/RouteControllerTest.java | 36 +++++++++++++++++++ .../webapi/controller/RouteController.java | 9 +++++ 2 files changed, 45 insertions(+) diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/RouteControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/RouteControllerTest.java index c5aa3545b947..062b02e9d9fd 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/RouteControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/RouteControllerTest.java @@ -339,4 +339,40 @@ void testAddRouteGivenResponseTimeoutLessThanMin() throws JsonProcessingExceptio HttpResponse postHttpResponse = POST("/routes", jsonMapper.writeValueAsString(route)); assertStatus(HttpStatus.CONFLICT, postHttpResponse); } + + @Test + void testUpdateRouteGivenResponseTimeoutGreaterThanMax() throws JsonProcessingException { + Map route = new HashMap<>(); + route.put("name", "route-under-test"); + route.put("url", "http://stub"); + + HttpResponse postHttpResponse = POST("/routes", jsonMapper.writeValueAsString(route)); + + route.put("responseTimeout", ThreadLocalRandom.current().nextInt(61, Integer.MAX_VALUE)); + HttpResponse updateHttpResponse = + PUT( + "/routes/" + + postHttpResponse.content().get("response.uid").as(JsonString.class).string(), + jsonMapper.writeValueAsString(route)); + + assertStatus(HttpStatus.CONFLICT, updateHttpResponse); + } + + @Test + void testUpdateRouteGivenResponseTimeoutLessThanMin() throws JsonProcessingException { + Map route = new HashMap<>(); + route.put("name", "route-under-test"); + route.put("url", "http://stub"); + + HttpResponse postHttpResponse = POST("/routes", jsonMapper.writeValueAsString(route)); + + route.put("responseTimeout", ThreadLocalRandom.current().nextInt(Integer.MIN_VALUE, 1)); + HttpResponse updateHttpResponse = + PUT( + "/routes/" + + postHttpResponse.content().get("response.uid").as(JsonString.class).string(), + jsonMapper.writeValueAsString(route)); + + assertStatus(HttpStatus.CONFLICT, updateHttpResponse); + } } diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/RouteController.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/RouteController.java index 66346ed58b19..14e478f907bc 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/RouteController.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/RouteController.java @@ -131,6 +131,15 @@ private Optional getSubPath(String path, String id) { @Override protected void preCreateEntity(Route route) throws ConflictException { + validateRoute(route); + } + + @Override + protected void preUpdateEntity(Route route, Route newRoute) throws ConflictException { + validateRoute(newRoute); + } + + protected void validateRoute(Route route) throws ConflictException { if (route.getResponseTimeout() != null && (route.getResponseTimeout() < 1 || route.getResponseTimeout() > 60)) { throw new ConflictException( From 5c0206eadab2302458bed7c426fc33b8a2aa3865 Mon Sep 17 00:00:00 2001 From: cjmamo <823038+cjmamo@users.noreply.github.com> Date: Thu, 6 Feb 2025 18:46:19 +0100 Subject: [PATCH 08/23] fix: handle max body size --- .../org/hisp/dhis/route/RouteService.java | 14 ++++++++++ .../controller/RouteControllerTest.java | 26 +++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java index 935140e7f37f..3ea4b4cf3805 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java @@ -29,6 +29,7 @@ import static org.hisp.dhis.config.HibernateEncryptionConfig.AES_128_STRING_ENCRYPTOR; +import com.fasterxml.jackson.databind.ObjectMapper; import io.netty.handler.timeout.ReadTimeoutException; import jakarta.servlet.http.HttpServletRequest; import java.io.IOException; @@ -57,6 +58,7 @@ import org.jasypt.encryption.pbe.PBEStringCleanablePasswordEncryptor; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.core.io.buffer.DataBufferLimitException; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; @@ -86,6 +88,8 @@ public class RouteService { @Autowired @Getter @Setter private ClientHttpConnector clientHttpConnector; + @Autowired @Getter @Setter private ObjectMapper objectMapper; + private static final Set ALLOWED_REQUEST_HEADERS = Set.of( "accept", @@ -246,6 +250,16 @@ public ResponseEntity execute( .onErrorReturn( throwable -> throwable.getCause() instanceof ReadTimeoutException, new ResponseEntity<>(HttpStatus.GATEWAY_TIMEOUT)) + .onErrorResume( + throwable -> throwable.getCause() instanceof DataBufferLimitException, + throwable -> { + String message = + String.format( + """ +{"message":"%s"}""", + throwable.getCause().getMessage()); + return Mono.just(new ResponseEntity<>(message, HttpStatus.BAD_GATEWAY)); + }) .block(); HttpHeaders responseHeaders = filterResponseHeaders(response.getHeaders()); String responseBody = response.getBody(); diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/RouteControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/RouteControllerTest.java index 062b02e9d9fd..19be6b90af07 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/RouteControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/RouteControllerTest.java @@ -31,6 +31,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockserver.model.HttpRequest.request; import com.fasterxml.jackson.core.JsonProcessingException; @@ -141,6 +142,31 @@ public void afterEach() { mockServerClient.reset(); } + @Test + void testRunRouteWhenResponseBodyExceedsLimit() throws JsonProcessingException { + mockServerClient + .when(request().withPath("/")) + .respond(org.mockserver.model.HttpResponse.response("{}")); + + Map route = new HashMap<>(); + route.put("name", "route-under-test"); + route.put("url", "https://dhis2.org"); + + HttpResponse postHttpResponse = POST("/routes", jsonMapper.writeValueAsString(route)); + HttpResponse runHttpResponse = + GET( + "/routes/{id}/run", + postHttpResponse.content().get("response.uid").as(JsonString.class).string()); + + String message = + runHttpResponse + .error(HttpStatus.BAD_GATEWAY) + .get("message") + .as(JsonString.class) + .string(); + assertTrue(message.startsWith("Exceeded limit on max bytes to buffer : ")); + } + @Test void testRunRouteWhenResponseDurationExceedsRouteResponseTimeout() throws JsonProcessingException { From 7ec47f84d4a4584acaf32b73f75d17e50a4ad5b2 Mon Sep 17 00:00:00 2001 From: cjmamo <823038+cjmamo@users.noreply.github.com> Date: Thu, 6 Feb 2025 19:26:52 +0100 Subject: [PATCH 09/23] refactor: eliminate code smell --- .../src/main/java/org/hisp/dhis/route/RouteService.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java index 3ea4b4cf3805..7fc53f4ae148 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java @@ -254,10 +254,7 @@ public ResponseEntity execute( throwable -> throwable.getCause() instanceof DataBufferLimitException, throwable -> { String message = - String.format( - """ -{"message":"%s"}""", - throwable.getCause().getMessage()); + String.format("{\"message\":\"%s\"}", throwable.getCause().getMessage()); return Mono.just(new ResponseEntity<>(message, HttpStatus.BAD_GATEWAY)); }) .block(); From 677b880df38de31d91e451341f0284cf141fff28 Mon Sep 17 00:00:00 2001 From: cjmamo <823038+cjmamo@users.noreply.github.com> Date: Thu, 6 Feb 2025 19:39:24 +0100 Subject: [PATCH 10/23] refactor: add DB constraint --- .../db/migration/2.42/V2_42_36__Add_responsetimeout_route.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dhis-2/dhis-support/dhis-support-db-migration/src/main/resources/org/hisp/dhis/db/migration/2.42/V2_42_36__Add_responsetimeout_route.sql b/dhis-2/dhis-support/dhis-support-db-migration/src/main/resources/org/hisp/dhis/db/migration/2.42/V2_42_36__Add_responsetimeout_route.sql index d7d395e5507b..79ea2c56a331 100644 --- a/dhis-2/dhis-support/dhis-support-db-migration/src/main/resources/org/hisp/dhis/db/migration/2.42/V2_42_36__Add_responsetimeout_route.sql +++ b/dhis-2/dhis-support/dhis-support-db-migration/src/main/resources/org/hisp/dhis/db/migration/2.42/V2_42_36__Add_responsetimeout_route.sql @@ -1 +1 @@ -ALTER TABLE route ADD COLUMN IF NOT EXISTS responsetimeout INTEGER; \ No newline at end of file +ALTER TABLE route ADD COLUMN IF NOT EXISTS responsetimeout INTEGER CHECK (responsetimeout > 0 AND responsetimeout <= 60); \ No newline at end of file From 266326be6c9dec4a708a6f3f4bf8c3d05c739530 Mon Sep 17 00:00:00 2001 From: cjmamo <823038+cjmamo@users.noreply.github.com> Date: Mon, 10 Feb 2025 10:12:13 +0100 Subject: [PATCH 11/23] refactor: rename responseTimeout to responseTimeoutSeconds use int instead of Integer for responseTimeout to remove null checks remove redundant annotations rename references use `Json.object` instead handcrafting JSON in RouteService --- .../main/java/org/hisp/dhis/route/Route.java | 5 ++- .../org/hisp/dhis/route/RouteService.java | 27 +++++-------- .../hisp/dhis/route/hibernate/Route.hbm.xml | 2 +- .../V2_42_36__Add_responsetimeout_route.sql | 2 +- .../controller/RouteControllerTest.java | 40 +++++++++---------- .../webapi/controller/RouteController.java | 3 +- 6 files changed, 37 insertions(+), 42 deletions(-) diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/route/Route.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/route/Route.java index c87af8e14a97..3221fbd41817 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/route/Route.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/route/Route.java @@ -48,6 +48,7 @@ @EqualsAndHashCode(callSuper = true) @Accessors(chain = true) public class Route extends BaseIdentifiableObject implements MetadataObject { + public static final String DEFAULT_RESPONSE_TIMEOUT_SECONDS = "10"; public static final String PATH_WILDCARD_SUFFIX = "/**"; @JsonProperty private String description; @@ -67,8 +68,8 @@ public class Route extends BaseIdentifiableObject implements MetadataObject { /** Optional. Required authorities for invoking the route. */ @JsonProperty private List authorities = new ArrayList<>(); - @JsonProperty(defaultValue = "10") - private Integer responseTimeout; + @JsonProperty(defaultValue = DEFAULT_RESPONSE_TIMEOUT_SECONDS) + private int responseTimeoutSeconds = Integer.parseInt(DEFAULT_RESPONSE_TIMEOUT_SECONDS); /** * If the route url ends with /** return true. Otherwise return false. diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java index 7fc53f4ae148..c4582b8b4eed 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java @@ -29,7 +29,6 @@ import static org.hisp.dhis.config.HibernateEncryptionConfig.AES_128_STRING_ENCRYPTOR; -import com.fasterxml.jackson.databind.ObjectMapper; import io.netty.handler.timeout.ReadTimeoutException; import jakarta.servlet.http.HttpServletRequest; import java.io.IOException; @@ -48,15 +47,14 @@ import java.util.function.Function; import javax.annotation.Nonnull; import javax.annotation.PostConstruct; -import lombok.Getter; import lombok.RequiredArgsConstructor; -import lombok.Setter; import lombok.extern.slf4j.Slf4j; import org.hibernate.Hibernate; import org.hisp.dhis.feedback.BadRequestException; +import org.hisp.dhis.jsontree.Json; +import org.hisp.dhis.jsontree.JsonObject; import org.hisp.dhis.user.UserDetails; import org.jasypt.encryption.pbe.PBEStringCleanablePasswordEncryptor; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.core.io.buffer.DataBufferLimitException; import org.springframework.http.HttpHeaders; @@ -86,9 +84,7 @@ public class RouteService { @Qualifier(AES_128_STRING_ENCRYPTOR) private final PBEStringCleanablePasswordEncryptor encryptor; - @Autowired @Getter @Setter private ClientHttpConnector clientHttpConnector; - - @Autowired @Getter @Setter private ObjectMapper objectMapper; + private final ClientHttpConnector clientHttpConnector; private static final Set ALLOWED_REQUEST_HEADERS = Set.of( @@ -218,12 +214,8 @@ public ResponseEntity execute( clientHttpRequest -> { Object nativeRequest = clientHttpRequest.getNativeRequest(); if (nativeRequest instanceof HttpClientRequest httpClientRequest) { - if (route.getResponseTimeout() == null) { - httpClientRequest.responseTimeout(Duration.ofMillis(10_000)); - } else { - httpClientRequest.responseTimeout( - Duration.ofSeconds(route.getResponseTimeout())); - } + httpClientRequest.responseTimeout( + Duration.ofSeconds(route.getResponseTimeoutSeconds())); } }) .bodyValue(body); @@ -253,9 +245,12 @@ public ResponseEntity execute( .onErrorResume( throwable -> throwable.getCause() instanceof DataBufferLimitException, throwable -> { - String message = - String.format("{\"message\":\"%s\"}", throwable.getCause().getMessage()); - return Mono.just(new ResponseEntity<>(message, HttpStatus.BAD_GATEWAY)); + JsonObject message = + Json.object( + jsonObjectBuilder -> + jsonObjectBuilder.addString( + "message", throwable.getCause().getMessage())); + return Mono.just(new ResponseEntity<>(message.toJson(), HttpStatus.BAD_GATEWAY)); }) .block(); HttpHeaders responseHeaders = filterResponseHeaders(response.getHeaders()); diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/resources/org/hisp/dhis/route/hibernate/Route.hbm.xml b/dhis-2/dhis-services/dhis-service-core/src/main/resources/org/hisp/dhis/route/hibernate/Route.hbm.xml index e62a2c69015a..23712f5ede3a 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/resources/org/hisp/dhis/route/hibernate/Route.hbm.xml +++ b/dhis-2/dhis-services/dhis-service-core/src/main/resources/org/hisp/dhis/route/hibernate/Route.hbm.xml @@ -39,7 +39,7 @@ - + diff --git a/dhis-2/dhis-support/dhis-support-db-migration/src/main/resources/org/hisp/dhis/db/migration/2.42/V2_42_36__Add_responsetimeout_route.sql b/dhis-2/dhis-support/dhis-support-db-migration/src/main/resources/org/hisp/dhis/db/migration/2.42/V2_42_36__Add_responsetimeout_route.sql index 79ea2c56a331..b3fad58f301d 100644 --- a/dhis-2/dhis-support/dhis-support-db-migration/src/main/resources/org/hisp/dhis/db/migration/2.42/V2_42_36__Add_responsetimeout_route.sql +++ b/dhis-2/dhis-support/dhis-support-db-migration/src/main/resources/org/hisp/dhis/db/migration/2.42/V2_42_36__Add_responsetimeout_route.sql @@ -1 +1 @@ -ALTER TABLE route ADD COLUMN IF NOT EXISTS responsetimeout INTEGER CHECK (responsetimeout > 0 AND responsetimeout <= 60); \ No newline at end of file +ALTER TABLE route ADD COLUMN IF NOT EXISTS responsetimeoutseconds INTEGER NOT NULL DEFAULT 10 CHECK (responsetimeoutseconds > 0 AND responsetimeoutseconds <= 60); \ No newline at end of file diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/RouteControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/RouteControllerTest.java index 19be6b90af07..16837329426c 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/RouteControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/RouteControllerTest.java @@ -119,32 +119,32 @@ public ClientHttpConnector clientHttpConnector() { @Nested public class IntegrationTest extends PostgresControllerIntegrationTestBase { - private static GenericContainer mockServerContainer; - private MockServerClient mockServerClient; + private static GenericContainer routeTargetMockServerContainer; + private MockServerClient routeTargetMockServerClient; @BeforeAll public static void beforeAll() { - mockServerContainer = + routeTargetMockServerContainer = new GenericContainer<>("mockserver/mockserver") .waitingFor(new HttpWaitStrategy().forStatusCode(404)) .withExposedPorts(1080); - mockServerContainer.start(); + routeTargetMockServerContainer.start(); } @BeforeEach public void beforeEach() { - mockServerClient = - new MockServerClient("localhost", mockServerContainer.getFirstMappedPort()); + routeTargetMockServerClient = + new MockServerClient("localhost", routeTargetMockServerContainer.getFirstMappedPort()); } @AfterEach public void afterEach() { - mockServerClient.reset(); + routeTargetMockServerClient.reset(); } @Test void testRunRouteWhenResponseBodyExceedsLimit() throws JsonProcessingException { - mockServerClient + routeTargetMockServerClient .when(request().withPath("/")) .respond(org.mockserver.model.HttpResponse.response("{}")); @@ -170,15 +170,15 @@ void testRunRouteWhenResponseBodyExceedsLimit() throws JsonProcessingException { @Test void testRunRouteWhenResponseDurationExceedsRouteResponseTimeout() throws JsonProcessingException { - mockServerClient + routeTargetMockServerClient .when(request().withPath("/")) .respond( org.mockserver.model.HttpResponse.response("{}").withDelay(TimeUnit.SECONDS, 10)); Map route = new HashMap<>(); route.put("name", "route-under-test"); - route.put("url", "http://localhost:" + mockServerContainer.getFirstMappedPort()); - route.put("responseTimeout", 5); + route.put("url", "http://localhost:" + routeTargetMockServerContainer.getFirstMappedPort()); + route.put("responseTimeoutSeconds", 5); HttpResponse postHttpResponse = POST("/routes", jsonMapper.writeValueAsString(route)); HttpResponse runHttpResponse = @@ -192,14 +192,14 @@ void testRunRouteWhenResponseDurationExceedsRouteResponseTimeout() @Test void testRunRouteWhenResponseDurationDoesNotExceedRouteResponseTimeout() throws JsonProcessingException { - mockServerClient + routeTargetMockServerClient .when(request().withPath("/")) .respond(org.mockserver.model.HttpResponse.response("{}")); Map route = new HashMap<>(); route.put("name", "route-under-test"); - route.put("url", "http://localhost:" + mockServerContainer.getFirstMappedPort()); - route.put("responseTimeout", 5); + route.put("url", "http://localhost:" + routeTargetMockServerContainer.getFirstMappedPort()); + route.put("responseTimeoutSeconds", 5); HttpResponse postHttpResponse = POST("/routes", jsonMapper.writeValueAsString(route)); HttpResponse runHttpResponse = @@ -212,7 +212,7 @@ void testRunRouteWhenResponseDurationDoesNotExceedRouteResponseTimeout() @Test void testRunRouteWhenResponseIsHttpError() throws JsonProcessingException { - mockServerClient + routeTargetMockServerClient .when(request().withPath("/")) .respond( org.mockserver.model.HttpResponse.response( @@ -221,7 +221,7 @@ void testRunRouteWhenResponseIsHttpError() throws JsonProcessingException { Map route = new HashMap<>(); route.put("name", "route-under-test"); - route.put("url", "http://localhost:" + mockServerContainer.getFirstMappedPort()); + route.put("url", "http://localhost:" + routeTargetMockServerContainer.getFirstMappedPort()); HttpResponse postHttpResponse = POST("/routes", jsonMapper.writeValueAsString(route)); HttpResponse runHttpResponse = @@ -349,7 +349,7 @@ void testAddRouteGivenResponseTimeoutGreaterThanMax() throws JsonProcessingExcep Map route = new HashMap<>(); route.put("name", "route-under-test"); route.put("url", "http://stub"); - route.put("responseTimeout", ThreadLocalRandom.current().nextInt(61, Integer.MAX_VALUE)); + route.put("responseTimeoutSeconds", ThreadLocalRandom.current().nextInt(61, Integer.MAX_VALUE)); HttpResponse postHttpResponse = POST("/routes", jsonMapper.writeValueAsString(route)); assertStatus(HttpStatus.CONFLICT, postHttpResponse); @@ -360,7 +360,7 @@ void testAddRouteGivenResponseTimeoutLessThanMin() throws JsonProcessingExceptio Map route = new HashMap<>(); route.put("name", "route-under-test"); route.put("url", "http://stub"); - route.put("responseTimeout", ThreadLocalRandom.current().nextInt(Integer.MIN_VALUE, 1)); + route.put("responseTimeoutSeconds", ThreadLocalRandom.current().nextInt(Integer.MIN_VALUE, 1)); HttpResponse postHttpResponse = POST("/routes", jsonMapper.writeValueAsString(route)); assertStatus(HttpStatus.CONFLICT, postHttpResponse); @@ -374,7 +374,7 @@ void testUpdateRouteGivenResponseTimeoutGreaterThanMax() throws JsonProcessingEx HttpResponse postHttpResponse = POST("/routes", jsonMapper.writeValueAsString(route)); - route.put("responseTimeout", ThreadLocalRandom.current().nextInt(61, Integer.MAX_VALUE)); + route.put("responseTimeoutSeconds", ThreadLocalRandom.current().nextInt(61, Integer.MAX_VALUE)); HttpResponse updateHttpResponse = PUT( "/routes/" @@ -392,7 +392,7 @@ void testUpdateRouteGivenResponseTimeoutLessThanMin() throws JsonProcessingExcep HttpResponse postHttpResponse = POST("/routes", jsonMapper.writeValueAsString(route)); - route.put("responseTimeout", ThreadLocalRandom.current().nextInt(Integer.MIN_VALUE, 1)); + route.put("responseTimeoutSeconds", ThreadLocalRandom.current().nextInt(Integer.MIN_VALUE, 1)); HttpResponse updateHttpResponse = PUT( "/routes/" diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/RouteController.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/RouteController.java index 14e478f907bc..664009ddec4e 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/RouteController.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/RouteController.java @@ -140,8 +140,7 @@ protected void preUpdateEntity(Route route, Route newRoute) throws ConflictExcep } protected void validateRoute(Route route) throws ConflictException { - if (route.getResponseTimeout() != null - && (route.getResponseTimeout() < 1 || route.getResponseTimeout() > 60)) { + if (route.getResponseTimeoutSeconds() < 1 || route.getResponseTimeoutSeconds() > 60) { throw new ConflictException( "Route response timeout must be greater than 0 seconds and less than or equal to 60 seconds"); } From 85e607b87cc92ac9332e9b30fb33e1f02a44456c Mon Sep 17 00:00:00 2001 From: cjmamo <823038+cjmamo@users.noreply.github.com> Date: Mon, 10 Feb 2025 11:13:21 +0100 Subject: [PATCH 12/23] fix(security): reduce default to 5s after conversation with @amcgee --- dhis-2/dhis-api/src/main/java/org/hisp/dhis/route/Route.java | 2 +- .../db/migration/2.42/V2_42_36__Add_responsetimeout_route.sql | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/route/Route.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/route/Route.java index 3221fbd41817..cd89d1966ac6 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/route/Route.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/route/Route.java @@ -48,7 +48,7 @@ @EqualsAndHashCode(callSuper = true) @Accessors(chain = true) public class Route extends BaseIdentifiableObject implements MetadataObject { - public static final String DEFAULT_RESPONSE_TIMEOUT_SECONDS = "10"; + public static final String DEFAULT_RESPONSE_TIMEOUT_SECONDS = "5"; public static final String PATH_WILDCARD_SUFFIX = "/**"; @JsonProperty private String description; diff --git a/dhis-2/dhis-support/dhis-support-db-migration/src/main/resources/org/hisp/dhis/db/migration/2.42/V2_42_36__Add_responsetimeout_route.sql b/dhis-2/dhis-support/dhis-support-db-migration/src/main/resources/org/hisp/dhis/db/migration/2.42/V2_42_36__Add_responsetimeout_route.sql index b3fad58f301d..83230dd50cfa 100644 --- a/dhis-2/dhis-support/dhis-support-db-migration/src/main/resources/org/hisp/dhis/db/migration/2.42/V2_42_36__Add_responsetimeout_route.sql +++ b/dhis-2/dhis-support/dhis-support-db-migration/src/main/resources/org/hisp/dhis/db/migration/2.42/V2_42_36__Add_responsetimeout_route.sql @@ -1 +1 @@ -ALTER TABLE route ADD COLUMN IF NOT EXISTS responsetimeoutseconds INTEGER NOT NULL DEFAULT 10 CHECK (responsetimeoutseconds > 0 AND responsetimeoutseconds <= 60); \ No newline at end of file +ALTER TABLE route ADD COLUMN IF NOT EXISTS responsetimeoutseconds INTEGER NOT NULL DEFAULT 5 CHECK (responsetimeoutseconds > 0 AND responsetimeoutseconds <= 60); \ No newline at end of file From db77f1024773edc811f3690202d2e11791f410eb Mon Sep 17 00:00:00 2001 From: cjmamo <823038+cjmamo@users.noreply.github.com> Date: Tue, 11 Feb 2025 13:08:15 +0100 Subject: [PATCH 13/23] perf: stream request and response bodies feat: support binary bodies --- .../dhis-services/dhis-service-core/pom.xml | 5 + .../org/hisp/dhis/route/RouteService.java | 52 ++++-- .../webapi/ControllerIntegrationTestBase.java | 7 + .../controller/RouteControllerTest.java | 154 ++++++++++-------- .../webapi/controller/RouteController.java | 5 +- 5 files changed, 136 insertions(+), 87 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-core/pom.xml b/dhis-2/dhis-services/dhis-service-core/pom.xml index 478976e7809f..15587d61b598 100644 --- a/dhis-2/dhis-services/dhis-service-core/pom.xml +++ b/dhis-2/dhis-services/dhis-service-core/pom.xml @@ -108,6 +108,11 @@ reactor-netty-http 1.2.2 + + org.reactivestreams + reactive-streams + 1.0.4 + io.netty netty-handler diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java index c4582b8b4eed..8a929b7d2fd0 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java @@ -32,7 +32,6 @@ import io.netty.handler.timeout.ReadTimeoutException; import jakarta.servlet.http.HttpServletRequest; import java.io.IOException; -import java.nio.charset.StandardCharsets; import java.time.Duration; import java.util.Arrays; import java.util.Collection; @@ -56,17 +55,23 @@ import org.hisp.dhis.user.UserDetails; import org.jasypt.encryption.pbe.PBEStringCleanablePasswordEncryptor; import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.core.io.InputStreamResource; +import org.springframework.core.io.buffer.DataBuffer; +import org.springframework.core.io.buffer.DataBufferFactory; import org.springframework.core.io.buffer.DataBufferLimitException; +import org.springframework.core.io.buffer.DataBufferUtils; +import org.springframework.core.io.buffer.DefaultDataBufferFactory; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.http.client.reactive.ClientHttpConnector; import org.springframework.stereotype.Service; -import org.springframework.util.StreamUtils; import org.springframework.util.StringUtils; import org.springframework.web.reactive.function.client.WebClient; +import org.springframework.web.servlet.mvc.method.annotation.StreamingResponseBody; import org.springframework.web.util.UriComponentsBuilder; +import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import reactor.netty.http.client.HttpClientRequest; @@ -86,6 +91,10 @@ public class RouteService { private final ClientHttpConnector clientHttpConnector; + private DataBufferFactory dataBufferFactory; + + private WebClient webClient; + private static final Set ALLOWED_REQUEST_HEADERS = Set.of( "accept", @@ -116,11 +125,11 @@ public class RouteService { "cache-control", "last-modified", "etag"); - private WebClient webClient; @PostConstruct public void postConstruct() { webClient = WebClient.builder().clientConnector(clientHttpConnector).build(); + dataBufferFactory = new DefaultDataBufferFactory(); } /** @@ -163,7 +172,7 @@ public Route getRouteWithDecryptedAuth(@Nonnull String id) { * @throws IOException * @throws BadRequestException */ - public ResponseEntity execute( + public ResponseEntity execute( Route route, UserDetails userDetails, Optional subPath, HttpServletRequest request) throws IOException, BadRequestException { @@ -200,12 +209,13 @@ public ResponseEntity execute( uriComponentsBuilder.path(subPath.get()); } - String body = StreamUtils.copyToString(request.getInputStream(), StandardCharsets.UTF_8); - HttpMethod httpMethod = Objects.requireNonNullElse(HttpMethod.valueOf(request.getMethod()), HttpMethod.GET); String targetUri = uriComponentsBuilder.toUriString(); + Flux requestBodyFlux = + DataBufferUtils.read( + new InputStreamResource(request.getInputStream()), dataBufferFactory, 4096); WebClient.RequestHeadersSpec requestHeadersSpec = webClient .method(httpMethod) @@ -218,7 +228,8 @@ public ResponseEntity execute( Duration.ofSeconds(route.getResponseTimeoutSeconds())); } }) - .bodyValue(body); + .body(requestBodyFlux, DataBuffer.class); + for (Map.Entry> header : headers.entrySet()) { requestHeadersSpec = requestHeadersSpec.header(header.getKey(), header.getValue().toArray(new String[0])); @@ -236,9 +247,9 @@ public ResponseEntity execute( .retrieve() .onStatus(httpStatusCode -> true, clientResponse -> Mono.empty()); - ResponseEntity response = + ResponseEntity> responseEntityFlux = responseSpec - .toEntity(String.class) + .toEntityFlux(DataBuffer.class) .onErrorReturn( throwable -> throwable.getCause() instanceof ReadTimeoutException, new ResponseEntity<>(HttpStatus.GATEWAY_TIMEOUT)) @@ -250,21 +261,34 @@ public ResponseEntity execute( jsonObjectBuilder -> jsonObjectBuilder.addString( "message", throwable.getCause().getMessage())); - return Mono.just(new ResponseEntity<>(message.toJson(), HttpStatus.BAD_GATEWAY)); + + return Mono.just( + new ResponseEntity<>( + Flux.just(dataBufferFactory.wrap(message.toJson().getBytes())), + HttpStatus.BAD_GATEWAY)); }) .block(); - HttpHeaders responseHeaders = filterResponseHeaders(response.getHeaders()); - String responseBody = response.getBody(); log.info( "Request '{}' '{}' responded with status '{}' for route '{}' ('{}')", httpMethod, targetUri, - response.getStatusCode(), + responseEntityFlux.getStatusCode(), route.getName(), route.getUid()); - return new ResponseEntity<>(responseBody, responseHeaders, response.getStatusCode()); + StreamingResponseBody streamingResponseBody = + out -> { + if (responseEntityFlux.getBody() != null) { + DataBufferUtils.write(responseEntityFlux.getBody(), out) + .doOnNext(DataBufferUtils.releaseConsumer()) + .blockLast(); + } else { + out.close(); + } + }; + return new ResponseEntity<>( + streamingResponseBody, responseEntityFlux.getHeaders(), responseEntityFlux.getStatusCode()); } /** diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/test/webapi/ControllerIntegrationTestBase.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/test/webapi/ControllerIntegrationTestBase.java index 3a67ac6083f5..6c978cace60e 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/test/webapi/ControllerIntegrationTestBase.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/test/webapi/ControllerIntegrationTestBase.java @@ -248,6 +248,13 @@ public String getHeader(String name) { } } + protected final MvcResult webRequestWithAsyncMvcResult(MockHttpServletRequestBuilder request) { + return exceptionAsFail( + () -> + mvc.perform(MockMvcRequestBuilders.asyncDispatch(webRequestWithMvcResult(request))) + .andReturn()); + } + protected final MvcResult webRequestWithMvcResult(MockHttpServletRequestBuilder request) { return exceptionAsFail(() -> mvc.perform(request.session(session)).andReturn()); } diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/RouteControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/RouteControllerTest.java index 16837329426c..96f036dd11e0 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/RouteControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/RouteControllerTest.java @@ -31,11 +31,12 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockserver.model.HttpRequest.request; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; +import java.io.UnsupportedEncodingException; +import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -43,9 +44,11 @@ import java.util.concurrent.TimeUnit; import org.hisp.dhis.common.auth.ApiHeadersAuthScheme; import org.hisp.dhis.common.auth.ApiQueryParamsAuthScheme; +import org.hisp.dhis.http.HttpMethod; import org.hisp.dhis.http.HttpStatus; import org.hisp.dhis.jsontree.JsonObject; import org.hisp.dhis.jsontree.JsonString; +import org.hisp.dhis.jsontree.JsonValue; import org.hisp.dhis.route.RouteService; import org.hisp.dhis.test.webapi.PostgresControllerIntegrationTestBase; import org.junit.jupiter.api.AfterEach; @@ -61,6 +64,7 @@ import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.MvcResult; import org.springframework.test.web.servlet.client.MockMvcHttpConnector; import org.springframework.test.web.servlet.setup.MockMvcBuilders; import org.springframework.transaction.annotation.Transactional; @@ -98,6 +102,8 @@ public ClientHttpConnector clientHttpConnector() { } String queryString = ((MockHttpServletRequest) request).getQueryString(); + response.setContentType("application/json"); + response .getWriter() .write( @@ -142,31 +148,6 @@ public void afterEach() { routeTargetMockServerClient.reset(); } - @Test - void testRunRouteWhenResponseBodyExceedsLimit() throws JsonProcessingException { - routeTargetMockServerClient - .when(request().withPath("/")) - .respond(org.mockserver.model.HttpResponse.response("{}")); - - Map route = new HashMap<>(); - route.put("name", "route-under-test"); - route.put("url", "https://dhis2.org"); - - HttpResponse postHttpResponse = POST("/routes", jsonMapper.writeValueAsString(route)); - HttpResponse runHttpResponse = - GET( - "/routes/{id}/run", - postHttpResponse.content().get("response.uid").as(JsonString.class).string()); - - String message = - runHttpResponse - .error(HttpStatus.BAD_GATEWAY) - .get("message") - .as(JsonString.class) - .string(); - assertTrue(message.startsWith("Exceeded limit on max bytes to buffer : ")); - } - @Test void testRunRouteWhenResponseDurationExceedsRouteResponseTimeout() throws JsonProcessingException { @@ -181,12 +162,18 @@ void testRunRouteWhenResponseDurationExceedsRouteResponseTimeout() route.put("responseTimeoutSeconds", 5); HttpResponse postHttpResponse = POST("/routes", jsonMapper.writeValueAsString(route)); - HttpResponse runHttpResponse = - GET( - "/routes/{id}/run", - postHttpResponse.content().get("response.uid").as(JsonString.class).string()); - - assertStatus(HttpStatus.GATEWAY_TIMEOUT, runHttpResponse); + MvcResult mvcResult = + webRequestWithAsyncMvcResult( + buildMockRequest( + HttpMethod.GET, + "/routes/" + + postHttpResponse.content().get("response.uid").as(JsonString.class).string() + + "/run", + new ArrayList<>(), + "application/json", + null)); + + assertEquals(504, mvcResult.getResponse().getStatus()); } @Test @@ -202,16 +189,23 @@ void testRunRouteWhenResponseDurationDoesNotExceedRouteResponseTimeout() route.put("responseTimeoutSeconds", 5); HttpResponse postHttpResponse = POST("/routes", jsonMapper.writeValueAsString(route)); - HttpResponse runHttpResponse = - GET( - "/routes/{id}/run", - postHttpResponse.content().get("response.uid").as(JsonString.class).string()); - - assertStatus(HttpStatus.OK, runHttpResponse); + MvcResult mvcResult = + webRequestWithAsyncMvcResult( + buildMockRequest( + HttpMethod.GET, + "/routes/" + + postHttpResponse.content().get("response.uid").as(JsonString.class).string() + + "/run", + new ArrayList<>(), + "application/json", + null)); + + assertEquals(200, mvcResult.getResponse().getStatus()); } @Test - void testRunRouteWhenResponseIsHttpError() throws JsonProcessingException { + void testRunRouteWhenResponseIsHttpError() + throws JsonProcessingException, UnsupportedEncodingException { routeTargetMockServerClient .when(request().withPath("/")) .respond( @@ -224,59 +218,77 @@ void testRunRouteWhenResponseIsHttpError() throws JsonProcessingException { route.put("url", "http://localhost:" + routeTargetMockServerContainer.getFirstMappedPort()); HttpResponse postHttpResponse = POST("/routes", jsonMapper.writeValueAsString(route)); - HttpResponse runHttpResponse = - GET( - "/routes/{id}/run", - postHttpResponse.content().get("response.uid").as(JsonString.class).string()); - - assertStatus(HttpStatus.NOT_FOUND, runHttpResponse); - assertEquals("not found", runHttpResponse.error().getMessage()); + MvcResult mvcResult = + webRequestWithAsyncMvcResult( + buildMockRequest( + HttpMethod.GET, + "/routes/" + + postHttpResponse.content().get("response.uid").as(JsonString.class).string() + + "/run", + new ArrayList<>(), + "application/json", + null)); + + assertEquals(404, mvcResult.getResponse().getStatus()); + JsonObject responseBody = + JsonValue.of(mvcResult.getResponse().getContentAsString()).asObject(); + assertEquals("not found", responseBody.get("message").as(JsonString.class).string()); } } @Test - void testRunRouteGivenApiQueryParamsAuthScheme() throws JsonProcessingException { + void testRunRouteGivenApiQueryParamsAuthScheme() + throws JsonProcessingException, UnsupportedEncodingException { Map route = new HashMap<>(); route.put("name", "route-under-test"); route.put("auth", Map.of("type", "api-query-params", "queryParams", Map.of("token", "foo"))); route.put("url", "http://stub"); HttpResponse postHttpResponse = POST("/routes", jsonMapper.writeValueAsString(route)); - HttpResponse runHttpResponse = - GET( - "/routes/{id}/run", - postHttpResponse.content().get("response.uid").as(JsonString.class).string()); - - assertStatus(HttpStatus.OK, runHttpResponse); - assertEquals("John Doe", runHttpResponse.content().get("name").as(JsonString.class).string()); - assertEquals( - "token=foo", runHttpResponse.content().get("queryString").as(JsonString.class).string()); + MvcResult mvcResult = + webRequestWithAsyncMvcResult( + buildMockRequest( + HttpMethod.GET, + "/routes/" + + postHttpResponse.content().get("response.uid").as(JsonString.class).string() + + "/run", + new ArrayList<>(), + "application/json", + null)); + + assertEquals(200, mvcResult.getResponse().getStatus()); + JsonObject responseBody = JsonValue.of(mvcResult.getResponse().getContentAsString()).asObject(); + + assertEquals("John Doe", responseBody.get("name").as(JsonString.class).string()); + assertEquals("token=foo", responseBody.get("queryString").as(JsonString.class).string()); } @Test - void testRunRouteGivenApiHeadersAuthScheme() throws JsonProcessingException { + void testRunRouteGivenApiHeadersAuthScheme() + throws JsonProcessingException, UnsupportedEncodingException { Map route = new HashMap<>(); route.put("name", "route-under-test"); route.put("auth", Map.of("type", "api-headers", "headers", Map.of("X-API-KEY", "foo"))); route.put("url", "http://stub"); HttpResponse postHttpResponse = POST("/routes", jsonMapper.writeValueAsString(route)); - HttpResponse runHttpResponse = - GET( - "/routes/{id}/run", - postHttpResponse.content().get("response.uid").as(JsonString.class).string()); - - assertStatus(HttpStatus.OK, runHttpResponse); - assertEquals("John Doe", runHttpResponse.content().get("name").as(JsonString.class).string()); + MvcResult mvcResult = + webRequestWithAsyncMvcResult( + buildMockRequest( + HttpMethod.GET, + "/routes/" + + postHttpResponse.content().get("response.uid").as(JsonString.class).string() + + "/run", + new ArrayList<>(), + "application/json", + null)); + + assertEquals(200, mvcResult.getResponse().getStatus()); + JsonObject responseBody = JsonValue.of(mvcResult.getResponse().getContentAsString()).asObject(); + assertEquals("John Doe", responseBody.asObject().get("name").as(JsonString.class).string()); assertEquals( "foo", - runHttpResponse - .content() - .get("headers") - .asObject() - .get("X-API-KEY") - .as(JsonString.class) - .string()); + responseBody.get("headers").asObject().get("X-API-KEY").as(JsonString.class).string()); } @Test diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/RouteController.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/RouteController.java index 664009ddec4e..0d9b230dbd2f 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/RouteController.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/RouteController.java @@ -54,6 +54,7 @@ import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.bind.annotation.ResponseStatus; import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.servlet.mvc.method.annotation.StreamingResponseBody; /** * @author Morten Olav Hansen @@ -75,7 +76,7 @@ public class RouteController extends AbstractCrudController run( + public ResponseEntity run( @PathVariable("id") String id, @CurrentUser UserDetails currentUser, HttpServletRequest request) @@ -92,7 +93,7 @@ public ResponseEntity run( RequestMethod.DELETE, RequestMethod.PATCH }) - public ResponseEntity runWithSubpath( + public ResponseEntity runWithSubpath( @PathVariable("id") String id, @CurrentUser UserDetails currentUser, HttpServletRequest request) From b198d5812282df41efb30c744d1063913da03b3c Mon Sep 17 00:00:00 2001 From: cjmamo <823038+cjmamo@users.noreply.github.com> Date: Tue, 11 Feb 2025 13:12:35 +0100 Subject: [PATCH 14/23] refactor: follow-up PR comments --- .../dhis-api/src/main/java/org/hisp/dhis/route/Route.java | 6 +++--- .../src/main/java/org/hisp/dhis/route/RouteService.java | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/route/Route.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/route/Route.java index cd89d1966ac6..72439043ff5b 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/route/Route.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/route/Route.java @@ -48,7 +48,7 @@ @EqualsAndHashCode(callSuper = true) @Accessors(chain = true) public class Route extends BaseIdentifiableObject implements MetadataObject { - public static final String DEFAULT_RESPONSE_TIMEOUT_SECONDS = "5"; + public static final int DEFAULT_RESPONSE_TIMEOUT_SECONDS = 5; public static final String PATH_WILDCARD_SUFFIX = "/**"; @JsonProperty private String description; @@ -68,8 +68,8 @@ public class Route extends BaseIdentifiableObject implements MetadataObject { /** Optional. Required authorities for invoking the route. */ @JsonProperty private List authorities = new ArrayList<>(); - @JsonProperty(defaultValue = DEFAULT_RESPONSE_TIMEOUT_SECONDS) - private int responseTimeoutSeconds = Integer.parseInt(DEFAULT_RESPONSE_TIMEOUT_SECONDS); + @JsonProperty(defaultValue = ""+DEFAULT_RESPONSE_TIMEOUT_SECONDS) + private int responseTimeoutSeconds = DEFAULT_RESPONSE_TIMEOUT_SECONDS; /** * If the route url ends with /** return true. Otherwise return false. diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java index 8a929b7d2fd0..52a65c68fb29 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java @@ -258,8 +258,8 @@ public ResponseEntity execute( throwable -> { JsonObject message = Json.object( - jsonObjectBuilder -> - jsonObjectBuilder.addString( + obj -> + obj.addString( "message", throwable.getCause().getMessage())); return Mono.just( From 2005c82e24ebfb163f47d265d775624ebc32c582 Mon Sep 17 00:00:00 2001 From: cjmamo <823038+cjmamo@users.noreply.github.com> Date: Tue, 11 Feb 2025 13:15:58 +0100 Subject: [PATCH 15/23] fix: filter response headers --- .../src/main/java/org/hisp/dhis/route/RouteService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java index 52a65c68fb29..fd9cad3ec471 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java @@ -288,7 +288,7 @@ public ResponseEntity execute( } }; return new ResponseEntity<>( - streamingResponseBody, responseEntityFlux.getHeaders(), responseEntityFlux.getStatusCode()); + streamingResponseBody, filterResponseHeaders(responseEntityFlux.getHeaders()), responseEntityFlux.getStatusCode()); } /** From 173a24f2cad479e4f4b2abb63a4c5897064420c6 Mon Sep 17 00:00:00 2001 From: cjmamo <823038+cjmamo@users.noreply.github.com> Date: Tue, 11 Feb 2025 13:16:57 +0100 Subject: [PATCH 16/23] style: reformat --- .../dhis-api/src/main/java/org/hisp/dhis/route/Route.java | 2 +- .../src/main/java/org/hisp/dhis/route/RouteService.java | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/route/Route.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/route/Route.java index 72439043ff5b..26b523529053 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/route/Route.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/route/Route.java @@ -68,7 +68,7 @@ public class Route extends BaseIdentifiableObject implements MetadataObject { /** Optional. Required authorities for invoking the route. */ @JsonProperty private List authorities = new ArrayList<>(); - @JsonProperty(defaultValue = ""+DEFAULT_RESPONSE_TIMEOUT_SECONDS) + @JsonProperty(defaultValue = "" + DEFAULT_RESPONSE_TIMEOUT_SECONDS) private int responseTimeoutSeconds = DEFAULT_RESPONSE_TIMEOUT_SECONDS; /** diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java index fd9cad3ec471..5701cca39bd0 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java @@ -258,9 +258,7 @@ public ResponseEntity execute( throwable -> { JsonObject message = Json.object( - obj -> - obj.addString( - "message", throwable.getCause().getMessage())); + obj -> obj.addString("message", throwable.getCause().getMessage())); return Mono.just( new ResponseEntity<>( @@ -288,7 +286,9 @@ public ResponseEntity execute( } }; return new ResponseEntity<>( - streamingResponseBody, filterResponseHeaders(responseEntityFlux.getHeaders()), responseEntityFlux.getStatusCode()); + streamingResponseBody, + filterResponseHeaders(responseEntityFlux.getHeaders()), + responseEntityFlux.getStatusCode()); } /** From 77888e17efcd308c3fabe8f2569a17042e743433 Mon Sep 17 00:00:00 2001 From: cjmamo <823038+cjmamo@users.noreply.github.com> Date: Tue, 11 Feb 2025 13:26:23 +0100 Subject: [PATCH 17/23] refactor: drop error handing for DataBufferLimitException since it can't happen anymore due to streaming --- .../java/org/hisp/dhis/route/RouteService.java | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java index 5701cca39bd0..c13b6d66efbb 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java @@ -50,15 +50,12 @@ import lombok.extern.slf4j.Slf4j; import org.hibernate.Hibernate; import org.hisp.dhis.feedback.BadRequestException; -import org.hisp.dhis.jsontree.Json; -import org.hisp.dhis.jsontree.JsonObject; import org.hisp.dhis.user.UserDetails; import org.jasypt.encryption.pbe.PBEStringCleanablePasswordEncryptor; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.core.io.InputStreamResource; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.core.io.buffer.DataBufferFactory; -import org.springframework.core.io.buffer.DataBufferLimitException; import org.springframework.core.io.buffer.DataBufferUtils; import org.springframework.core.io.buffer.DefaultDataBufferFactory; import org.springframework.http.HttpHeaders; @@ -253,18 +250,6 @@ public ResponseEntity execute( .onErrorReturn( throwable -> throwable.getCause() instanceof ReadTimeoutException, new ResponseEntity<>(HttpStatus.GATEWAY_TIMEOUT)) - .onErrorResume( - throwable -> throwable.getCause() instanceof DataBufferLimitException, - throwable -> { - JsonObject message = - Json.object( - obj -> obj.addString("message", throwable.getCause().getMessage())); - - return Mono.just( - new ResponseEntity<>( - Flux.just(dataBufferFactory.wrap(message.toJson().getBytes())), - HttpStatus.BAD_GATEWAY)); - }) .block(); log.info( From 2a4b84045d7a00f0353f4e5e3cb152d6187d5782 Mon Sep 17 00:00:00 2001 From: cjmamo <823038+cjmamo@users.noreply.github.com> Date: Tue, 11 Feb 2025 13:33:48 +0100 Subject: [PATCH 18/23] refactor: drop redundant close on output stream --- .../src/main/java/org/hisp/dhis/route/RouteService.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java index c13b6d66efbb..f3edd1a49a28 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java @@ -266,8 +266,6 @@ public ResponseEntity execute( DataBufferUtils.write(responseEntityFlux.getBody(), out) .doOnNext(DataBufferUtils.releaseConsumer()) .blockLast(); - } else { - out.close(); } }; return new ResponseEntity<>( From 062fbbbf7ee11f93e70f62ff32c5aeea8cae1ed0 Mon Sep 17 00:00:00 2001 From: cjmamo <823038+cjmamo@users.noreply.github.com> Date: Tue, 11 Feb 2025 14:27:45 +0100 Subject: [PATCH 19/23] touch --- .../src/main/java/org/hisp/dhis/route/RouteService.java | 1 + 1 file changed, 1 insertion(+) diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java index f3edd1a49a28..ae2cc5ed9338 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java @@ -336,3 +336,4 @@ private HttpHeaders filterHeaders( return headers; } } + From 6990a9953e37433b062d5fb393a16b1f5c6e929f Mon Sep 17 00:00:00 2001 From: cjmamo <823038+cjmamo@users.noreply.github.com> Date: Tue, 11 Feb 2025 14:30:18 +0100 Subject: [PATCH 20/23] style: reformat --- .../src/main/java/org/hisp/dhis/route/RouteService.java | 1 - 1 file changed, 1 deletion(-) diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java index ae2cc5ed9338..f3edd1a49a28 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java @@ -336,4 +336,3 @@ private HttpHeaders filterHeaders( return headers; } } - From f747384737a1d2fa3688063121e9418f48f6db3f Mon Sep 17 00:00:00 2001 From: cjmamo <823038+cjmamo@users.noreply.github.com> Date: Fri, 14 Feb 2025 16:57:05 +0100 Subject: [PATCH 21/23] fix: clean up resources and bump request buffer size --- .../java/org/hisp/dhis/route/RouteService.java | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java index f3edd1a49a28..ffc2224cd023 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java @@ -212,7 +212,8 @@ public ResponseEntity execute( Flux requestBodyFlux = DataBufferUtils.read( - new InputStreamResource(request.getInputStream()), dataBufferFactory, 4096); + new InputStreamResource(request.getInputStream()), dataBufferFactory, 8192) + .doOnNext(DataBufferUtils.releaseConsumer()); WebClient.RequestHeadersSpec requestHeadersSpec = webClient .method(httpMethod) @@ -263,9 +264,15 @@ public ResponseEntity execute( StreamingResponseBody streamingResponseBody = out -> { if (responseEntityFlux.getBody() != null) { - DataBufferUtils.write(responseEntityFlux.getBody(), out) - .doOnNext(DataBufferUtils.releaseConsumer()) - .blockLast(); + try { + Flux dataBufferFlux = + DataBufferUtils.write(responseEntityFlux.getBody(), out) + .doOnNext(DataBufferUtils.releaseConsumer()); + dataBufferFlux.blockLast(); + } catch (Exception e) { + out.close(); + throw e; + } } }; return new ResponseEntity<>( From c5fda78d0c3af5283ab04f1bbf92ccd21b1fc148 Mon Sep 17 00:00:00 2001 From: cjmamo <823038+cjmamo@users.noreply.github.com> Date: Fri, 14 Feb 2025 20:29:47 +0100 Subject: [PATCH 22/23] chore: update dependencies to fix reported security issues --- dhis-2/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dhis-2/pom.xml b/dhis-2/pom.xml index 068fc20fb065..3e8ba482be44 100644 --- a/dhis-2/pom.xml +++ b/dhis-2/pom.xml @@ -103,7 +103,7 @@ 3.5.3 - 6.1.14 + 6.1.17 3.4.1 2.7.18 2.7.4 From d38e91709218f5ff35ac35ab0ab4722e843607c0 Mon Sep 17 00:00:00 2001 From: cjmamo <823038+cjmamo@users.noreply.github.com> Date: Mon, 17 Feb 2025 11:36:20 +0100 Subject: [PATCH 23/23] fix: limit total transfer time to 5 mins --- .../src/main/java/org/hisp/dhis/route/RouteService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java index ffc2224cd023..3d2ec8df8b98 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/route/RouteService.java @@ -268,7 +268,7 @@ public ResponseEntity execute( Flux dataBufferFlux = DataBufferUtils.write(responseEntityFlux.getBody(), out) .doOnNext(DataBufferUtils.releaseConsumer()); - dataBufferFlux.blockLast(); + dataBufferFlux.blockLast(Duration.ofMinutes(5)); } catch (Exception e) { out.close(); throw e;