Author: awiner
Date: Wed Jun 10 20:37:37 2009
New Revision: 783494
URL: http://svn.apache.org/viewvc?rev=783494&view=rev
Log:
Unify proxy handling of errors, and some small improvements:
- Make servlets truly trivial by moving exception handling into ProxyBase
- Report GadgetExceptions for internal server errors as 500s, and log as
warnings
- Catch illegal exceptions for incorrect "refresh" params on proxy requests and
treat as 400s
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestServlet.java
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyBase.java
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestHandlerTest.java
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyBaseTest.java
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyHandlerTest.java
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java?rev=783494&r1=783493&r2=783494&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java
Wed Jun 10 20:37:37 2009
@@ -79,7 +79,7 @@
response.getOutputStream().println("/* ---- Start " + url + " ----
*/");
ResponseWrapper wrapper = new ResponseWrapper(response);
- proxyHandler.fetch(new RequestWrapper(request, url), wrapper);
+ proxyHandler.doFetch(new RequestWrapper(request, url), wrapper);
if (wrapper.getStatus() != HttpServletResponse.SC_OK) {
response.getOutputStream().println(
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java?rev=783494&r1=783493&r2=783494&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java
Wed Jun 10 20:37:37 2009
@@ -77,7 +77,7 @@
* Executes a request, returning the response as JSON to be handled by
makeRequest.
*/
@Override
- public void fetch(HttpServletRequest request, HttpServletResponse response)
+ protected void doFetch(HttpServletRequest request, HttpServletResponse
response)
throws GadgetException, IOException {
HttpRequest rcr = buildHttpRequest(request);
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestServlet.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestServlet.java?rev=783494&r1=783493&r2=783494&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestServlet.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestServlet.java
Wed Jun 10 20:37:37 2009
@@ -19,17 +19,14 @@
package org.apache.shindig.gadgets.servlet;
import org.apache.shindig.common.servlet.InjectedServlet;
-import org.apache.shindig.gadgets.GadgetException;
-
-import com.google.inject.Inject;
import java.io.IOException;
-import java.util.logging.Level;
-import java.util.logging.Logger;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
+import com.google.inject.Inject;
+
/**
* Handles calls to gadgets.io.makeRequest.
*
@@ -39,8 +36,6 @@
* makeRequest and open proxy calls.
*/
public class MakeRequestServlet extends InjectedServlet {
- private final static Logger LOG =
Logger.getLogger(MakeRequestServlet.class.getName());
-
private MakeRequestHandler makeRequestHandler;
@Inject
@@ -51,12 +46,7 @@
@Override
protected void doGet(HttpServletRequest request, HttpServletResponse
response)
throws IOException {
- try {
- makeRequestHandler.fetch(request, response);
- } catch (GadgetException e) {
- // TODO: Move this logic into ProxyHandler / MakeRequestHandler.
- outputError(e, response);
- }
+ makeRequestHandler.fetch(request, response);
}
@Override
@@ -64,15 +54,4 @@
throws IOException {
doGet(request, response);
}
-
- /**
- * Outputs an error message for the request if it fails.
- *
- * TODO: Eliminate this.
- */
- private static void outputError(GadgetException e, HttpServletResponse resp)
- throws IOException {
- LOG.log(Level.INFO, "makeRequest failed", e);
- resp.sendError(HttpServletResponse.SC_BAD_REQUEST, e.getMessage());
- }
}
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyBase.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyBase.java?rev=783494&r1=783493&r2=783494&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyBase.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyBase.java
Wed Jun 10 20:37:37 2009
@@ -30,6 +30,8 @@
import java.io.IOException;
import java.util.Set;
+import java.util.logging.Level;
+import java.util.logging.Logger;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
@@ -55,6 +57,8 @@
"vary", "expires", "date", "pragma", "cache-control",
"transfer-encoding", "www-authenticate"
);
+ private static final Logger logger =
Logger.getLogger(ProxyBase.class.getName());
+
/**
* Validates the given url.
*
@@ -112,12 +116,17 @@
*/
@SuppressWarnings("boxing")
protected void setResponseHeaders(HttpServletRequest request,
- HttpServletResponse response, HttpResponse results) {
+ HttpServletResponse response, HttpResponse results) throws
GadgetException {
int refreshInterval = 0;
if (results.isStrictNoCache()) {
refreshInterval = 0;
- } else if (request.getParameter(REFRESH_PARAM) != null) {
- refreshInterval = Integer.valueOf(request.getParameter(REFRESH_PARAM));
+ } else if (request.getParameter(REFRESH_PARAM) != null) {
+ try {
+ refreshInterval =
Integer.valueOf(request.getParameter(REFRESH_PARAM));
+ } catch (NumberFormatException nfe) {
+ throw new GadgetException(GadgetException.Code.INVALID_PARAMETER,
+ "refresh parameter is not a number");
+ }
} else {
refreshInterval = Math.max(60 * 60, (int)(results.getCacheTtl() /
1000L));
}
@@ -133,7 +142,39 @@
/**
* Processes the given request.
*/
- abstract public void fetch(HttpServletRequest request, HttpServletResponse
response)
+ public final void fetch(HttpServletRequest request, HttpServletResponse
response)
+ throws IOException {
+ try {
+ doFetch(request, response);
+ } catch (GadgetException e) {
+ outputError(response, e);
+ }
+ }
+
+ /**
+ * Outputs an error message for the request if it fails.
+ */
+ protected void outputError(HttpServletResponse resp, GadgetException e)
+ throws IOException {
+
+ int responseCode;
+ Level level = Level.FINE;
+
+ switch (e.getCode()) {
+ case INTERNAL_SERVER_ERROR:
+ responseCode = HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
+ level = Level.WARNING;
+ break;
+ default:
+ responseCode = HttpServletResponse.SC_BAD_REQUEST;
+ break;
+ }
+
+ logger.log(level, "Request failed", e);
+ resp.sendError(responseCode, e.getMessage());
+ }
+
+ abstract protected void doFetch(HttpServletRequest request,
HttpServletResponse response)
throws GadgetException, IOException;
protected boolean getIgnoreCache(HttpServletRequest request) {
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java?rev=783494&r1=783493&r2=783494&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java
Wed Jun 10 20:37:37 2009
@@ -117,7 +117,7 @@
}
@Override
- public void fetch(HttpServletRequest request, HttpServletResponse response)
+ protected void doFetch(HttpServletRequest request, HttpServletResponse
response)
throws IOException, GadgetException {
if (request.getHeader("If-Modified-Since") != null) {
response.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java?rev=783494&r1=783493&r2=783494&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java
Wed Jun 10 20:37:37 2009
@@ -19,24 +19,19 @@
package org.apache.shindig.gadgets.servlet;
import org.apache.shindig.common.servlet.InjectedServlet;
-import org.apache.shindig.gadgets.GadgetException;
-
-import com.google.inject.Inject;
import java.io.IOException;
-import java.util.logging.Level;
-import java.util.logging.Logger;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
+import com.google.inject.Inject;
+
/**
* Handles open proxy requests (used in rewriting and for URLs returned by
* gadgets.io.getProxyUrl).
*/
public class ProxyServlet extends InjectedServlet {
- private final static Logger LOG =
Logger.getLogger(ProxyServlet.class.getName());
-
private ProxyHandler proxyHandler;
@Inject
@@ -47,21 +42,6 @@
@Override
protected void doGet(HttpServletRequest request, HttpServletResponse
response)
throws IOException {
- try {
- proxyHandler.fetch(new ProxyServletRequest(request), response);
- } catch (GadgetException e) {
- outputError(e, response);
- }
- }
-
- /**
- * Outputs an error message for the request if it fails and if FINE logging
level.
- */
- private static void outputError(GadgetException e, HttpServletResponse resp)
- throws IOException {
- if (LOG.isLoggable(Level.FINE)) {
- LOG.log(Level.FINE, "Make Request failed", e);
- }
- resp.sendError(HttpServletResponse.SC_BAD_REQUEST, e.getMessage());
+ proxyHandler.fetch(new ProxyServletRequest(request), response);
}
}
Modified:
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestHandlerTest.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestHandlerTest.java?rev=783494&r1=783493&r2=783494&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestHandlerTest.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestHandlerTest.java
Wed Jun 10 20:37:37 2009
@@ -391,7 +391,7 @@
replay();
try {
- handler.fetch(request, recorder);
+ handler.doFetch(request, recorder);
fail("Should have thrown");
} catch (GadgetException e) {
// good.
Modified:
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyBaseTest.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyBaseTest.java?rev=783494&r1=783493&r2=783494&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyBaseTest.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyBaseTest.java
Wed Jun 10 20:37:37 2009
@@ -43,7 +43,7 @@
private final ProxyBase proxy = new ProxyBase() {
@Override
- public void fetch(HttpServletRequest request, HttpServletResponse
response) {
+ protected void doFetch(HttpServletRequest request, HttpServletResponse
response) {
// Nothing.
}
};
@@ -137,7 +137,7 @@
}
}
- public void testSetResponseHeaders() {
+ public void testSetResponseHeaders() throws Exception {
HttpResponse results = new HttpResponseBuilder().create();
replay();
@@ -149,7 +149,7 @@
assertEquals("attachment;filename=p.txt",
recorder.getHeader("Content-Disposition"));
}
- public void testSetResponseHeadersForFlash() {
+ public void testSetResponseHeadersForFlash() throws Exception {
HttpResponse results = new HttpResponseBuilder()
.setHeader("Content-Type", "application/x-shockwave-flash")
.create();
@@ -165,7 +165,7 @@
recorder.getHeader("Content-Disposition"));
}
- public void testSetResponseHeadersNoCache() {
+ public void testSetResponseHeadersNoCache() throws Exception {
Map<String, List<String>> headers =
Maps.newTreeMap(String.CASE_INSENSITIVE_ORDER);
headers.put("Pragma", Arrays.asList("no-cache"));
HttpResponse results = new HttpResponseBuilder()
@@ -182,7 +182,7 @@
assertEquals("attachment;filename=p.txt",
recorder.getHeader("Content-Disposition"));
}
- public void testSetResponseHeadersForceParam() {
+ public void testSetResponseHeadersForceParam() throws Exception {
HttpResponse results = new HttpResponseBuilder().create();
expect(request.getParameter(ProxyBase.REFRESH_PARAM)).andReturn("30").anyTimes();
replay();
@@ -193,6 +193,19 @@
assertEquals("attachment;filename=p.txt",
recorder.getHeader("Content-Disposition"));
}
+ public void testSetResponseHeadersForceParamInvalid() throws Exception {
+ HttpResponse results = new HttpResponseBuilder().create();
+
expect(request.getParameter(ProxyBase.REFRESH_PARAM)).andReturn("foo").anyTimes();
+ replay();
+
+ try {
+ proxy.setResponseHeaders(request, recorder, results);
+ } catch (GadgetException e) {
+ assertEquals(GadgetException.Code.INVALID_PARAMETER, e.getCode());
+ }
+ }
+
+
public void testGetParameter() {
expect(request.getParameter("foo")).andReturn("bar");
replay();
Modified:
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyHandlerTest.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyHandlerTest.java?rev=783494&r1=783493&r2=783494&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyHandlerTest.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyHandlerTest.java
Wed Jun 10 20:37:37 2009
@@ -116,7 +116,7 @@
expect(lockedDomainService.isSafeForOpenProxy("www.example.com")).andReturn(false);
replay();
try {
- proxyHandler.fetch(request, response);
+ proxyHandler.doFetch(request, response);
fail("Should have thrown");
} catch (GadgetException e) {
// good
