Nice change :) Small nits:
http://codereview.appspot.com/1696056/diff/1/3 File java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java (left): http://codereview.appspot.com/1696056/diff/1/3#oldcode140 java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java:140: for (Map.Entry<String, String> entry : results.getHeaders().entries()) { you should use UriUtils here. @See http://codereview.appspot.com/1855044/show and http://codereview.appspot.com/1855041/show. http://codereview.appspot.com/1696056/diff/1/3#oldcode174 java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java:174: IOUtils.copy(results.getResponse(), response.getOutputStream()); @See http://codereview.appspot.com/1855044/diff/45001/46005 http://codereview.appspot.com/1696056/diff/1/3 File java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java (right): http://codereview.appspot.com/1696056/diff/1/3#newcode57 java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java:57: public static final String OUTPUT_JS_PARAM_VALUE = "js"; maybe this should be &output=json ? http://codereview.appspot.com/1696056/diff/1/3#newcode195 java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java:195: private void emitOutputJs(String contentType, HttpResponse results, HttpServletResponse response) would something like emitResponseAsJSON look better ? basically to convey that we are emitting a json object which has the actual resource and metadata of original page. http://codereview.appspot.com/1696056/diff/1/2 File java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyHandlerTest.java (right): http://codereview.appspot.com/1696056/diff/1/2#newcode409 java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyHandlerTest.java:409: assertEquals("data:" + expectedType + ";base64;charset=" + expectedEncoding + "," + content64, would be nice to show the response in string form for tests. Returning "data:" + expectedType + ... is kind of exactly what the original function does, we are not really testing it. Maybe you can return js.getString(ProxyHandler.DATA_URI_KEY) here and in test testOutputDataUriWithCharset, you can say assertEquals("data:expectedType;base64;....", checkOutputDataUri("text/bar; charset=ISO-8859-1", "text/bar", "ISO-8859-1"); Makes sense ? http://codereview.appspot.com/1696056/show
