Author: johnh
Date: Thu Jan 21 04:22:30 2010
New Revision: 901523
URL: http://svn.apache.org/viewvc?rev=901523&view=rev
Log:
Patch from Chirag Shah, description:
"""
Shindig's GadgetRenderingServlet should respond with an appropriate 4xx/5xx HTTP
status code when there are errors. It currently responds with 200 OK when
something goes wrong.
"""
This closes SHINDIG-1266.
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/HtmlRenderer.java
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/ProxyRenderer.java
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/Renderer.java
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingException.java
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingResults.java
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetRenderingServlet.java
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/render/RendererTest.java
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetRenderingServletTest.java
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/HtmlRenderer.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/HtmlRenderer.java?rev=901523&r1=901522&r2=901523&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/HtmlRenderer.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/HtmlRenderer.java
Thu Jan 21 04:22:30 2010
@@ -33,6 +33,9 @@
import com.google.inject.Inject;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
/**
* Handles producing output markup for a gadget based on the provided context.
*/
@@ -90,9 +93,9 @@
return mc.getContent();
} catch (GadgetException e) {
- throw new RenderingException(e.getMessage(), e);
+ throw new RenderingException(e.getMessage(), e,
HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
} catch (RewritingException e) {
- throw new RenderingException(e.getMessage(), e);
+ throw new RenderingException(e.getMessage(), e,
HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
}
}
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/ProxyRenderer.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/ProxyRenderer.java?rev=901523&r1=901522&r2=901523&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/ProxyRenderer.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/ProxyRenderer.java
Thu Jan 21 04:22:30 2010
@@ -38,6 +38,8 @@
import com.google.common.collect.ImmutableList;
import com.google.inject.Inject;
+import javax.servlet.http.HttpServletResponse;
+
/**
* Implements proxied rendering.
*/
@@ -106,7 +108,7 @@
if (response.isError()) {
throw new RenderingException("Unable to reach remote host. HTTP status "
+
- response.getHttpStatusCode());
+ response.getHttpStatusCode(), HttpServletResponse.SC_NOT_FOUND);
}
return response.getResponseAsString();
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/Renderer.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/Renderer.java?rev=901523&r1=901522&r2=901523&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/Renderer.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/Renderer.java
Thu Jan 21 04:22:30 2010
@@ -30,6 +30,7 @@
import com.google.inject.Inject;
+import javax.servlet.http.HttpServletResponse;
import java.util.List;
import java.util.logging.Logger;
import java.util.regex.Pattern;
@@ -64,7 +65,8 @@
*/
public RenderingResults render(GadgetContext context) {
if (!validateParent(context)) {
- return RenderingResults.error("Unsupported parent parameter. Check your
container code.");
+ return RenderingResults.error("Unsupported parent parameter. Check your
container code.",
+ HttpServletResponse.SC_BAD_REQUEST);
}
try {
@@ -73,7 +75,7 @@
if (gadget.getCurrentView() == null) {
return RenderingResults.error("Unable to locate an appropriate view in
this gadget. " +
"Requested: '" + gadget.getContext().getView() +
- "' Available: " + gadget.getSpec().getViews().keySet());
+ "' Available: " + gadget.getSpec().getViews().keySet(),
HttpServletResponse.SC_NOT_FOUND);
}
if (gadget.getCurrentView().getType() == View.ContentType.URL) {
@@ -81,25 +83,25 @@
}
if (!lockedDomainService.gadgetCanRender(context.getHost(), gadget,
context.getContainer())) {
- return RenderingResults.error("Invalid domain");
+ return RenderingResults.error("Invalid domain",
HttpServletResponse.SC_BAD_REQUEST);
}
return RenderingResults.ok(renderer.render(gadget));
} catch (RenderingException e) {
- return logError(context.getUrl(), e);
+ return logError(context.getUrl(), e.getHttpStatusCode(), e);
} catch (ProcessingException e) {
- return logError(context.getUrl(), e);
+ return logError(context.getUrl(),
HttpServletResponse.SC_INTERNAL_SERVER_ERROR, e);
} catch (RuntimeException e) {
if (e.getCause() instanceof GadgetException) {
- return logError(context.getUrl(), e.getCause());
+ return logError(context.getUrl(),
HttpServletResponse.SC_INTERNAL_SERVER_ERROR, e.getCause());
}
throw e;
}
}
- private RenderingResults logError(Uri gadgetUrl, Throwable t) {
+ private RenderingResults logError(Uri gadgetUrl, int statusCode, Throwable
t) {
LOG.info("Failed to render gadget " + gadgetUrl + ": " + t.getMessage());
- return RenderingResults.error(t.getMessage());
+ return RenderingResults.error(t.getMessage(), statusCode);
}
/**
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingException.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingException.java?rev=901523&r1=901522&r2=901523&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingException.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingException.java
Thu Jan 21 04:22:30 2010
@@ -18,6 +18,8 @@
*/
package org.apache.shindig.gadgets.render;
+import javax.servlet.http.HttpServletResponse;
+
/**
* Exceptions thrown during gadget rendering.
*
@@ -25,15 +27,24 @@
* be easily localizable.
*/
public class RenderingException extends Exception {
- public RenderingException(Throwable t) {
+ private final int httpStatusCode;
+
+ public RenderingException(Throwable t, int httpStatusCode) {
super(t);
+ this.httpStatusCode = httpStatusCode;
}
- public RenderingException(String message) {
+ public RenderingException(String message, int httpStatusCode) {
super(message);
+ this.httpStatusCode = httpStatusCode;
}
- public RenderingException(String message, Throwable t) {
+ public RenderingException(String message, Throwable t, int httpStatusCode) {
super(message, t);
+ this.httpStatusCode = httpStatusCode;
+ }
+
+ public int getHttpStatusCode() {
+ return httpStatusCode;
}
}
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingResults.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingResults.java?rev=901523&r1=901522&r2=901523&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingResults.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingResults.java
Thu Jan 21 04:22:30 2010
@@ -22,6 +22,8 @@
import com.google.common.base.Preconditions;
+import javax.servlet.http.HttpServletResponse;
+
/**
* Contains the results of a rendering operation.
*/
@@ -29,26 +31,32 @@
private final Status status;
private final String content;
private final String errorMessage;
+ private final int httpStatusCode;
+
private final Uri redirect;
- private RenderingResults(Status status, String content, String errorMessage,
Uri redirect) {
+ private RenderingResults(Status status, String content, String errorMessage,
+ int httpStatusCode, Uri redirect) {
this.status = status;
this.content = content;
this.errorMessage = errorMessage;
+ this.httpStatusCode = httpStatusCode;
+
this.redirect = redirect;
}
public static RenderingResults ok(String content) {
- return new RenderingResults(Status.OK, content, null, null);
+ return new RenderingResults(Status.OK, content, null,
HttpServletResponse.SC_OK, null);
}
- public static RenderingResults error(String errorMessage) {
- return new RenderingResults(Status.ERROR, null, errorMessage, null);
+ public static RenderingResults error(String errorMessage, int
httpStatusCode) {
+ return new RenderingResults(Status.ERROR, null, errorMessage,
httpStatusCode, null);
}
public static RenderingResults mustRedirect(Uri redirect) {
Preconditions.checkNotNull(redirect);
- return new RenderingResults(Status.MUST_REDIRECT, null, null, redirect);
+ return new RenderingResults(Status.MUST_REDIRECT, null, null,
HttpServletResponse.SC_FOUND,
+ redirect);
}
/**
@@ -75,6 +83,14 @@
}
/**
+ * @return The HTTP status code for rendering. Only available when status is
ERROR.
+ */
+ public int getHttpStatusCode() {
+ Preconditions.checkState(status == Status.ERROR, "Only available when
status is ERROR.");
+ return httpStatusCode;
+ }
+
+ /**
* @return The error message for rendering. Only available when status is
ERROR.
*/
public Uri getRedirect() {
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetRenderingServlet.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetRenderingServlet.java?rev=901523&r1=901522&r2=901523&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetRenderingServlet.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetRenderingServlet.java
Thu Jan 21 04:22:30 2010
@@ -89,6 +89,7 @@
resp.getWriter().print(results.getContent());
break;
case ERROR:
+ resp.setStatus(results.getHttpStatusCode());
resp.getWriter().print(StringEscapeUtils.escapeHtml(results.getErrorMessage()));
break;
case MUST_REDIRECT:
Modified:
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/render/RendererTest.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/render/RendererTest.java?rev=901523&r1=901522&r2=901523&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/render/RendererTest.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/render/RendererTest.java
Thu Jan 21 04:22:30 2010
@@ -38,6 +38,7 @@
import org.junit.Before;
import org.junit.Test;
+import javax.servlet.http.HttpServletResponse;
import java.util.Arrays;
import java.util.Map;
@@ -108,10 +109,11 @@
@Test
public void handlesRenderingExceptionGracefully() {
- htmlRenderer.exception = new RenderingException("oh no!");
+ htmlRenderer.exception = new RenderingException("four-oh-four",
HttpServletResponse.SC_NOT_FOUND);
RenderingResults results = renderer.render(makeContext("html"));
assertEquals(RenderingResults.Status.ERROR, results.getStatus());
- assertEquals("oh no!", results.getErrorMessage());
+ assertEquals("four-oh-four", results.getErrorMessage());
+ assertEquals(HttpServletResponse.SC_NOT_FOUND,
results.getHttpStatusCode());
}
@Test
Modified:
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetRenderingServletTest.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetRenderingServletTest.java?rev=901523&r1=901522&r2=901523&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetRenderingServletTest.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetRenderingServletTest.java
Thu Jan 21 04:22:30 2010
@@ -87,28 +87,32 @@
public void errorsPassedThrough() throws Exception {
servlet.setRenderer(renderer);
expect(renderer.render(isA(GadgetContext.class)))
- .andReturn(RenderingResults.error("busted"));
+ .andReturn(RenderingResults.error("busted",
HttpServletResponse.SC_INTERNAL_SERVER_ERROR));
control.replay();
servlet.doGet(request, recorder);
- assertEquals(HttpServletResponse.SC_OK, recorder.getHttpStatusCode());
+ assertEquals(HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
recorder.getHttpStatusCode());
assertNull("Cache-Control header passed where it should not be.",
recorder.getHeader("Cache-Control"));
assertEquals("busted", recorder.getResponseAsString());
+
}
@Test
public void errorsAreEscaped() throws Exception {
servlet.setRenderer(renderer);
expect(renderer.render(isA(GadgetContext.class)))
-
.andReturn(RenderingResults.error("busted<script>alert(document.domain)</script>"));
+
.andReturn(RenderingResults.error("busted<script>alert(document.domain)</script>",
+ HttpServletResponse.SC_INTERNAL_SERVER_ERROR));
control.replay();
servlet.doGet(request, recorder);
assertEquals("busted<script>alert(document.domain)</script>",
recorder.getResponseAsString());
+
+ assertEquals(HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
recorder.getHttpStatusCode());
}
@Test
