This is an automated email from the ASF dual-hosted git repository.
enorman pushed a commit to branch master
in repository
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-servlets-resolver.git
The following commit(s) were added to refs/heads/master by this push:
new e680d90 SLING-10021 The DefaultErrorHandlerServlet should consider
the 'Accept' header (#14)
e680d90 is described below
commit e680d900b31edd68cf49ed7ec84a21ff76ad5097
Author: Eric Norman <[email protected]>
AuthorDate: Tue Dec 29 19:06:12 2020 -0800
SLING-10021 The DefaultErrorHandlerServlet should consider the 'Accept'
header (#14)
SLING-10021 The DefaultErrorHandlerServlet should consider the 'Accept'
header
---
.gitignore | 1 +
bnd.bnd | 4 +-
pom.xml | 60 +++++-
.../defaults/DefaultErrorHandlerServlet.java | 89 ++++++++-
.../defaults/DefaultErrorHandlerServletTest.java | 212 +++++++++++++++++++++
.../internal/helper/ScriptSelection2Test.java | 2 +-
.../resolver/it/ServletResolverTestSupport.java | 1 +
7 files changed, 365 insertions(+), 4 deletions(-)
diff --git a/.gitignore b/.gitignore
index 0eefb1f..0a3f3d1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -16,3 +16,4 @@ maven-eclipse.xml
.DS_Store
jcr.log
atlassian-ide-plugin.xml
+dependency-reduced-pom.xml
diff --git a/bnd.bnd b/bnd.bnd
index 05a4426..dab71c1 100644
--- a/bnd.bnd
+++ b/bnd.bnd
@@ -1,12 +1,14 @@
Import-Package:\
!org.apache.sling.engine.impl.*,\
+ !org.apache.sling.servlets.post.impl.helper.*,\
*
Provide-Capability:\
osgi.extender;osgi.extender="org.apache.sling.servlets.resolver";version:Version="1.1"
-includeresource:\
-
@org.apache.sling.engine-*.jar!/org/apache/sling/engine/impl/request/SlingRequestPathInfo*
+
@org.apache.sling.engine-*.jar!/org/apache/sling/engine/impl/request/SlingRequestPathInfo*,\
+
@org.apache.sling.servlets.post-*.jar!/org/apache/sling/servlets/post/impl/helper/MediaRangeList*
-plugin:\
org.apache.sling.bnd.plugin.headers.parameters.remove.Plugin;\
diff --git a/pom.xml b/pom.xml
index 60ce509..ce13245 100644
--- a/pom.xml
+++ b/pom.xml
@@ -91,6 +91,10 @@
<pattern>org.apache.sling.engine.impl.request</pattern>
<shadedPattern>org.apache.sling.servlets.resolver.internal.engine</shadedPattern>
</relocation>
+ <relocation>
+
<pattern>org.apache.sling.servlets.post.impl.helper</pattern>
+
<shadedPattern>org.apache.sling.servlets.resolver.internal.helper</shadedPattern>
+ </relocation>
</relocations>
</configuration>
</execution>
@@ -206,6 +210,12 @@
<artifactId>javax.servlet-api</artifactId>
</dependency>
<dependency>
+ <groupId>org.apache.geronimo.specs</groupId>
+ <artifactId>geronimo-json_1.1_spec</artifactId>
+ <version>1.3</version>
+ <scope>provided</scope>
+ </dependency>
+ <dependency>
<groupId>org.apache.sling</groupId>
<artifactId>org.apache.sling.api</artifactId>
<version>2.23.1-SNAPSHOT</version>
@@ -254,6 +264,12 @@
</dependency>
<dependency>
<groupId>org.apache.sling</groupId>
+ <artifactId>org.apache.sling.servlets.post</artifactId>
+ <version>2.3.16</version>
+ <scope>provided</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.sling</groupId>
<artifactId>org.apache.sling.commons.osgi</artifactId>
<version>2.2.0</version>
<scope>provided</scope>
@@ -298,8 +314,26 @@
</dependency>
<dependency>
<groupId>org.apache.sling</groupId>
+ <artifactId>org.apache.sling.jcr.resource</artifactId>
+ <version>2.9.2</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.sling</groupId>
+ <artifactId>org.apache.sling.testing.osgi-mock.core</artifactId>
+ <version>2.4.16</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.sling</groupId>
+ <artifactId>org.apache.sling.testing.sling-mock.core</artifactId>
+ <version>2.5.0</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.sling</groupId>
<artifactId>org.apache.sling.testing.sling-mock.junit4</artifactId>
- <version>2.4.0</version>
+ <version>2.5.0</version>
<scope>test</scope>
</dependency>
<dependency>
@@ -318,6 +352,12 @@
</dependency>
<dependency>
<groupId>org.ops4j.pax.exam</groupId>
+ <artifactId>pax-exam-spi</artifactId>
+ <version>${org.ops4j.pax.exam.version}</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.ops4j.pax.exam</groupId>
<artifactId>pax-exam</artifactId>
<version>${org.ops4j.pax.exam.version}</version>
<scope>test</scope>
@@ -377,6 +417,24 @@
<version>2.2.0</version>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.sling</groupId>
+ <artifactId>org.apache.sling.commons.johnzon</artifactId>
+ <version>1.2.6</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.jackrabbit</groupId>
+ <artifactId>jackrabbit-jcr-commons</artifactId>
+ <version>2.13.1</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.geronimo.specs</groupId>
+ <artifactId>geronimo-atinject_1.0_spec</artifactId>
+ <version>1.0</version>
+ <scope>test</scope>
+ </dependency>
</dependencies>
</project>
diff --git
a/src/main/java/org/apache/sling/servlets/resolver/internal/defaults/DefaultErrorHandlerServlet.java
b/src/main/java/org/apache/sling/servlets/resolver/internal/defaults/DefaultErrorHandlerServlet.java
index 0fe9199..d85b636 100644
---
a/src/main/java/org/apache/sling/servlets/resolver/internal/defaults/DefaultErrorHandlerServlet.java
+++
b/src/main/java/org/apache/sling/servlets/resolver/internal/defaults/DefaultErrorHandlerServlet.java
@@ -20,18 +20,23 @@ package
org.apache.sling.servlets.resolver.internal.defaults;
import java.io.IOException;
import java.io.PrintWriter;
+import java.io.StringWriter;
+import javax.json.Json;
+import javax.json.stream.JsonGenerator;
import javax.servlet.GenericServlet;
import javax.servlet.Servlet;
import javax.servlet.ServletException;
import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.apache.sling.api.SlingConstants;
import org.apache.sling.api.SlingHttpServletRequest;
import org.apache.sling.api.request.RequestProgressTracker;
import org.apache.sling.api.request.ResponseUtil;
+import org.apache.sling.servlets.post.impl.helper.MediaRangeList;
import org.osgi.framework.Constants;
import org.osgi.service.component.annotations.Component;
import org.slf4j.Logger;
@@ -51,6 +56,8 @@ import org.slf4j.LoggerFactory;
"sling.servlet.prefix=-1"
})
public class DefaultErrorHandlerServlet extends GenericServlet {
+ private static final String JSON_CONTENT_TYPE = "application/json";
+ private static final String HTML_CONTENT_TYPE = "text/html";
/** default log */
private final Logger log =
LoggerFactory.getLogger(DefaultErrorHandlerServlet.class);
@@ -73,6 +80,21 @@ public class DefaultErrorHandlerServlet extends
GenericServlet {
statusMessage = statusToString(statusCode);
}
+ //properly consider the 'Accept' header conditions to decide whether
to send json or html back
+ if (req instanceof HttpServletRequest &&
+ JSON_CONTENT_TYPE.equals(new
MediaRangeList((HttpServletRequest)req).prefer(HTML_CONTENT_TYPE,
JSON_CONTENT_TYPE))) {
+ renderJson(req, res, statusMessage, requestUri, servletName,
statusCode);
+ } else {
+ //default to HTML rendering
+ renderHtml(req, res, statusMessage, requestUri, servletName,
statusCode);
+ }
+ }
+
+ /**
+ * Render the error as html
+ */
+ protected void renderHtml(ServletRequest req, ServletResponse res, String
statusMessage, String requestUri,
+ String servletName, int statusCode) throws IOException {
// start the response message
final PrintWriter pw = sendIntro((HttpServletResponse) res, statusCode,
statusMessage, requestUri, servletName);
@@ -108,6 +130,71 @@ public class DefaultErrorHandlerServlet extends
GenericServlet {
}
/**
+ * Render the error as json
+ */
+ protected void renderJson(ServletRequest req, ServletResponse res, String
statusMessage, String requestUri,
+ String servletName, int statusCode) throws IOException {
+ HttpServletResponse response = (HttpServletResponse)res;
+ if (!response.isCommitted()) {
+ response.reset();
+ response.setStatus(statusCode);
+ response.setContentType(JSON_CONTENT_TYPE);
+ response.setCharacterEncoding("UTF-8");
+ } else {
+ // Response already committed: don't change status, but report
+ // the error inline and warn about that
+ log.warn("Response already committed, unable to change status,
output might not be well formed");
+ }
+
+ // send the error as JSON
+ try (JsonGenerator jsonGenerator =
Json.createGenerator(res.getWriter())) {
+ jsonGenerator.writeStartObject();
+ jsonGenerator.write("status", statusCode);
+
+ String msg =
(String)req.getAttribute(SlingConstants.ERROR_MESSAGE);
+ if (msg != null && !msg.isEmpty()) {
+ jsonGenerator.write("message", statusMessage);
+ }
+
+ if (requestUri != null && !requestUri.isEmpty()) {
+ jsonGenerator.write("requestUri", requestUri);
+ }
+
+ if (servletName != null && !servletName.isEmpty()) {
+ jsonGenerator.write("servletName", servletName);
+ }
+
+ String exceptionType =
(String)req.getAttribute(SlingConstants.ERROR_EXCEPTION_TYPE);
+ if (exceptionType != null && !exceptionType.isEmpty()) {
+ jsonGenerator.write("exceptionType", exceptionType);
+ }
+
+ // dump the stack trace
+ if (req.getAttribute(SlingConstants.ERROR_EXCEPTION) instanceof
Throwable) {
+ final Throwable throwable = (Throwable)
req.getAttribute(SlingConstants.ERROR_EXCEPTION);
+ try (StringWriter sw = new StringWriter();
+ PrintWriter pw = new PrintWriter(sw)) {
+ printStackTrace(pw, throwable);
+ jsonGenerator.write("exception", sw.toString());
+ }
+ }
+
+ // dump the request progress tracker
+ if (req instanceof SlingHttpServletRequest) {
+ // dump the request progress tracker
+ final RequestProgressTracker tracker =
((SlingHttpServletRequest)req).getRequestProgressTracker();
+ StringWriter strWriter = new StringWriter();
+ try (PrintWriter progressWriter = new PrintWriter(strWriter)) {
+ tracker.dump(progressWriter);
+ }
+ jsonGenerator.write("requestProgress", strWriter.toString());
+ }
+
+ jsonGenerator.writeEnd();
+ }
+ }
+
+ /**
* Print the stack trace for the root exception if the throwable is a
* {@link ServletException}. If this does not contain an exception,
* the throwable itself is printed.
@@ -154,7 +241,7 @@ public class DefaultErrorHandlerServlet extends
GenericServlet {
response.reset();
response.setStatus(statusCode);
- response.setContentType("text/html");
+ response.setContentType(HTML_CONTENT_TYPE);
response.setCharacterEncoding("UTF-8");
pw = response.getWriter();
diff --git
a/src/test/java/org/apache/sling/servlets/resolver/internal/defaults/DefaultErrorHandlerServletTest.java
b/src/test/java/org/apache/sling/servlets/resolver/internal/defaults/DefaultErrorHandlerServletTest.java
new file mode 100644
index 0000000..ba54f8f
--- /dev/null
+++
b/src/test/java/org/apache/sling/servlets/resolver/internal/defaults/DefaultErrorHandlerServletTest.java
@@ -0,0 +1,212 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.sling.servlets.resolver.internal.defaults;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.Reader;
+import java.io.StringReader;
+import java.io.StringWriter;
+import java.util.Enumeration;
+import java.util.regex.Pattern;
+
+import javax.json.Json;
+import javax.json.JsonObject;
+import javax.json.JsonReader;
+import javax.servlet.ServletContext;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletResponse;
+
+import org.apache.sling.api.SlingConstants;
+import org.apache.sling.commons.testing.sling.MockSlingHttpServletRequest;
+import org.apache.sling.commons.testing.sling.MockSlingHttpServletResponse;
+import org.junit.Test;
+
+/**
+ * SLING-10021 test 'Accept' content-type handling in the default error
handler servlet
+ */
+public class DefaultErrorHandlerServletTest {
+
+ @Test
+ public void testJsonErrorResponse() throws IOException, ServletException {
+ // mock a request that accepts a json response
+ MockSlingHttpServletRequest req = new
MockErrorSlingHttpServletRequest("application/json,*/*;q=0.9");
+ MockSlingHttpServletResponse res = new
MockErrorSlingHttpServletResponse(false);
+
+ DefaultErrorHandlerServlet errorServlet = new
DefaultErrorHandlerServlet();
+ errorServlet.init(new MockServletConfig());
+ errorServlet.service(req, res);
+
+ // verify we got json back
+ assertEquals("application/json", res.getContentType());
+ String responseOutput = res.getOutput().toString();
+
+ // check the json content matches what would be sent from the
DefaultErrorHandlingServlet
+ try (Reader reader = new StringReader(responseOutput);
+ JsonReader jsonReader = Json.createReader(reader)) {
+ JsonObject jsonObj = jsonReader.readObject();
+ assertEquals(500, jsonObj.getInt("status"));
+ assertEquals("/testuri", jsonObj.getString("requestUri"));
+ assertEquals("org.apache.sling.test.ServletName",
jsonObj.getString("servletName"));
+ assertEquals("Test Error Message", jsonObj.getString("message"));
+ assertTrue(jsonObj.getString("exception").contains("Test
Exception"));
+ assertEquals(Exception.class.getName(),
jsonObj.getString("exceptionType"));
+ }
+
+ }
+
+ @Test
+ public void testHtmlErrorResponse() throws IOException, ServletException {
+ // mock a request that accepts an html response
+ MockSlingHttpServletRequest req = new
MockErrorSlingHttpServletRequest("text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8");
+ MockSlingHttpServletResponse res = new
MockErrorSlingHttpServletResponse(false);
+
+ DefaultErrorHandlerServlet errorServlet = new
DefaultErrorHandlerServlet();
+ errorServlet.init(new MockServletConfig());
+ errorServlet.service(req, res);
+
+ // verify we got json back
+ assertEquals("text/html", res.getContentType());
+ String responseOutput = res.getOutput().toString();
+
+ // check the html content matches what would be sent from the
DefaultErrorHandlingServlet
+ Pattern regex = Pattern.compile("The requested URL \\/testuri resulted
in an error in org.apache.sling.test.ServletName\\.", Pattern.MULTILINE);
+ assertTrue("Expected error message",
regex.matcher(responseOutput).find());
+ assertTrue(responseOutput.contains("Test Exception"));
+ }
+
+ /**
+ * Mock impl to simulate enough of a servlet context to satisfy what is
used
+ * by DefaultErrorHandlerServlet
+ */
+ private static final class MockServletConfig implements
javax.servlet.ServletConfig {
+
+ @Override
+ public String getServletName() {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public ServletContext getServletContext() {
+ return new org.apache.sling.servlethelpers.MockServletContext() {
+ @Override
+ public String getServerInfo() {
+ return "Test Server Info";
+ }
+ };
+ }
+
+ @Override
+ public String getInitParameter(String name) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public Enumeration<String> getInitParameterNames() {
+ throw new UnsupportedOperationException();
+ }
+
+ }
+
+ /**
+ * Mock impl to simulate an error response
+ */
+ private static final class MockErrorSlingHttpServletResponse extends
MockSlingHttpServletResponse {
+
+ private PrintWriter writer;
+ private StringWriter strWriter;
+ private boolean committed;
+
+ public MockErrorSlingHttpServletResponse(boolean committed) {
+ super();
+ this.committed = committed;
+ }
+
+ @Override
+ public boolean isCommitted() {
+ return this.committed;
+ }
+
+ @Override
+ public void reset() {
+ //no-op
+ }
+
+ @Override
+ public PrintWriter getWriter() throws IOException {
+ // the super MockWriter throws away the buffer during
+ // the flush() calls, so we can't use it to verify
+ // content in the response.
+ if (this.writer == null) {
+ strWriter = new StringWriter();
+ this.strWriter.getBuffer();
+ this.writer = new PrintWriter(strWriter);
+ }
+ return this.writer;
+ }
+
+ @Override
+ public StringBuffer getOutput() {
+ return strWriter.getBuffer();
+ }
+
+ }
+
+ /**
+ * Mock impl to simulate an error request
+ */
+ private static final class MockErrorSlingHttpServletRequest extends
MockSlingHttpServletRequest {
+ private String accept;
+
+ private MockErrorSlingHttpServletRequest(String accept) {
+ super(null, null, null, null, null);
+ this.accept = accept;
+ }
+
+ @Override
+ public String getHeader(String name) {
+ if ("Accept".equals(name)) {
+ return accept;
+ }
+ return super.getHeader(name);
+ }
+
+ @Override
+ public Object getAttribute(String name) {
+ if (SlingConstants.ERROR_STATUS.equals(name)) {
+ return HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
+ } else if (SlingConstants.ERROR_MESSAGE.equals(name)) {
+ return "Test Error Message";
+ } else if (SlingConstants.ERROR_REQUEST_URI.equals(name)) {
+ return "/testuri";
+ } else if (SlingConstants.ERROR_SERVLET_NAME.equals(name)) {
+ return "org.apache.sling.test.ServletName";
+ } else if (SlingConstants.ERROR_EXCEPTION.equals(name)) {
+ return new Exception("Test Exception");
+ } else if (SlingConstants.ERROR_EXCEPTION_TYPE.equals(name)) {
+ return Exception.class.getName();
+ }
+ return super.getAttribute(name);
+ }
+
+ }
+
+
+}
diff --git
a/src/test/java/org/apache/sling/servlets/resolver/internal/helper/ScriptSelection2Test.java
b/src/test/java/org/apache/sling/servlets/resolver/internal/helper/ScriptSelection2Test.java
index f66a4c8..81a7f23 100644
---
a/src/test/java/org/apache/sling/servlets/resolver/internal/helper/ScriptSelection2Test.java
+++
b/src/test/java/org/apache/sling/servlets/resolver/internal/helper/ScriptSelection2Test.java
@@ -30,7 +30,7 @@ import org.apache.sling.api.SlingHttpServletRequest;
import org.apache.sling.api.resource.PersistenceException;
import org.apache.sling.api.resource.Resource;
import org.apache.sling.api.resource.ResourceResolver;
-import org.apache.sling.jcr.resource.JcrResourceConstants;
+import org.apache.sling.jcr.resource.api.JcrResourceConstants;
import org.apache.sling.testing.mock.sling.junit.SlingContext;
import org.apache.sling.testing.mock.sling.servlet.MockRequestPathInfo;
import org.apache.sling.testing.mock.sling.servlet.MockSlingHttpServletRequest;
diff --git
a/src/test/java/org/apache/sling/servlets/resolver/it/ServletResolverTestSupport.java
b/src/test/java/org/apache/sling/servlets/resolver/it/ServletResolverTestSupport.java
index 046ca84..f2d20da 100644
---
a/src/test/java/org/apache/sling/servlets/resolver/it/ServletResolverTestSupport.java
+++
b/src/test/java/org/apache/sling/servlets/resolver/it/ServletResolverTestSupport.java
@@ -78,6 +78,7 @@ public class ServletResolverTestSupport extends TestSupport {
versionResolver.setVersionFromProject("org.apache.sling",
"org.apache.sling.resourceresolver");
versionResolver.setVersionFromProject("org.apache.sling",
"org.apache.sling.scripting.api");
versionResolver.setVersionFromProject("org.apache.sling",
"org.apache.sling.scripting.core");
+ versionResolver.setVersionFromProject("org.apache.sling",
"org.apache.sling.commons.johnzon");
return options(
composite(
when(vmOpt != null).useOptions(
