Author: johnh
Date: Mon Jul 26 22:12:54 2010
New Revision: 979468
URL: http://svn.apache.org/viewvc?rev=979468&view=rev
Log:
For AccelHandler/HtmlAccelServlet, return the same HTTP status code as that of
the fetched resource. Emit the same error text as well in this case.
This is done w/ one caveat, to convert HTTP 500 to HTTP 502, done on a
configurable basis (default=true). This is done to prevent common monitoring
software from seeing Accel itself as faulty (status 500), vs. the server it's
proxying.
Patch provided by Gagan Singh.
Modified:
shindig/trunk/java/common/conf/shindig.properties
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/AccelHandler.java
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/UriUtils.java
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/HtmlAccelServletTest.java
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/UriUtilsTest.java
Modified: shindig/trunk/java/common/conf/shindig.properties
URL:
http://svn.apache.org/viewvc/shindig/trunk/java/common/conf/shindig.properties?rev=979468&r1=979467&r2=979468&view=diff
==============================================================================
--- shindig/trunk/java/common/conf/shindig.properties (original)
+++ shindig/trunk/java/common/conf/shindig.properties Mon Jul 26 22:12:54 2010
@@ -141,3 +141,8 @@ org.apache.shindig.serviceExpirationDura
# both - return both fields for full compatibility
#
shindig.json-rpc.result-field=result
+
+# Remap "Internal server error"s received from the basicHttpFetcherProxy
server to
+# "Bad Gateway error"s, so that it is clear to the user that the proxy server
is
+# the one that threw the exception.
+shindig.accelerate.remapInternalServerError=true
Modified:
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/AccelHandler.java
URL:
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/AccelHandler.java?rev=979468&r1=979467&r2=979468&view=diff
==============================================================================
---
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/AccelHandler.java
(original)
+++
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/AccelHandler.java
Mon Jul 26 22:12:54 2010
@@ -28,6 +28,7 @@ import org.apache.shindig.gadgets.Gadget
import org.apache.shindig.gadgets.GadgetException;
import org.apache.shindig.gadgets.http.HttpRequest;
import org.apache.shindig.gadgets.http.HttpResponse;
+import org.apache.shindig.gadgets.http.HttpResponseBuilder;
import org.apache.shindig.gadgets.http.RequestPipeline;
import org.apache.shindig.gadgets.rewrite.DomWalker;
import org.apache.shindig.gadgets.rewrite.ResponseRewriterRegistry;
@@ -59,15 +60,19 @@ public class AccelHandler extends ProxyB
protected final RequestPipeline requestPipeline;
protected final ResponseRewriterRegistry contentRewriterRegistry;
protected final AccelUriManager uriManager;
+ protected final boolean remapInternalServerError;
@Inject
public AccelHandler(RequestPipeline requestPipeline,
@Named("shindig.accelerate.response.rewriter.registry")
ResponseRewriterRegistry contentRewriterRegistry,
- AccelUriManager accelUriManager) {
+ AccelUriManager accelUriManager,
+ @Named("shindig.accelerate.remapInternalServerError")
+ Boolean remapInternalServerError) {
this.requestPipeline = requestPipeline;
this.contentRewriterRegistry = contentRewriterRegistry;
this.uriManager = accelUriManager;
+ this.remapInternalServerError = remapInternalServerError;
}
@Override
@@ -83,26 +88,25 @@ public class AccelHandler extends ProxyB
HttpRequest req = buildHttpRequest(request, proxyUri);
HttpResponse results = requestPipeline.execute(req);
- if (!handleErrors(results, response)) {
- // In case of errors where we want to short circuit the rewriting and
- // throw appropriate gadget exception.
- return;
- }
-
- // Rewrite the content.
- try {
- results = contentRewriterRegistry.rewriteHttpResponse(req, results);
- } catch (RewritingException e) {
- if (!isRecoverable(req, results, e)) {
- throw new GadgetException(GadgetException.Code.INTERNAL_SERVER_ERROR,
e,
- e.getHttpStatusCode());
+ HttpResponse errorResponse = handleErrors(results);
+ if (errorResponse == null) {
+ // No error. Lets rewrite the content.
+ try {
+ results = contentRewriterRegistry.rewriteHttpResponse(req, results);
+ } catch (RewritingException e) {
+ if (!isRecoverable(req, results, e)) {
+ throw new
GadgetException(GadgetException.Code.INTERNAL_SERVER_ERROR, e,
+ e.getHttpStatusCode());
+ }
}
+ } else {
+ results = errorResponse;
}
// Copy the response headers and status code to the final http servlet
// response.
UriUtils.copyResponseHeadersAndStatusCode(
- results, response, true,
+ results, response, remapInternalServerError, true,
UriUtils.DisallowedHeaders.OUTPUT_TRANSFER_DIRECTIVES,
UriUtils.DisallowedHeaders.CLIENT_STATE_DIRECTIVES);
@@ -110,6 +114,7 @@ public class AccelHandler extends ProxyB
// had the rewrite mime type header.
rewriteContentType(req, response);
+ // Copy the content.
IOUtils.copy(results.getResponse(), response.getOutputStream());
}
@@ -210,27 +215,23 @@ public class AccelHandler extends ProxyB
}
/**
- * Process errors when fetching uri using request pipeline by throwing
- * GadgetException in error cases.
+ * Process errors when fetching uri using request pipeline and return the
+ * error response to be returned to the user if any.
* @param results The http response returned by request pipeline.
- * @param response The http servlet response to be returned to the user.
- * @return True if there is no error, false otherwise.
- * @throws IOException In case of errors.
+ * @return An HttpResponse instance encapsulating error message and status
+ * code to be returned to the user in case of errors, null otherwise.
*/
- protected boolean handleErrors(HttpResponse results, HttpServletResponse
response)
- throws IOException {
+ protected HttpResponse handleErrors(HttpResponse results) {
if (results == null) {
- response.sendError(HttpServletResponse.SC_BAD_REQUEST,
ERROR_FETCHING_DATA);
- return false;
+ return new HttpResponseBuilder()
+ .setHttpStatusCode(HttpServletResponse.SC_NOT_FOUND)
+ .setResponse(ERROR_FETCHING_DATA.getBytes())
+ .create();
}
- if (results.getHttpStatusCode() == HttpServletResponse.SC_NOT_FOUND) {
- response.sendError(HttpServletResponse.SC_NOT_FOUND,
ERROR_FETCHING_DATA);
- return false;
- } else if (results.isError()) {
- response.sendError(HttpServletResponse.SC_BAD_GATEWAY,
ERROR_FETCHING_DATA);
- return false;
+ if (results.isError()) {
+ return results;
}
- return true;
+ return null;
}
}
Modified:
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/UriUtils.java
URL:
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/UriUtils.java?rev=979468&r1=979467&r2=979468&view=diff
==============================================================================
---
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/UriUtils.java
(original)
+++
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/UriUtils.java
Mon Jul 26 22:12:54 2010
@@ -108,6 +108,8 @@ public class UriUtils {
* response.
* @param data The http response when fetching the requested accel uri.
* @param resp The servlet response to return back to client.
+ * @param remapInternalServerError If true, then SC_INTERNAL_SERVER_ERROR is
+ * remapped to SC_BAD_GATEWAY.
* @param setHeaders If true, then setHeader method of HttpServletResponse is
* called, otherwise addHeader is called for every header.
* @param disallowedResponseHeaders Disallowed response headers to omit from
the response
@@ -116,6 +118,7 @@ public class UriUtils {
*/
public static void copyResponseHeadersAndStatusCode(
HttpResponse data, HttpServletResponse resp,
+ boolean remapInternalServerError,
boolean setHeaders,
DisallowedHeaders... disallowedResponseHeaders)
throws IOException {
@@ -138,9 +141,11 @@ public class UriUtils {
}
}
- // External "internal error" should be mapped to gateway error.
- if (data.getHttpStatusCode() == HttpResponse.SC_INTERNAL_SERVER_ERROR) {
- resp.sendError(HttpResponse.SC_BAD_GATEWAY);
+ if (remapInternalServerError) {
+ // External "internal error" should be mapped to gateway error.
+ if (data.getHttpStatusCode() == HttpResponse.SC_INTERNAL_SERVER_ERROR) {
+ resp.setStatus(HttpResponse.SC_BAD_GATEWAY);
+ }
}
}
Modified:
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/HtmlAccelServletTest.java
URL:
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/HtmlAccelServletTest.java?rev=979468&r1=979467&r2=979468&view=diff
==============================================================================
---
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/HtmlAccelServletTest.java
(original)
+++
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/HtmlAccelServletTest.java
Mon Jul 26 22:12:54 2010
@@ -90,7 +90,7 @@ public class HtmlAccelServletTest extend
Arrays.<ResponseRewriter>asList(rewriter), null);
servlet = new HtmlAccelServlet();
servlet.setHandler(new AccelHandler(pipeline, rewriterRegistry,
- accelUriManager));
+ accelUriManager, true));
}
private void expectRequest(String extraPath, String url) {
@@ -149,19 +149,21 @@ public class HtmlAccelServletTest extend
servlet.doGet(request, recorder);
verify();
- assertEquals("Error fetching data", recorder.getResponseAsString());
- assertEquals(400, recorder.getHttpStatusCode());
+ assertEquals(AccelHandler.ERROR_FETCHING_DATA,
+ recorder.getResponseAsString());
+ assertEquals(404, recorder.getHttpStatusCode());
}
@Test
- public void testHtmlAccelNoHtml() throws Exception {
- String url = "http://example.org/data.xml";;
+ public void testHtmlAccelRewriteSimple() throws Exception {
+ String url = "http://example.org/data.html";;
String data = "<html><body>Hello World</body></html>";
+ ((FakeCaptureRewriter) rewriter).setContentToRewrite(REWRITE_CONTENT);
HttpRequest req = new HttpRequest(Uri.parse(url));
HttpResponse resp = new HttpResponseBuilder()
.setResponse(data.getBytes())
- .setHeader("Content-Type", "text/xml")
+ .setHeader("Content-Type", "text/html")
.setHttpStatusCode(200)
.create();
expect(pipeline.execute(req)).andReturn(resp).once();
@@ -170,20 +172,24 @@ public class HtmlAccelServletTest extend
servlet.doGet(request, recorder);
verify();
- assertEquals(data, recorder.getResponseAsString());
+ assertEquals(REWRITE_CONTENT, recorder.getResponseAsString());
+ assertEquals(200, recorder.getHttpStatusCode());
+ assertTrue(rewriter.responseWasRewritten());
}
@Test
- public void testHtmlAccelRewriteSimple() throws Exception {
+ public void testHtmlAccelRewriteDoesNotFollowRedirects() throws Exception {
String url = "http://example.org/data.html";;
- String data = "<html><body>Hello World</body></html>";
+ String data = "<html><body>Moved to new page</body></html>";
+ String redirectLocation = "http://example-redirected.org/data.html";;
- ((FakeCaptureRewriter) rewriter).setContentToRewrite(REWRITE_CONTENT);
+ ((FakeCaptureRewriter) rewriter).setContentToRewrite(data);
HttpRequest req = new HttpRequest(Uri.parse(url));
HttpResponse resp = new HttpResponseBuilder()
.setResponse(data.getBytes())
.setHeader("Content-Type", "text/html")
- .setHttpStatusCode(200)
+ .setHeader("Location", redirectLocation)
+ .setHttpStatusCode(302)
.create();
expect(pipeline.execute(req)).andReturn(resp).once();
expectRequest("", url);
@@ -191,13 +197,14 @@ public class HtmlAccelServletTest extend
servlet.doGet(request, recorder);
verify();
- assertEquals(REWRITE_CONTENT, recorder.getResponseAsString());
- assertEquals(200, recorder.getHttpStatusCode());
+ assertEquals(data, recorder.getResponseAsString());
+ assertEquals(redirectLocation, recorder.getHeader("Location"));
+ assertEquals(302, recorder.getHttpStatusCode());
assertTrue(rewriter.responseWasRewritten());
}
@Test
- public void testHtmlAccelRewriteErrorCode() throws Exception {
+ public void testHtmlAccelReturnsOriginal404MessageAndCode() throws Exception
{
String url = "http://example.org/data.html";;
String data = "<html><body>This is error page</body></html>";
@@ -214,7 +221,7 @@ public class HtmlAccelServletTest extend
servlet.doGet(request, recorder);
verify();
- assertEquals(AccelHandler.ERROR_FETCHING_DATA,
recorder.getResponseAsString());
+ assertEquals(data, recorder.getResponseAsString());
assertEquals(404, recorder.getHttpStatusCode());
assertFalse(rewriter.responseWasRewritten());
}
@@ -237,7 +244,7 @@ public class HtmlAccelServletTest extend
servlet.doGet(request, recorder);
verify();
- assertEquals(AccelHandler.ERROR_FETCHING_DATA,
recorder.getResponseAsString());
+ assertEquals(data, recorder.getResponseAsString());
assertEquals(502, recorder.getHttpStatusCode());
assertFalse(rewriter.responseWasRewritten());
}
@@ -265,4 +272,27 @@ public class HtmlAccelServletTest extend
assertEquals("POST", req.getValue().getMethod());
assertEquals("hello=world", req.getValue().getPostBodyAsString());
}
+
+ @Test
+ public void testHtmlAccelReturnsSameStatusCodeAndMessageWhenError() throws
Exception {
+ String url = "http://example.org/data.html";;
+ String data = "<html><body>This is error page</body></html>";
+
+ ((FakeCaptureRewriter) rewriter).setContentToRewrite(data);
+ HttpRequest req = new HttpRequest(Uri.parse(url));
+ HttpResponse resp = new HttpResponseBuilder()
+ .setResponse(data.getBytes())
+ .setHeader("Content-Type", "text/html")
+ .setHttpStatusCode(5001)
+ .create();
+ expect(pipeline.execute(req)).andReturn(resp).once();
+ expectRequest("", url);
+ replay();
+
+ servlet.doGet(request, recorder);
+ verify();
+ assertEquals(data, recorder.getResponseAsString());
+ assertEquals(5001, recorder.getHttpStatusCode());
+ assertFalse(rewriter.responseWasRewritten());
+ }
}
Modified:
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/UriUtilsTest.java
URL:
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/UriUtilsTest.java?rev=979468&r1=979467&r2=979468&view=diff
==============================================================================
---
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/UriUtilsTest.java
(original)
+++
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/UriUtilsTest.java
Mon Jul 26 22:12:54 2010
@@ -75,7 +75,7 @@ public class UriUtilsTest extends EasyMo
replay();
- UriUtils.copyResponseHeadersAndStatusCode(resp, response, false,
+ UriUtils.copyResponseHeadersAndStatusCode(resp, response, false, false,
UriUtils.DisallowedHeaders.OUTPUT_TRANSFER_DIRECTIVES,
UriUtils.DisallowedHeaders.CACHING_DIRECTIVES);
verify();
@@ -103,7 +103,63 @@ public class UriUtilsTest extends EasyMo
replay();
- UriUtils.copyResponseHeadersAndStatusCode(resp, response, true,
+ UriUtils.copyResponseHeadersAndStatusCode(resp, response, false, true,
+ UriUtils.DisallowedHeaders.OUTPUT_TRANSFER_DIRECTIVES,
+ UriUtils.DisallowedHeaders.CACHING_DIRECTIVES);
+ verify();
+ }
+
+ @Test
+ public void testCopyResponseHeadersAndStatusCode_RemapTrue() throws
Exception {
+ HttpResponse resp = new HttpResponseBuilder()
+ .setHttpStatusCode(500)
+ .addHeader("hello", "world1")
+ .addHeader("hello", "world2")
+ .addHeader("hello\\u2297", "bad header")
+ .addHeader("Content-length", "10")
+ .addHeader("vary", "1")
+ .create();
+ HttpServletResponse response = mock(HttpServletResponse.class);
+
+ response.setStatus(502);
+ EasyMock.expectLastCall().once();
+
+ response.setHeader("hello", "world1");
+ EasyMock.expectLastCall().once();
+ response.setHeader("hello", "world2");
+ EasyMock.expectLastCall().once();
+
+
+ replay();
+ UriUtils.copyResponseHeadersAndStatusCode(resp, response, true, true,
+ UriUtils.DisallowedHeaders.OUTPUT_TRANSFER_DIRECTIVES,
+ UriUtils.DisallowedHeaders.CACHING_DIRECTIVES);
+ verify();
+ }
+
+ @Test
+ public void testCopyResponseHeadersAndStatusCode_RemapFalse() throws
Exception {
+ HttpResponse resp = new HttpResponseBuilder()
+ .setHttpStatusCode(500)
+ .addHeader("hello", "world1")
+ .addHeader("hello", "world2")
+ .addHeader("hello\\u2297", "bad header")
+ .addHeader("Content-length", "10")
+ .addHeader("vary", "1")
+ .create();
+ HttpServletResponse response = mock(HttpServletResponse.class);
+
+ response.setStatus(500);
+ EasyMock.expectLastCall().once();
+
+ response.setHeader("hello", "world1");
+ EasyMock.expectLastCall().once();
+ response.setHeader("hello", "world2");
+ EasyMock.expectLastCall().once();
+
+
+ replay();
+ UriUtils.copyResponseHeadersAndStatusCode(resp, response, false, true,
UriUtils.DisallowedHeaders.OUTPUT_TRANSFER_DIRECTIVES,
UriUtils.DisallowedHeaders.CACHING_DIRECTIVES);
verify();
