> On Oct. 10, 2012, 6:34 a.m., Henry Saputra wrote: > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java, > > line 171 > > <https://reviews.apache.org/r/7309/diff/3/?file=175008#file175008line171> > > > > Do we need else if over here? I think the if checks will guarantee that > > the Content-Disposition header is valid?
Yes, you are right, we don't need the if here. I will fix it. > On Oct. 10, 2012, 6:34 a.m., Henry Saputra wrote: > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java, > > line 168 > > <https://reviews.apache.org/r/7309/diff/3/?file=175008#file175008line168> > > > > Can we change this check with StringUtils.isBlank instead of "== null" > > just in case the response origin server return empty or blank > > Content-Disposition header? In theory, we don't need to check the empty because it will be checked by the following "contentDispositionValue.indexOf("attachment;") == -1". But I guess use it does no harm. Will update the patch with your suggestion. Thanks, Henry. - Erik ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7309/#review12306 ----------------------------------------------------------- On Oct. 10, 2012, 7:03 a.m., Erik Bi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7309/ > ----------------------------------------------------------- > > (Updated Oct. 10, 2012, 7:03 a.m.) > > > Review request for shindig, Ryan Baxter, Dan Dumont, Stanton Sievers, and > Rich Thompson. > > > Description > ------- > > Problem 1: Shindig proxy doesn't respect the "content-Disposition" header > correctly, so when user click a button to "download" the file via shindig > proxy use, the file name in the pop-up dialog is always "p.txt". Here code is > updated to copy the right Content-Disposition header. > > Problem 2: Even with the right Content-Disposition header, IE8 handles > Content-Disposition header differently. > > IE8 doesn't handle the Content-Disposition header like "Content-Disposition = > attachment; filename*= UTF-8''%e2%82%ac%20rates" correctly (should be a > limitation of IE8). > > Even with the right Content-Disposition header, when you open click a button > to "download" the file via shindig proxy, you will get a message box asking > if you want to open or save this file. At that time file name displayed will > not be the one in the header, but the path name of the current urlstart, in > Shindig, it will probably be "proxy." > > In order to fix, append a file name on proxy url and ignore that at the proxy > server side. > > For example, when you call > gadgets.io.getProxyUrl("http://target.example.com/image.gif"); > it will get result like > http://host/proxy/image.gif?url=http%3a%2f%2ftarget.example.com%2fimage.gif" + > "&refresh=3600" + > "&g=http%3a%2f%2fwww.gadget.com%2fgadget.xml" + > "&c=foo" > > It will not effect any existing code > > > This addresses bug shindig-1875. > https://issues.apache.org/jira/browse/shindig-1875 > > > Diffs > ----- > > http://svn.apache.org/repos/asf/shindig/trunk/config/container.js 1369611 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/feature.xml > 1383008 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js > 1395276 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/core.io/iotest.js > 1366998 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java > 1366998 > > Diff: https://reviews.apache.org/r/7309/diff/ > > > Testing > ------- > > Add Test case in iotest.js > > > Thanks, > > Erik Bi > >