Issue 434044: Loosen strictness of Uri parsing for Proxy and Concat managers

Issue 434044: Loosen strictness of Uri parsing for Proxy and Concat managers (Closed)

Can't Edit
Can't Publish+Mail
Start Review

Created:
15 years, 10 months ago by johnfargo

Modified:
15 years, 10 months ago

Reviewers:
henry.saputra, zhoresh, Paul Lindner, shindig.remailer, chirag

Base URL:
http://svn.apache.org/repos/asf/shindig/trunk/

Visibility:
Public.

Patch Set 1 #

Total comments: 3

Created: 15 years, 10 months ago

Download [raw] [tar.bz2]

Unified diffs	Side-by-side diffs	Stats (+61 lines, -11 lines)			Patch
java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java	View	3 chunks	+16 lines, -6 lines	2 comments	Download
java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultProxyUriManager.java	View	4 chunks	+8 lines, -1 line	1 comment	Download
java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManagerTest.java	View	2 chunks	+15 lines, -4 lines	0 comments	Download
java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultProxyUriManagerTest.java	View	1 chunk	+22 lines, -0 lines	0 comments	Download

Messages

Total messages: 5

Expand All Messages | Collapse All Messages

chirag

http://codereview.appspot.com/434044/diff/1/5 File java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java (right): http://codereview.appspot.com/434044/diff/1/5#newcode59 java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java:59: @Named("shindig.uri.concat.use-strict-parsing") Boolean useStrict) { Can we also define "shindig.uri.concat.use-strict-parsing" ...

15 years, 10 months ago (2010-03-13 00:15:23 UTC) #2

henry.saputra

http://codereview.appspot.com/434044/diff/1/5 File java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java (right): http://codereview.appspot.com/434044/diff/1/5#newcode60 java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java:60: this.strictParsing = useStrict; Instead of auto unbox maybe use ...

15 years, 10 months ago (2010-03-13 01:01:33 UTC) #3

Paul Lindner

seems okay, should add some docs and rationale why this is needed and in what ...

15 years, 10 months ago (2010-03-16 05:22:34 UTC) #4

johnfargo

15 years, 10 months ago (2010-03-16 22:37:30 UTC) #5

Thanks for all the feedback. I've addressed all by:

1. Adding default config values (set to false) in shindig.properties
2. Added documentation in shindig.properties with justification regarding
strict parsing
3. Correspondingly changed @Inject'ed values to Strings, converted to
Boolean via Boolean.parseBoolean(String)

Patch on the way.

On Mon, Mar 15, 2010 at 10:22 PM, <lindner@inuus.com> wrote:

> seems okay, should add some docs and rationale why this is needed and in
> what situation.
>
>
>
> http://codereview.appspot.com/434044/show
>

Expand All Messages | Collapse All Messages