From 939ee99b3ea25526abc5581b4fda6ddf3ddc677c Mon Sep 17 00:00:00 2001 From: Andreas Schaefer Date: Fri, 15 Jul 2022 14:34:01 -0700 Subject: [PATCH 1/4] SLING-11458 - fixed a regresssion for the original Ticket SLING-9626 The Json Writer that wraps the response writer must not be flushed or closed as this can lead to issues down the chain --- .../org/apache/sling/graphql/core/servlet/GraphQLServlet.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/apache/sling/graphql/core/servlet/GraphQLServlet.java b/src/main/java/org/apache/sling/graphql/core/servlet/GraphQLServlet.java index 9bba5a8..895ca98 100644 --- a/src/main/java/org/apache/sling/graphql/core/servlet/GraphQLServlet.java +++ b/src/main/java/org/apache/sling/graphql/core/servlet/GraphQLServlet.java @@ -327,7 +327,9 @@ private void execute(Resource resource, SlingHttpServletRequest request, SlingHt private void execute(@NotNull String persistedQuery, SlingHttpServletRequest request, SlingHttpServletResponse response) throws IOException { response.setContentType("application/json"); response.setCharacterEncoding("UTF-8"); - try (JsonWriter writer = Json.createWriter(response.getWriter())) { + // The Response Writer cannot be flushed or closed here + try { + JsonWriter writer = Json.createWriter(response.getWriter()); final QueryParser.Result result = QueryParser.fromJSON(persistedQuery); Map executionResult = queryExecutor.execute(result.getQuery(), result.getVariables(), request.getResource(), request.getRequestPathInfo().getSelectors()); From 169b01368d805c609ebde49311d4c3a511c2bc9a Mon Sep 17 00:00:00 2001 From: Andreas Schaefer Date: Fri, 15 Jul 2022 15:01:44 -0700 Subject: [PATCH 2/4] Fixed also non-persistent execute --- .../apache/sling/graphql/core/servlet/GraphQLServlet.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/apache/sling/graphql/core/servlet/GraphQLServlet.java b/src/main/java/org/apache/sling/graphql/core/servlet/GraphQLServlet.java index 895ca98..7243671 100644 --- a/src/main/java/org/apache/sling/graphql/core/servlet/GraphQLServlet.java +++ b/src/main/java/org/apache/sling/graphql/core/servlet/GraphQLServlet.java @@ -315,9 +315,10 @@ private void execute(Resource resource, SlingHttpServletRequest request, SlingHt return; } - try (JsonWriter writer = Json.createWriter(response.getWriter())) { + try { + JsonWriter writer = Json.createWriter(response.getWriter()); Map executionResult = queryExecutor.execute(query, result.getVariables(), resource, - request.getRequestPathInfo().getSelectors()); + request.getRequestPathInfo().getSelectors()); writer.write(Json.createObjectBuilder(executionResult).build().asJsonObject()); } catch(Exception ex) { throw new IOException(ex); From 66c0ab9c85cd15d499b78f8dda2d2a8220e03fc6 Mon Sep 17 00:00:00 2001 From: Andreas Schaefer Date: Mon, 18 Jul 2022 10:25:32 -0700 Subject: [PATCH 3/4] Added a better comment to both times the writer must not be closed --- .../org/apache/sling/graphql/core/servlet/GraphQLServlet.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/apache/sling/graphql/core/servlet/GraphQLServlet.java b/src/main/java/org/apache/sling/graphql/core/servlet/GraphQLServlet.java index 7243671..d40da4e 100644 --- a/src/main/java/org/apache/sling/graphql/core/servlet/GraphQLServlet.java +++ b/src/main/java/org/apache/sling/graphql/core/servlet/GraphQLServlet.java @@ -314,7 +314,7 @@ private void execute(Resource resource, SlingHttpServletRequest request, SlingHt response.sendError(HttpServletResponse.SC_BAD_REQUEST, "Missing request parameter:" + P_QUERY); return; } - + // The Response Writer cannot be flushed or closed here to avoid issues with Redirects etc try { JsonWriter writer = Json.createWriter(response.getWriter()); Map executionResult = queryExecutor.execute(query, result.getVariables(), resource, @@ -328,7 +328,7 @@ private void execute(Resource resource, SlingHttpServletRequest request, SlingHt private void execute(@NotNull String persistedQuery, SlingHttpServletRequest request, SlingHttpServletResponse response) throws IOException { response.setContentType("application/json"); response.setCharacterEncoding("UTF-8"); - // The Response Writer cannot be flushed or closed here + // The Response Writer cannot be flushed or closed here to avoid issues with Redirects etc try { JsonWriter writer = Json.createWriter(response.getWriter()); final QueryParser.Result result = QueryParser.fromJSON(persistedQuery); From a4864bc8bba5fe76a6b50e782d17713383c4cdd3 Mon Sep 17 00:00:00 2001 From: Andreas Schaefer Date: Thu, 21 Jul 2022 11:49:50 -0700 Subject: [PATCH 4/4] Write a simple Print Writer Wrapper to prevent the closing on end of try block --- .../graphql/core/servlet/GraphQLServlet.java | 16 ++++++++++++---- .../graphql/core/servlet/GraphQLServletTest.java | 4 ++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/apache/sling/graphql/core/servlet/GraphQLServlet.java b/src/main/java/org/apache/sling/graphql/core/servlet/GraphQLServlet.java index d40da4e..43adfef 100644 --- a/src/main/java/org/apache/sling/graphql/core/servlet/GraphQLServlet.java +++ b/src/main/java/org/apache/sling/graphql/core/servlet/GraphQLServlet.java @@ -21,6 +21,8 @@ package org.apache.sling.graphql.core.servlet; import java.io.IOException; +import java.io.PrintWriter; +import java.io.Writer; import java.util.Arrays; import java.util.Map; import java.util.regex.Matcher; @@ -315,8 +317,7 @@ private void execute(Resource resource, SlingHttpServletRequest request, SlingHt return; } // The Response Writer cannot be flushed or closed here to avoid issues with Redirects etc - try { - JsonWriter writer = Json.createWriter(response.getWriter()); + try (JsonWriter writer = Json.createWriter(new NoCloseWriterWrapper(response.getWriter()))) { Map executionResult = queryExecutor.execute(query, result.getVariables(), resource, request.getRequestPathInfo().getSelectors()); writer.write(Json.createObjectBuilder(executionResult).build().asJsonObject()); @@ -329,8 +330,7 @@ private void execute(@NotNull String persistedQuery, SlingHttpServletRequest req response.setContentType("application/json"); response.setCharacterEncoding("UTF-8"); // The Response Writer cannot be flushed or closed here to avoid issues with Redirects etc - try { - JsonWriter writer = Json.createWriter(response.getWriter()); + try (JsonWriter writer = Json.createWriter(new NoCloseWriterWrapper(response.getWriter()))) { final QueryParser.Result result = QueryParser.fromJSON(persistedQuery); Map executionResult = queryExecutor.execute(result.getQuery(), result.getVariables(), request.getResource(), request.getRequestPathInfo().getSelectors()); @@ -356,4 +356,12 @@ private String getLocationHeaderValue(@NotNull SlingHttpServletRequest request, } + private class NoCloseWriterWrapper extends PrintWriter { + public NoCloseWriterWrapper(@NotNull Writer out) { + super(out); + } + + @Override + public void close() {} + } } diff --git a/src/test/java/org/apache/sling/graphql/core/servlet/GraphQLServletTest.java b/src/test/java/org/apache/sling/graphql/core/servlet/GraphQLServletTest.java index 1fe9d84..ae276cd 100644 --- a/src/test/java/org/apache/sling/graphql/core/servlet/GraphQLServletTest.java +++ b/src/test/java/org/apache/sling/graphql/core/servlet/GraphQLServletTest.java @@ -54,6 +54,8 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -183,6 +185,7 @@ private void assertPostWithBody(String contentType, int expectedStatus) throws I MockSlingHttpServletResponse response = context.response(); MockSlingHttpServletRequest request = new MockSlingHttpServletRequest(context.bundleContext()); + java.io.PrintWriter writer = spy(response.getWriter()); request.setMethod("POST"); request.setContent(TEST_QUERY.getBytes(StandardCharsets.UTF_8)); @@ -195,6 +198,7 @@ private void assertPostWithBody(String contentType, int expectedStatus) throws I servlet.doPost(request, response); assertEquals(expectedStatus, response.getStatus()); + verify(writer, never()).close(); } @Test