From a9668d14ce043c8b2b83e85d257bc905b3797e46 Mon Sep 17 00:00:00 2001
From: Pedro Igor <pigor.craveiro@gmail.com>
Date: Mon, 14 Feb 2022 19:39:13 -0300
Subject: [PATCH] Proper error response when handing unexpected errors

Closes #10176
---
 .../integration/web/QuarkusRequestFilter.java | 25 ++++++++++++-----
 .../testsuite/admin/ComponentsTest.java       | 28 +++++++++++++++++++
 2 files changed, 46 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 44d8f3351d9..724996b7e8a 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
@@ -25,6 +25,9 @@ import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.KeycloakSessionFactory;
 import org.keycloak.models.KeycloakTransactionManager;
 
+import io.netty.handler.codec.http.HttpHeaderNames;
+import io.netty.handler.codec.http.HttpHeaderValues;
+import io.netty.handler.codec.http.HttpResponseStatus;
 import io.vertx.core.AsyncResult;
 import io.vertx.core.Handler;
 import io.vertx.core.Promise;
@@ -64,7 +67,7 @@ public class QuarkusRequestFilter implements Handler<RoutingContext> {
             try {
                 tx.begin();
                 context.next();
-                promise.complete();
+                promise.tryComplete();
             } catch (Throwable cause) {
                 promise.fail(cause);
                 // re-throw so that the any exception is handled from parent
@@ -90,6 +93,10 @@ public class QuarkusRequestFilter implements Handler<RoutingContext> {
                 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());
             }
         });
     }
@@ -105,14 +112,18 @@ public class QuarkusRequestFilter implements Handler<RoutingContext> {
 
     protected void close(KeycloakSession session) {
         KeycloakTransactionManager tx = session.getTransactionManager();
-        if (tx.isActive()) {
-            if (tx.getRollbackOnly()) {
-                tx.rollback();
-            } else {
-                tx.commit();
+
+        try {
+            if (tx.isActive()) {
+                if (tx.getRollbackOnly()) {
+                    tx.rollback();
+                } else {
+                    tx.commit();
+                }
             }
+        } finally {
+            session.close();
         }
-        session.close();
     }
 
     private ClientConnection createClientConnection(HttpServerRequest request) {
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/ComponentsTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/ComponentsTest.java
index aa3a4ff1468..db8698a6711 100644
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/ComponentsTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/ComponentsTest.java
@@ -172,6 +172,34 @@ public class ComponentsTest extends AbstractAdminTest {
         assertEquals("fixed-id", id);
     }
 
+    @Test
+    public void failCreateWithLongName() {
+        StringBuilder name = new StringBuilder();
+
+        while (name.length() < 30) {
+            name.append("invalid");
+        }
+
+        ComponentRepresentation rep = createComponentRepresentation(name.toString());
+
+        rep.getConfig().addFirst("required", "foo");
+
+        ComponentsResource components = realm.components();
+
+        try (Response response = components.add(rep)) {
+            if (Response.Status.INTERNAL_SERVER_ERROR.getStatusCode() == response.getStatus()) {
+                // using database should fail due to constraint violations
+                assertFalse(components.query().stream().map(ComponentRepresentation::getName).anyMatch(name.toString()::equals));
+            } else if (Response.Status.CREATED.getStatusCode() == response.getStatus()) {
+                // using the map storage should work because there are no constraints
+                String id = ApiUtil.getCreatedId(response);
+                assertNotNull(components.component(id).toRepresentation());
+            } else {
+                fail("Unexpected response");
+            }
+        }
+    }
+
     @Test
     public void testUpdate() {
         ComponentRepresentation rep = createComponentRepresentation("mycomponent");