Happy to see logic slowly consolidating.
http://codereview.appspot.com/1855044/diff/45001/46005 File java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java (right): http://codereview.appspot.com/1855044/diff/45001/46005#newcode146 java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java:146: UriUtils.DisallowedHeaders.CLIENT_STATE_DIRECTIVES); These look to me like the right header groups. The full delta, as of this patch revision, is: Previously allowed, now disallowed: "server" (from OUTPUT_TRANSFER_DIRECTIVES) Previously disallowed, now passed through: "etag", "last-modified". Presumably these are the ones you mention are coming in an updated patch#? (CLIENT_STATE_DIRECTIVES additions) http://codereview.appspot.com/1855044/diff/45001/46005#newcode154 java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java:154: IOUtils.copy(results.getResponse(), response.getOutputStream()); Alas, not so. Per: http://java.sun.com/products/servlet/2.3/javadoc/javax/servlet/http/HttpServletResponse.html#sendError(int) "After using this method, the response should be considered to be committed and should not be written to." What we really want is: response.sendError(code, results.getResponseAsString()); On 2010/07/26 18:46:46, gagan.goku wrote:
On 2010/07/26 18:40:22, zhoresh wrote: > You probably don't want to copy data in case of error.
actually we do. If the original site returns an informative error
message, we
should send it back to the user shouldnt we ?
http://codereview.appspot.com/1855044/diff/45001/46006 File java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/UriUtils.java (right): http://codereview.appspot.com/1855044/diff/45001/46006#newcode216 java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/UriUtils.java:216: public static void rewriteContentType(HttpRequest req, HttpServletResponse response) { nit: I'd prefer this to be maybeRewriteContentType, to reflect the if-condition. http://codereview.appspot.com/1855044/show
