http://codereview.appspot.com/1635042/diff/2001/3003 File java/gadgets/src/main/java/org/apache/shindig/gadgets/http/BasicHttpFetcher.java (right):
http://codereview.appspot.com/1635042/diff/2001/3003#newcode101 java/gadgets/src/main/java/org/apache/shindig/gadgets/http/BasicHttpFetcher.java:101: protected final String REQUEST_URI_HEADER_NAME = "Google-RequestUri"; I think the naming convention is to start with X- so maybe name it "X-Shindig-RequestUri" http://codereview.appspot.com/1635042/diff/2001/3003#newcode213 java/gadgets/src/main/java/org/apache/shindig/gadgets/http/BasicHttpFetcher.java:213: // Add hook for overridden host header in order to support proxying. Please add more details to what you try to do http://codereview.appspot.com/1635042/diff/2001/3003#newcode219 java/gadgets/src/main/java/org/apache/shindig/gadgets/http/BasicHttpFetcher.java:219: String requestUri = request.getHeaders(REQUEST_URI_HEADER_NAME)[0] I think it can be empty list. http://codereview.appspot.com/1635042/diff/2001/3003#newcode222 java/gadgets/src/main/java/org/apache/shindig/gadgets/http/BasicHttpFetcher.java:222: request.removeHeader(request.getHeaders("Host")[0]); Don't you want removeHeader("Host") ? http://codereview.appspot.com/1635042/diff/2001/3003#newcode310 java/gadgets/src/main/java/org/apache/shindig/gadgets/http/BasicHttpFetcher.java:310: int port = 80; // default port -1 will tell HttpHost to use default (which is 80) and not add port to url. So new code should do the same. http://codereview.appspot.com/1635042/diff/2001/3003#newcode507 java/gadgets/src/main/java/org/apache/shindig/gadgets/http/BasicHttpFetcher.java:507: host = new HttpHost(splits[0], Integer.parseInt(splits[1])); Handle NumberFormatException (even just to add message to exception that indicate bad configuration) And probably support no port (default to -1) http://codereview.appspot.com/1635042/show
