Skip to content

Commit 369ea1d

Browse files
committed
Make tests for 413/431 responses more robust
Use Awaitility to make sure the expected response code is received within a reasonable timeout (1 min).
1 parent ac65e0a commit 369ea1d

File tree

1 file changed

+50
-38
lines changed

1 file changed

+50
-38
lines changed

integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisApplicationIntegrationTest.java

Lines changed: 50 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -22,17 +22,19 @@
2222
import static org.apache.polaris.service.it.env.PolarisClient.polarisClient;
2323
import static org.assertj.core.api.Assertions.assertThat;
2424
import static org.assertj.core.api.Assertions.assertThatThrownBy;
25+
import static org.testcontainers.shaded.org.awaitility.Awaitility.await;
2526

2627
import com.auth0.jwt.JWT;
2728
import com.auth0.jwt.algorithms.Algorithm;
28-
import jakarta.ws.rs.ProcessingException;
2929
import jakarta.ws.rs.client.Entity;
3030
import jakarta.ws.rs.client.Invocation;
3131
import jakarta.ws.rs.core.Response;
3232
import java.io.IOException;
3333
import java.nio.file.Files;
3434
import java.nio.file.Path;
35+
import java.time.Duration;
3536
import java.time.Instant;
37+
import java.time.temporal.ChronoUnit;
3638
import java.util.List;
3739
import java.util.Map;
3840
import org.apache.commons.io.FileUtils;
@@ -594,47 +596,57 @@ public void testWarehouseNotSpecified() throws IOException {
594596
}
595597

596598
@Test
597-
public void testRequestHeaderTooLarge() {
598-
Invocation.Builder request = managementApi.request("v1/principal-roles");
599-
600-
// The default limit is 8KiB and each of these headers is at least 8 bytes, so 1500 definitely
601-
// exceeds the limit
602-
for (int i = 0; i < 1500; i++) {
603-
request = request.header("header" + i, "" + i);
604-
}
605-
606-
try {
607-
try (Response response = request.post(Entity.json(new PrincipalRole("r")))) {
608-
assertThat(response)
609-
.returns(
610-
Response.Status.REQUEST_HEADER_FIELDS_TOO_LARGE.getStatusCode(),
611-
Response::getStatus);
612-
}
613-
} catch (ProcessingException e) {
614-
// In some runtime environments the request above will return a 431 but in others it'll result
615-
// in a ProcessingException from the socket being closed. The test asserts that one of those
616-
// things happens.
599+
public void testRequestHeaderTooLarge() throws Exception {
600+
// Use a dedicated client with retries due to non-deterministic behaviour of reading the
601+
// response code and possible abrupt connection resets
602+
try (PolarisClient localClient = polarisClient(endpoints)) {
603+
await()
604+
.atMost(Duration.of(1, ChronoUnit.MINUTES))
605+
.untilAsserted(
606+
() -> {
607+
Invocation.Builder request =
608+
localClient.managementApi(clientCredentials).request("v1/principal-roles");
609+
// The default limit is 8KiB and each of these headers is at least 8 bytes, so 1500
610+
// definitely exceeds the limit
611+
for (int i = 0; i < 1500; i++) {
612+
request = request.header("header" + i, "" + i);
613+
}
614+
615+
try (Response response = request.post(Entity.json(new PrincipalRole("r")))) {
616+
assertThat(response.getStatus())
617+
.isEqualTo(Response.Status.REQUEST_HEADER_FIELDS_TOO_LARGE.getStatusCode());
618+
}
619+
});
617620
}
618621
}
619622

620623
@Test
621-
public void testRequestBodyTooLarge() {
622-
// The behaviour in case of large requests depends on the specific server configuration.
623-
// This test assumes that the server under test is configured to deny requests larger than
624-
// 1000000 bytes. The test payload below assumes UTF8 encoding of ASCII charts plus a bit of
625-
// JSON overhead.
626-
Entity<PrincipalRole> largeRequest = Entity.json(new PrincipalRole("r".repeat(1000001)));
627-
try (Response response = managementApi.request("v1/principal-roles").post(largeRequest)) {
628-
// Note we only validate the status code here because per RFC 9110, the server MAY not provide
629-
// a response body. The HTTP status line is still expected to be provided.
630-
assertThat(response.getStatus())
631-
.isEqualTo(Response.Status.REQUEST_ENTITY_TOO_LARGE.getStatusCode());
632-
} catch (ProcessingException e) {
633-
// Per RFC 9110 servers MAY close the connection in case of 413 responses, which
634-
// might cause the client to fail to read the status code (cf. RFC 9112, section 9.6).
635-
// TODO: servers are expected to close connections gracefully. It might be worth investigating
636-
// whether "connection closed" exceptions are a client-side bug.
637-
assertThat(e).hasMessageContaining("Connection was closed");
624+
public void testRequestBodyTooLarge() throws Exception {
625+
// Use a dedicated client with retries due to non-deterministic behaviour of reading the
626+
// response code and possible abrupt connection resets
627+
try (PolarisClient localClient = polarisClient(endpoints)) {
628+
await()
629+
.atMost(Duration.of(1, ChronoUnit.MINUTES))
630+
.untilAsserted(
631+
() -> {
632+
// The behaviour in case of large requests depends on the specific server
633+
// configuration. This test assumes that the server under test is configured to deny
634+
// requests larger than 1000000 bytes. The test payload below assumes UTF8 encoding
635+
// of ASCII charts plus a bit of JSON overhead.
636+
Entity<PrincipalRole> largeRequest =
637+
Entity.json(new PrincipalRole("r".repeat(1000001)));
638+
try (Response response =
639+
localClient
640+
.managementApi(clientCredentials)
641+
.request("v1/principal-roles")
642+
.post(largeRequest)) {
643+
// Note we only validate the status code here because per RFC 9110, the server MAY
644+
// not provide a response body. The HTTP status line is still expected to be
645+
// provided most of the time.
646+
assertThat(response.getStatus())
647+
.isEqualTo(Response.Status.REQUEST_ENTITY_TOO_LARGE.getStatusCode());
648+
}
649+
});
638650
}
639651
}
640652

0 commit comments

Comments
 (0)