Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(71)

Issue 1976047: Fix proxy to handle uri generated by CssRewriter

Can't Edit
Can't Publish+Mail
Start Review
Created:
15 years, 6 months ago by zhoresh
Modified:
15 years, 6 months ago
Reviewers:
johnfargo, dev-remailer
Base URL:
http://svn.apache.org/repos/asf/shindig/trunk
Visibility:
Public.

Description

CssRewriter use Caja to update reference urls to proxy. Caja code encode the url (container= become containe%3d) The change support encoded chain params. Also included couple other minor issue that was found in production.

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+130 lines, -67 lines) Patch
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java View 2 chunks +8 lines, -2 lines 0 comments Download
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java View 2 chunks +2 lines, -2 lines 1 comment Download
java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultProxyUriManager.java View 13 chunks +39 lines, -27 lines 0 comments Download
java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultProxyUriManagerTest.java View 24 chunks +81 lines, -36 lines 0 comments Download

Messages

Total messages: 3
zhoresh
15 years, 6 months ago (2010-08-21 01:11:58 UTC) #1
johnfargo
LGTM++ I like the doDecode rather than doNotDecode logic. http://codereview.appspot.com/1976047/diff/1/2 File java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java (right): http://codereview.appspot.com/1976047/diff/1/2#newcode2 java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java:2: ...
15 years, 6 months ago (2010-08-21 01:23:33 UTC) #2
zhoresh
15 years, 6 months ago (2010-08-24 16:46:22 UTC) #3
On 2010/08/21 01:23:33, johnfargo wrote:
> LGTM++
> 
> I like the doDecode rather than doNotDecode logic.
> 
> http://codereview.appspot.com/1976047/diff/1/2
> File
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java
> (right):
> 
> http://codereview.appspot.com/1976047/diff/1/2#newcode2
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java:2:
> * Licensed to the Apache Software Foundation (ASF) under one
> chunk mismatch in this patch

Thanks! fixed and submitted.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b