You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@shindig.apache.org by jo...@apache.org on 2010/02/12 01:56:48 UTC
svn commit: r909194 - in /shindig/trunk/java/gadgets/src:
main/java/org/apache/shindig/gadgets/
main/java/org/apache/shindig/gadgets/http/
main/java/org/apache/shindig/gadgets/servlet/
main/java/org/apache/shindig/gadgets/templates/ test/java/org/apach...
Author: johnh
Date: Fri Feb 12 00:56:45 2010
New Revision: 909194
URL: http://svn.apache.org/viewvc?rev=909194&view=rev
Log:
BasicHttpFetcher use apache HttpClient.
Which doesn't handle cleanly uris, for example underscores in url.
The change pass the url already parsed to HttpClient.
Patch provided by Ziv Horesh.
Modified:
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/AbstractSpecFactory.java
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/BasicHttpFetcher.java
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/HtmlAccelServlet.java
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/HttpRequestHandler.java
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/TemplateLibraryFactory.java
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpFetcherTest.java
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/HtmlAccelServletTest.java
Modified: shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/AbstractSpecFactory.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/AbstractSpecFactory.java?rev=909194&r1=909193&r2=909194&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/AbstractSpecFactory.java (original)
+++ shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/AbstractSpecFactory.java Fri Feb 12 00:56:45 2010
@@ -128,10 +128,15 @@
HttpResponse response = pipeline.execute(request);
if (response.getHttpStatusCode() != HttpResponse.SC_OK) {
+ int retcode = response.getHttpStatusCode();
+ if (retcode == HttpResponse.SC_INTERNAL_SERVER_ERROR) {
+ // Convert external "internal error" to gateway error:
+ retcode = HttpResponse.SC_BAD_GATEWAY;
+ }
throw new GadgetException(GadgetException.Code.FAILED_TO_RETRIEVE_CONTENT,
"Unable to retrieve spec for " + query.specUri + ". HTTP error " +
response.getHttpStatusCode(),
- HttpResponse.SC_BAD_GATEWAY);
+ retcode);
}
try {
Modified: shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/BasicHttpFetcher.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/BasicHttpFetcher.java?rev=909194&r1=909193&r2=909194&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/BasicHttpFetcher.java (original)
+++ shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/BasicHttpFetcher.java Fri Feb 12 00:56:45 2010
@@ -28,6 +28,7 @@
import org.apache.http.HeaderElement;
import org.apache.http.HttpEntity;
import org.apache.http.HttpException;
+import org.apache.http.HttpHost;
import org.apache.http.HttpRequestInterceptor;
import org.apache.http.HttpResponseInterceptor;
import org.apache.http.HttpVersion;
@@ -63,6 +64,8 @@
import org.apache.http.params.HttpProtocolParams;
import org.apache.http.protocol.HttpContext;
import org.apache.http.util.EntityUtils;
+import org.apache.shindig.common.uri.Uri;
+import org.apache.shindig.gadgets.GadgetException;
import java.io.IOException;
import java.io.InputStream;
@@ -79,6 +82,8 @@
import java.util.zip.Inflater;
import java.util.zip.InflaterInputStream;
+import javax.servlet.http.HttpServletResponse;
+
/**
* A simple HTTP fetcher implementation based on Apache httpclient. Not recommended for production deployments until
* the following issues are addressed:
@@ -245,15 +250,49 @@
}
}
- public HttpResponse fetch(org.apache.shindig.gadgets.http.HttpRequest request) {
+ public HttpResponse fetch(org.apache.shindig.gadgets.http.HttpRequest request)
+ throws GadgetException {
HttpUriRequest httpMethod = null;
Preconditions.checkNotNull(request);
final String methodType = request.getMethod();
- final String requestUri = request.getUri().toString();
final org.apache.http.HttpResponse response;
final long started = System.currentTimeMillis();
+ // Break the request Uri to its components:
+ Uri uri = request.getUri();
+ if (StringUtils.isEmpty(uri.getAuthority())) {
+ throw new GadgetException(GadgetException.Code.INVALID_USER_DATA,
+ "Missing domain name for request: " + uri,
+ HttpServletResponse.SC_BAD_REQUEST);
+ }
+ if (StringUtils.isEmpty(uri.getScheme())) {
+ throw new GadgetException(GadgetException.Code.INVALID_USER_DATA,
+ "Missing schema for request: " + uri,
+ HttpServletResponse.SC_BAD_REQUEST);
+ }
+ String[] hostparts = uri.getAuthority().split(":");
+ int port = -1; // default port
+ if (hostparts.length > 2) {
+ throw new GadgetException(GadgetException.Code.INVALID_USER_DATA,
+ "Bad host name in request: " + uri.getAuthority(),
+ HttpServletResponse.SC_BAD_REQUEST);
+ }
+ if (hostparts.length == 2) {
+ try {
+ port = Integer.parseInt(hostparts[1]);
+ } catch (NumberFormatException e) {
+ throw new GadgetException(GadgetException.Code.INVALID_USER_DATA,
+ "Bad port number in request: " + uri.getAuthority(),
+ HttpServletResponse.SC_BAD_REQUEST);
+ }
+ }
+ HttpHost host = new HttpHost(hostparts[0], port, uri.getScheme());
+ String requestUri = uri.getPath();
+ if (uri.getQuery() != null) {
+ requestUri += "?" + uri.getQuery();
+ }
+
try {
if ("POST".equals(methodType) || "PUT".equals(methodType)) {
HttpEntityEnclosingRequestBase enclosingMethod = ("POST".equals(methodType))
@@ -277,8 +316,10 @@
if (!request.getFollowRedirects())
httpMethod.getParams().setBooleanParameter(ClientPNames.HANDLE_REDIRECTS, false);
-
- response = FETCHER.execute(httpMethod);
+
+ // HttpClient doesn't handle all cases when breaking url (specifically '_' in domain)
+ // So lets pass it the url parsed:
+ response = FETCHER.execute(host, httpMethod);
if (response == null)
throw new IOException("Unknown problem with request");
@@ -299,13 +340,15 @@
// Find timeout exceptions, respond accordingly
if (TIMEOUT_EXCEPTIONS.contains(e.getClass())) {
- LOG.warning("Timeout for " + request.getUri() + " Exception: " + e.getClass().getName() + " - " + e.getMessage() + " - " + (now - started) + "ms");
+ LOG.info("Timeout for " + request.getUri() + " Exception: " + e.getClass().getName() + " - " + e.getMessage() + " - " + (now - started) + "ms");
return HttpResponse.timeout();
}
- LOG.log(Level.WARNING, "Got Exception fetching " + request.getUri() + " - " + (now - started) + "ms", e);
+ LOG.log(Level.INFO, "Got Exception fetching " + request.getUri() + " - " + (now - started) + "ms", e);
- return HttpResponse.error();
+ // Separate shindig error from external error
+ throw new GadgetException(GadgetException.Code.INTERNAL_SERVER_ERROR, e,
+ HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
} finally {
// cleanup any outstanding resources..
if (httpMethod != null) try {
Modified: shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/HtmlAccelServlet.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/HtmlAccelServlet.java?rev=909194&r1=909193&r2=909194&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/HtmlAccelServlet.java (original)
+++ shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/HtmlAccelServlet.java Fri Feb 12 00:56:45 2010
@@ -106,7 +106,7 @@
try {
data = fetch(context);
} catch (GadgetException e) {
- resp.sendError(HttpServletResponse.SC_BAD_REQUEST, e.getMessage());
+ resp.sendError(e.getHttpStatusCode(), e.getMessage());
return;
}
@@ -115,7 +115,7 @@
resp.sendError(HttpServletResponse.SC_BAD_REQUEST, "Error fetching data");
return;
}
-
+
// If not html, just return data
if (!isHtmlContent(data)) {
respondVerbatim(data, resp);
@@ -153,6 +153,14 @@
// Call the gadget renderer to rewrite content:
super.doGet(reqWrapper,resp);
+
+ // Pass original return code:
+ if (data.getHttpStatusCode() == HttpResponse.SC_INTERNAL_SERVER_ERROR) {
+ // Convert external "internal error" to gateway error
+ resp.setStatus(HttpServletResponse.SC_BAD_GATEWAY);
+ } else if (data.getHttpStatusCode() >= 400) {
+ resp.setStatus(data.getHttpStatusCode());
+ }
}
@Override
@@ -191,6 +199,10 @@
}
}
response.setStatus(results.getHttpStatusCode());
+ if (results.getHttpStatusCode() == HttpResponse.SC_INTERNAL_SERVER_ERROR) {
+ // Convert external "internal error" to gateway error
+ response.setStatus(HttpServletResponse.SC_BAD_GATEWAY);
+ }
IOUtils.copy(results.getResponse(), response.getOutputStream());
}
Modified: shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/HttpRequestHandler.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/HttpRequestHandler.java?rev=909194&r1=909193&r2=909194&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/HttpRequestHandler.java (original)
+++ shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/HttpRequestHandler.java Fri Feb 12 00:56:45 2010
@@ -236,10 +236,10 @@
}
return httpApiResponse;
} catch (GadgetException ge) {
- throw new ProtocolException(HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
+ throw new ProtocolException(ge.getHttpStatusCode(),
ge.getMessage(), ge);
} catch (RewritingException re) {
- throw new ProtocolException(HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
+ throw new ProtocolException(re.getHttpStatusCode(),
re.getMessage(), re);
}
}
Modified: shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java?rev=909194&r1=909193&r2=909194&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java (original)
+++ shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java Fri Feb 12 00:56:45 2010
@@ -194,7 +194,12 @@
setResponseHeaders(request, response, results);
if (results.getHttpStatusCode() != HttpResponse.SC_OK) {
- response.sendError(results.getHttpStatusCode());
+ if (results.getHttpStatusCode() == HttpResponse.SC_INTERNAL_SERVER_ERROR) {
+ // External "internal error" should be mapped to gateway error
+ response.sendError(HttpResponse.SC_BAD_GATEWAY);
+ } else {
+ response.sendError(results.getHttpStatusCode());
+ }
}
IOUtils.copy(results.getResponse(), response.getOutputStream());
Modified: shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/TemplateLibraryFactory.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/TemplateLibraryFactory.java?rev=909194&r1=909193&r2=909194&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/TemplateLibraryFactory.java (original)
+++ shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/TemplateLibraryFactory.java Fri Feb 12 00:56:45 2010
@@ -61,9 +61,14 @@
request.setCacheTtl(300);
HttpResponse response = pipeline.execute(request);
if (response.getHttpStatusCode() != HttpResponse.SC_OK) {
+ int retcode = response.getHttpStatusCode();
+ if (retcode == HttpResponse.SC_INTERNAL_SERVER_ERROR) {
+ // Convert external "internal error" to gateway error:
+ retcode = HttpResponse.SC_BAD_GATEWAY;
+ }
throw new GadgetException(GadgetException.Code.FAILED_TO_RETRIEVE_CONTENT,
"Unable to retrieve template library xml. HTTP error " +
- response.getHttpStatusCode());
+ response.getHttpStatusCode(), retcode);
}
String content = response.getResponseAsString();
Modified: shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpFetcherTest.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpFetcherTest.java?rev=909194&r1=909193&r2=909194&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpFetcherTest.java (original)
+++ shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpFetcherTest.java Fri Feb 12 00:56:45 2010
@@ -19,10 +19,14 @@
package org.apache.shindig.gadgets.http;
import junitx.framework.ArrayAssert;
+
import org.apache.shindig.common.uri.Uri;
import org.apache.shindig.common.uri.UriBuilder;
+import org.apache.shindig.gadgets.GadgetException;
import org.junit.AfterClass;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
import org.junit.BeforeClass;
import org.junit.Test;
@@ -49,6 +53,61 @@
}
}
+ @Test public void testHttpBadHost() throws Exception {
+ Uri uri = Uri.parse("http://a:b:c/");
+ HttpRequest request = new HttpRequest(uri);
+ try {
+ HttpResponse response = fetcher.fetch(request);
+ fail("Expected GadgetException");
+ } catch (GadgetException e) {
+ assertEquals(400, e.getHttpStatusCode());
+ assertTrue(e.getMessage().contains("Bad host name in request"));
+ }
+ }
+
+ @Test public void testHttpBadPort() throws Exception {
+ Uri uri = Uri.parse("http://a:b/");
+ HttpRequest request = new HttpRequest(uri);
+ try {
+ HttpResponse response = fetcher.fetch(request);
+ fail("Expected GadgetException");
+ } catch (GadgetException e) {
+ assertEquals(400, e.getHttpStatusCode());
+ assertTrue(e.getMessage().contains("Bad port number in request"));
+ }
+ }
+
+ @Test public void testHttpBadUrl() throws Exception {
+ Uri uri = Uri.parse("host/data");
+ HttpRequest request = new HttpRequest(uri);
+ try {
+ HttpResponse response = fetcher.fetch(request);
+ fail("Expected GadgetException");
+ } catch (GadgetException e) {
+ assertEquals(400, e.getHttpStatusCode());
+ assertTrue(e.getMessage().contains("Missing domain name for request"));
+ }
+ }
+
+ @Test public void testHttpNoSchema() throws Exception {
+ Uri uri = Uri.parse("//host/data");
+ HttpRequest request = new HttpRequest(uri);
+ try {
+ HttpResponse response = fetcher.fetch(request);
+ fail("Expected GadgetException");
+ } catch (GadgetException e) {
+ assertEquals(400, e.getHttpStatusCode());
+ assertTrue(e.getMessage().contains("Missing schema for request"));
+ }
+ }
+
+ @Test public void testHttpUnderscore() throws Exception {
+ Uri uri = Uri.parse("http://0.test_host.com/data");
+ HttpRequest request = new HttpRequest(uri);
+ HttpResponse response = fetcher.fetch(request);
+ assertEquals(504, response.getHttpStatusCode()); //timeout
+ }
+
@Test public void testHttpFetch() throws Exception {
String content = "Hello, world!";
Uri uri = new UriBuilder(BASE_URL).addQueryParameter("body", content).toUri();
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=909194&r1=909193&r2=909194&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 Fri Feb 12 00:56:45 2010
@@ -103,7 +103,7 @@
.create();
expect(pipeline.execute(req)).andReturn(resp).once();
expect(request.getParameter("url")).andReturn(url).once();
- expect(request.getRequestURL()).andReturn(new StringBuffer("gmodules.com/gadgets/accel"))
+ expect(request.getRequestURL()).andReturn(new StringBuffer("apache.org/gadgets/accel"))
.once();
expect(request.getQueryString()).andReturn("url=" + url).once();
expect(renderer.render(isA(GadgetContext.class)))
@@ -117,6 +117,58 @@
}
@Test
+ public void testHtmlAccelRewriteErrorCode() throws Exception {
+ String url = "http://example.org/data.html";
+ String data = "<html><body>This is error page</body></html>";
+
+ HttpRequest req = new HttpRequest(Uri.parse(url));
+ HttpResponse resp = new HttpResponseBuilder()
+ .setResponse(data.getBytes())
+ .setHeader("Content-Type", "text/html")
+ .setHttpStatusCode(404)
+ .create();
+ expect(pipeline.execute(req)).andReturn(resp).once();
+ expect(request.getParameter("url")).andReturn(url).once();
+ expect(request.getRequestURL()).andReturn(new StringBuffer("apache.org/gadgets/accel"))
+ .once();
+ expect(request.getQueryString()).andReturn("url=" + url).once();
+ expect(renderer.render(isA(GadgetContext.class)))
+ .andReturn(RenderingResults.ok(REWRITE_CONTENT));
+ replay();
+
+ servlet.doGet(request, recorder);
+ verify();
+ assertEquals(REWRITE_CONTENT, recorder.getResponseAsString());
+ assertEquals(404, recorder.getHttpStatusCode());
+ }
+
+ @Test
+ public void testHtmlAccelRewriteInternalError() throws Exception {
+ String url = "http://example.org/data.html";
+ String data = "<html><body>This is error page</body></html>";
+
+ HttpRequest req = new HttpRequest(Uri.parse(url));
+ HttpResponse resp = new HttpResponseBuilder()
+ .setResponse(data.getBytes())
+ .setHeader("Content-Type", "text/html")
+ .setHttpStatusCode(500)
+ .create();
+ expect(pipeline.execute(req)).andReturn(resp).once();
+ expect(request.getParameter("url")).andReturn(url).once();
+ expect(request.getRequestURL()).andReturn(new StringBuffer("apache.org/gadgets/accel"))
+ .once();
+ expect(request.getQueryString()).andReturn("url=" + url).once();
+ expect(renderer.render(isA(GadgetContext.class)))
+ .andReturn(RenderingResults.ok(REWRITE_CONTENT));
+ replay();
+
+ servlet.doGet(request, recorder);
+ verify();
+ assertEquals(REWRITE_CONTENT, recorder.getResponseAsString());
+ assertEquals(502, recorder.getHttpStatusCode());
+ }
+
+ @Test
public void testHtmlAccelParams() throws Exception {
Renderer newRenderer = new Renderer(null, null, null, lockedDomainService) {
@@ -141,7 +193,7 @@
.create();
expect(pipeline.execute(req)).andReturn(resp).once();
expect(request.getParameter("url")).andReturn(url).once();
- expect(request.getRequestURL()).andReturn(new StringBuffer("gmodules.com/gadgets/accel"))
+ expect(request.getRequestURL()).andReturn(new StringBuffer("apache.org/gadgets/accel"))
.once();
expect(request.getQueryString()).andReturn("url=" + url).once();
replay();