Thanks Ziv -- Responses inline. On Fri, Mar 5, 2010 at 6:20 PM, <[email protected]> wrote:
> LGTM > > Minor comments > > > http://codereview.appspot.com/224098/diff/2001/3001 > File > > > java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/AbsolutePathLinkRewriter.java > (right): > > http://codereview.appspot.com/224098/diff/2001/3001#newcode58 > > java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/AbsolutePathLinkRewriter.java:58: > } catch (Exception e) { > Try to catch the specific exception you expect here. > Other exception might be sign for bugs to fix? > Good call. Caught Uri.UriException instead, which is what was intended in the first place. > > http://codereview.appspot.com/224098/diff/2001/3002 > File > > > java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/AbsolutePathLinkRewriterTest.java > (right): > > http://codereview.appspot.com/224098/diff/2001/3002#newcode37 > > java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/AbsolutePathLinkRewriterTest.java:37: > private static final Uri RELATIVE_URI = Uri.parse("/host/relative"); > Test also a local refeence (no /) > Or same schema ("//a.com/a/b") > Added path-relative testing as well. > And a bad element ("<a/>") Added test to ensure that such an element is bypassed. > > > http://codereview.appspot.com/224098/show >
