http://codereview.appspot.com/1822041/diff/9001/10004 File main/java/org/apache/shindig/gadgets/uri/UriUtils.java (right):
http://codereview.appspot.com/1822041/diff/9001/10004#newcode70 main/java/org/apache/shindig/gadgets/uri/UriUtils.java:70: // Headers that the fetcher itself would like to fill. This comment seems slightly cryptic to me - could you give a few more details as to why the fetcher needs to fill this. Could you also add a comment (similar to 38-42) just before the enum to explain what this enum is meant for? http://codereview.appspot.com/1822041/diff/9001/10004#newcode71 main/java/org/apache/shindig/gadgets/uri/UriUtils.java:71: POST_INCOMPATIBLE_HEADERS(ImmutableSet.of("content-length")); In order to use consistent terminology, would POST_INCOMPATIBLE_DIRECTIVES be a better name here? http://codereview.appspot.com/1822041/diff/9001/10004#newcode154 main/java/org/apache/shindig/gadgets/uri/UriUtils.java:154: public static void copyRequestHeaders(HttpServletRequest request, Similar to the previous method, using "data" for this parameter might make this more readable. http://codereview.appspot.com/1822041/diff/9001/10004#newcode158 main/java/org/apache/shindig/gadgets/uri/UriUtils.java:158: for (DisallowedRequestHeaders h : disallowedResponseHeaders) { disallowedResponseHeaders -> disallowedRequestHeaders. http://codereview.appspot.com/1822041/diff/9001/10004#newcode162 main/java/org/apache/shindig/gadgets/uri/UriUtils.java:162: Enumeration<String> headerNames = request.getHeaderNames(); Wouldn't using a block similar to 136-141 work here as well (with a small change that uses addHeader instead of setHeader)? http://codereview.appspot.com/1822041/diff/9001/10004#newcode190 main/java/org/apache/shindig/gadgets/uri/UriUtils.java:190: Enumeration<String> paramNames = request.getParameterNames(); Just wondering - Isn't there a shorter Map.Entry<String, String> kind of block that can be used here? http://codereview.appspot.com/1822041/show
