From 273e25bf14948e393eb908f450ba436948d334bf Mon Sep 17 00:00:00 2001 From: Pedro Igor Date: Tue, 22 Feb 2022 09:24:14 -0300 Subject: [PATCH] Fixing responses when unexpected errors occurs Closes #10338 --- .../integration/web/QuarkusRequestFilter.java | 22 +++++++--- .../testsuite/util/ContainerAssume.java | 5 +++ .../admin/authentication/FlowTest.java | 44 +++++++++++++++++++ 3 files changed, 64 insertions(+), 7 deletions(-) diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/integration/web/QuarkusRequestFilter.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/integration/web/QuarkusRequestFilter.java index 724996b7e8a9..ce824a86fd0a 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/integration/web/QuarkusRequestFilter.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/integration/web/QuarkusRequestFilter.java @@ -31,7 +31,9 @@ import io.vertx.core.AsyncResult; import io.vertx.core.Handler; import io.vertx.core.Promise; +import io.vertx.core.buffer.Buffer; import io.vertx.core.http.HttpServerRequest; +import io.vertx.core.http.HttpServerResponse; import io.vertx.ext.web.RoutingContext; /** @@ -60,7 +62,7 @@ private Handler> createBlockingHandler(RoutingContext context) { KeycloakSession session = sessionFactory.create(); configureContextualData(context, createClientConnection(context.request()), session); - configureEndHandler(context, promise, session); + configureEndHandler(context, session); KeycloakTransactionManager tx = session.getTransactionManager(); @@ -87,20 +89,26 @@ private Handler> createBlockingHandler(RoutingContext context) { * Creates a handler to close the {@link KeycloakSession} before the response is written to response but after Resteasy * is done with processing its output. */ - private void configureEndHandler(RoutingContext context, Promise promise, KeycloakSession session) { + private void configureEndHandler(RoutingContext context, KeycloakSession session) { context.addHeadersEndHandler(event -> { try { close(session); } catch (Throwable cause) { - promise.fail(cause); - context.response().headers().clear(); - context.response().putHeader(HttpHeaderNames.CONTENT_TYPE, HttpHeaderValues.TEXT_PLAIN); - context.response().putHeader(HttpHeaderNames.CONTENT_LENGTH, "0"); - context.response().setStatusCode(HttpResponseStatus.INTERNAL_SERVER_ERROR.code()); + unexpectedErrorResponse(context.response()); } }); } + private void unexpectedErrorResponse(HttpServerResponse response) { + response.headers().clear(); + response.putHeader(HttpHeaderNames.CONTENT_TYPE, HttpHeaderValues.TEXT_PLAIN); + response.putHeader(HttpHeaderNames.CONTENT_LENGTH, "0"); + response.setStatusCode(HttpResponseStatus.INTERNAL_SERVER_ERROR.code()); + response.putHeader(HttpHeaderNames.CONNECTION, HttpHeaderValues.CLOSE); + // writes an empty buffer to replace any data previously written + response.write(Buffer.buffer("")); + } + private void configureContextualData(RoutingContext context, ClientConnection connection, KeycloakSession session) { Resteasy.pushContext(ClientConnection.class, connection); Resteasy.pushContext(KeycloakSession.class, session); diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/ContainerAssume.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/ContainerAssume.java index 566c6fc4cde0..599440ae527c 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/ContainerAssume.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/ContainerAssume.java @@ -65,4 +65,9 @@ public static void assumeNotAuthServerQuarkus() { Assume.assumeFalse("Doesn't work on auth-server-quarkus", AuthServerTestEnricher.AUTH_SERVER_CONTAINER.equals("auth-server-quarkus")); } + + public static void assumeAuthServerQuarkus() { + Assume.assumeTrue("Only works on auth-server-quarkus", + AuthServerTestEnricher.AUTH_SERVER_CONTAINER.equals("auth-server-quarkus")); + } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/authentication/FlowTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/authentication/FlowTest.java index 760c7836fe11..b2a59e2a6390 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/authentication/FlowTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/authentication/FlowTest.java @@ -18,24 +18,35 @@ package org.keycloak.testsuite.admin.authentication; import org.junit.Assert; +import org.junit.Assume; import org.junit.Test; import org.keycloak.events.admin.OperationType; import org.keycloak.events.admin.ResourceType; import org.keycloak.representations.idm.AuthenticationExecutionExportRepresentation; import org.keycloak.representations.idm.AuthenticationExecutionInfoRepresentation; import org.keycloak.representations.idm.AuthenticationFlowRepresentation; +import org.keycloak.representations.idm.ComponentRepresentation; import org.keycloak.testsuite.util.AdminEventPaths; +import org.keycloak.testsuite.util.ContainerAssume; import javax.ws.rs.BadRequestException; import javax.ws.rs.ClientErrorException; +import javax.ws.rs.InternalServerErrorException; import javax.ws.rs.NotFoundException; import javax.ws.rs.core.Response; import javax.ws.rs.core.Response.Status; +import java.io.ByteArrayInputStream; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.function.Predicate; import static org.hamcrest.Matchers.containsString; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.fail; import static org.keycloak.testsuite.util.Matchers.body; import static org.keycloak.testsuite.util.Matchers.statusCodeIs; @@ -428,4 +439,37 @@ public void editExecutionFlowTest() { Assert.assertEquals("This is another child flow3", executionReps.get(0).getDescription()); } + @Test + public void failWithLongDescription() { + ContainerAssume.assumeAuthServerQuarkus(); + AuthenticationFlowRepresentation rep = authMgmtResource.getFlows().stream() + .filter(new Predicate() { + @Override + public boolean test(AuthenticationFlowRepresentation rep) { + return "docker auth".equals(rep.getAlias()); + } + }).findAny().orElse(null); + + assertNotNull(rep); + + StringBuilder name = new StringBuilder(); + + while (name.length() < 300) { + name.append("invalid"); + } + + rep.setDescription(name.toString()); + + try { + authMgmtResource.updateFlow(rep.getId(), rep); + } catch (InternalServerErrorException isee) { + try (Response response = isee.getResponse()) { + assertEquals(500, response.getStatus()); + assertEquals(0, response.getLength()); + assertEquals(0, ByteArrayInputStream.class.cast(response.getEntity()).available()); + } + } catch (Exception e) { + fail("Unexpected exception"); + } + } }