You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@shindig.apache.org by jo...@gmail.com on 2011/07/26 23:21:42 UTC

Review Request: Don't inject ___jsl.u if there's no non-empty value to inject.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1198/
-----------------------------------------------------------

Review request for shindig and Ziv Horesh.


Summary
-------

Decline to inject ___jsl.u="" or "null" when an empty value is present.


Diffs
-----

  trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslInfoVariableProcessor.java 1145744 
  trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/AddJslInfoVariableProcessorTest.java 1145744 

Diff: https://reviews.apache.org/r/1198/diff


Testing
-------


Thanks,

johnfargo

Re: Review Request: Don't inject ___jsl.u if there's no non-empty value to inject.

Posted by Michael Hermanto <mh...@gmail.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1198/#review1192
-----------------------------------------------------------



trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslInfoVariableProcessor.java
<https://reviews.apache.org/r/1198/#comment2602>

    There is no StringUtils.isEmptyOrWhitespace() or alike that can be used here?


- Michael


On 2011-07-26 21:21:42, johnfargo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1198/
> -----------------------------------------------------------
> 
> (Updated 2011-07-26 21:21:42)
> 
> 
> Review request for shindig and Ziv Horesh.
> 
> 
> Summary
> -------
> 
> Decline to inject ___jsl.u="" or "null" when an empty value is present.
> 
> 
> Diffs
> -----
> 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslInfoVariableProcessor.java 1145744 
>   trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/AddJslInfoVariableProcessorTest.java 1145744 
> 
> Diff: https://reviews.apache.org/r/1198/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> johnfargo
> 
>

Re: Review Request: Don't inject ___jsl.u if there's no non-empty value to inject.

Posted by Michael Hermanto <mh...@gmail.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1198/#review1193
-----------------------------------------------------------

Ship it!


- Michael


On 2011-07-26 21:21:42, johnfargo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1198/
> -----------------------------------------------------------
> 
> (Updated 2011-07-26 21:21:42)
> 
> 
> Review request for shindig and Ziv Horesh.
> 
> 
> Summary
> -------
> 
> Decline to inject ___jsl.u="" or "null" when an empty value is present.
> 
> 
> Diffs
> -----
> 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslInfoVariableProcessor.java 1145744 
>   trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/AddJslInfoVariableProcessorTest.java 1145744 
> 
> Diff: https://reviews.apache.org/r/1198/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> johnfargo
> 
>

Re: Review Request: Don't inject ___jsl.u if there's no non-empty value to inject.

Posted by jo...@gmail.com.


> On 2011-07-26 22:01:08, Jasvir Nagra wrote:
> > trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslInfoVariableProcessor.java, line 63
> > <https://reviews.apache.org/r/1198/diff/1/?file=27184#file27184line63>
> >
> >     What prevents features from containing a ']' character?

Nothing in code, just convention. Features are expected to be properly written and trusted code, precluding issues of XSS.


- johnfargo


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1198/#review1190
-----------------------------------------------------------


On 2011-07-26 21:21:42, johnfargo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1198/
> -----------------------------------------------------------
> 
> (Updated 2011-07-26 21:21:42)
> 
> 
> Review request for shindig and Ziv Horesh.
> 
> 
> Summary
> -------
> 
> Decline to inject ___jsl.u="" or "null" when an empty value is present.
> 
> 
> Diffs
> -----
> 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslInfoVariableProcessor.java 1145744 
>   trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/AddJslInfoVariableProcessorTest.java 1145744 
> 
> Diff: https://reviews.apache.org/r/1198/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> johnfargo
> 
>

Re: Review Request: Don't inject ___jsl.u if there's no non-empty value to inject.

Posted by Jasvir Nagra <ja...@gmail.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1198/#review1190
-----------------------------------------------------------



trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslInfoVariableProcessor.java
<https://reviews.apache.org/r/1198/#comment2599>

    What prevents features from containing a ']' character?


- Jasvir


On 2011-07-26 21:21:42, johnfargo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1198/
> -----------------------------------------------------------
> 
> (Updated 2011-07-26 21:21:42)
> 
> 
> Review request for shindig and Ziv Horesh.
> 
> 
> Summary
> -------
> 
> Decline to inject ___jsl.u="" or "null" when an empty value is present.
> 
> 
> Diffs
> -----
> 
>   trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslInfoVariableProcessor.java 1145744 
>   trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/AddJslInfoVariableProcessorTest.java 1145744 
> 
> Diff: https://reviews.apache.org/r/1198/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> johnfargo
> 
>