JsonResourceWriter fix for possible DOS attack (3)

[ianboston, 2010-01-16 09:43:21]

Some minor comments on this patch, other than that LGTM.

http://codereview.appspot.com/186167/diff/1/4
File
bundles/servlets/get/src/main/java/org/apache/sling/servlets/get/impl/JsonQueryServlet.java
(right):

http://codereview.appspot.com/186167/diff/1/4#newcode199
bundles/servlets/get/src/main/java/org/apache/sling/servlets/get/impl/JsonQueryServlet.java:199:

this segment of the patch looks like a white space change?

http://codereview.appspot.com/186167/diff/1/5
File
bundles/servlets/get/src/main/java/org/apache/sling/servlets/get/impl/RedirectServlet.java
(left):

http://codereview.appspot.com/186167/diff/1/5#oldcode55
bundles/servlets/get/src/main/java/org/apache/sling/servlets/get/impl/RedirectServlet.java:55:
* @scr.component immediate="true" metatype="no"
was there a reason to remove metatype ?

http://codereview.appspot.com/186167/diff/1/2
File
bundles/servlets/get/src/main/java/org/apache/sling/servlets/get/impl/helpers/JsonRendererServlet.java
(right):

http://codereview.appspot.com/186167/diff/1/2#newcode110
bundles/servlets/get/src/main/java/org/apache/sling/servlets/get/impl/helpers/JsonRendererServlet.java:110:
allowedLevel = Integer.parseInt(e.getMessage());
since you have a specific exception why not have an int in the exception rather
than parsing ?

[simong, 2010-01-18 10:11:23]

I noticed this was still marked as draft and not send out.

http://codereview.appspot.com/186167/diff/1/4
File
bundles/servlets/get/src/main/java/org/apache/sling/servlets/get/impl/JsonQueryServlet.java
(right):

http://codereview.appspot.com/186167/diff/1/4#newcode199
bundles/servlets/get/src/main/java/org/apache/sling/servlets/get/impl/JsonQueryServlet.java:199:

On 2010/01/16 09:43:21, ianboston wrote:
> this segment of the patch looks like a white space change?

Yes, my bad, this can be ignored.

http://codereview.appspot.com/186167/diff/1/5
File
bundles/servlets/get/src/main/java/org/apache/sling/servlets/get/impl/RedirectServlet.java
(left):

http://codereview.appspot.com/186167/diff/1/5#oldcode55
bundles/servlets/get/src/main/java/org/apache/sling/servlets/get/impl/RedirectServlet.java:55:
* @scr.component immediate="true" metatype="no"
Yes, I added an OSGi property. If I didn't remove it, the scr plugin would not
recognize it.

On 2010/01/16 09:43:21, ianboston wrote:
> was there a reason to remove metatype ?

http://codereview.appspot.com/186167/diff/1/2
File
bundles/servlets/get/src/main/java/org/apache/sling/servlets/get/impl/helpers/JsonRendererServlet.java
(right):

http://codereview.appspot.com/186167/diff/1/2#newcode110
bundles/servlets/get/src/main/java/org/apache/sling/servlets/get/impl/helpers/JsonRendererServlet.java:110:
allowedLevel = Integer.parseInt(e.getMessage());
You're right, I should have modified the exception as well.

On 2010/01/16 09:43:21, ianboston wrote:
> since you have a specific exception why not have an int in the exception
rather
> than parsing ?