You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Mike Pawlowski <mp...@ca.ibm.com> on 2013/09/30 20:02:39 UTC

Review Request 14406: Review Request: JSON RPC servlet does not support CORS properly

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

Review request for shindig.


Bugs: https://issues.apache.org/jira/browse/SHINDIG-1927
    https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/SHINDIG-1927


Repository: shindig


Description
-------

Attached a patch to implement full CORS support in Apache Shindig via the open source CORS servlet filter:

Name: CORS Filter
Version: 1.7.1
Homepage: http://software.dzhuvinov.com/cors-filter.html
License(s): Apache License, Version 2.0 (License link is broken)
Downloaded From: http://search.maven.org/#browse%7C540685910
Notes: N/A

Name: Java Property Utility 
Version: 1.9
Homepage: http://software.dzhuvinov.com/cors-filter-installation.html
License(s): Apache License, Version 2.0 (License link is broken)
Downloaded From: http://search.maven.org/#browse%7C-89813813
Notes: Required dependency for "CORS Filter"

See JIRA issue for details: https://issues.apache.org/jira/browse/SHINDIG-1927

Please note that this patch relies on the patch attached to the following JIRA issue:
Remove partial implementation of CORS support
https://issues.apache.org/jira/browse/SHINDIG-1934


Diffs
-----

  http://svn.apache.org/repos/asf/shindig/trunk/java/server-resources/pom.xml 1526722 
  http://svn.apache.org/repos/asf/shindig/trunk/java/server-resources/src/main/webapp/WEB-INF/web.xml 1526722 

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


Testing
-------

* Build was successful (except for a few unrelated build hiccups)
* Manual testing was successful
  => Test environment: 
        - (I) Shindig server deployed as a stand alone app hosted on its own domain; 
        - (II) Common container utilized by another app hosted on its own domain
  => Cross domain POST HTTP request was successful
      - e.g. http://localhost:9082/rpc?st=ownerId%3AviewerId%3Aappid%3Ashindig%3Aurl%3A0%3Adefault


Thanks,

Mike Pawlowski


Re: Review Request 14406: Review Request: JSON RPC servlet does not support CORS properly

Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14406/#review26550
-----------------------------------------------------------



http://svn.apache.org/repos/asf/shindig/trunk/pom.xml
<https://reviews.apache.org/r/14406/#comment51792>

    Minor formatting thing....are these tabs?  If so they should be spaces.


- Ryan Baxter


On Oct. 1, 2013, 2:25 a.m., Mike Pawlowski wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14406/
> -----------------------------------------------------------
> 
> (Updated Oct. 1, 2013, 2:25 a.m.)
> 
> 
> Review request for shindig.
> 
> 
> Bugs: https://issues.apache.org/jira/browse/SHINDIG-1927
>     https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/SHINDIG-1927
> 
> 
> Repository: shindig
> 
> 
> Description
> -------
> 
> Attached a patch to implement full CORS support in Apache Shindig via the open source CORS servlet filter:
> 
> Name: CORS Filter
> Version: 1.7.1
> Homepage: http://software.dzhuvinov.com/cors-filter.html
> License(s): Apache License, Version 2.0 (License link is broken)
> Downloaded From: http://search.maven.org/#browse%7C540685910
> Notes: N/A
> 
> Name: Java Property Utility 
> Version: 1.9
> Homepage: http://software.dzhuvinov.com/cors-filter-installation.html
> License(s): Apache License, Version 2.0 (License link is broken)
> Downloaded From: http://search.maven.org/#browse%7C-89813813
> Notes: Required dependency for "CORS Filter"
> 
> See JIRA issue for details: https://issues.apache.org/jira/browse/SHINDIG-1927
> 
> Please note that this patch relies on the patch attached to the following JIRA issue:
> Remove partial implementation of CORS support
> https://issues.apache.org/jira/browse/SHINDIG-1934
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/server-resources/src/main/webapp/WEB-INF/web.xml 1527855 
>   http://svn.apache.org/repos/asf/shindig/trunk/pom.xml 1527855 
> 
> Diff: https://reviews.apache.org/r/14406/diff/
> 
> 
> Testing
> -------
> 
> * Build was successful (except for a few unrelated build hiccups)
> * Manual testing was successful
>   => Test environment: 
>         - (I) Shindig server deployed as a stand alone app hosted on its own domain; 
>         - (II) Common container utilized by another app hosted on its own domain
>   => Cross domain POST HTTP request was successful
>       - e.g. http://localhost:9082/rpc?st=ownerId%3AviewerId%3Aappid%3Ashindig%3Aurl%3A0%3Adefault
> 
> 
> Thanks,
> 
> Mike Pawlowski
> 
>


Re: Review Request 14406: Review Request: JSON RPC servlet does not support CORS properly

Posted by Ryan Baxter <rb...@gmail.com>.

> On Oct. 1, 2013, 1:18 p.m., Rich Thompson wrote:
> > I think the default should be the CORS support is included, but disabled until the security issues noted are addressed.

The security issues being the fact that the configuration in the web.xml is too open?


- Ryan


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


On Oct. 1, 2013, 2:25 a.m., Mike Pawlowski wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14406/
> -----------------------------------------------------------
> 
> (Updated Oct. 1, 2013, 2:25 a.m.)
> 
> 
> Review request for shindig.
> 
> 
> Bugs: https://issues.apache.org/jira/browse/SHINDIG-1927
>     https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/SHINDIG-1927
> 
> 
> Repository: shindig
> 
> 
> Description
> -------
> 
> Attached a patch to implement full CORS support in Apache Shindig via the open source CORS servlet filter:
> 
> Name: CORS Filter
> Version: 1.7.1
> Homepage: http://software.dzhuvinov.com/cors-filter.html
> License(s): Apache License, Version 2.0 (License link is broken)
> Downloaded From: http://search.maven.org/#browse%7C540685910
> Notes: N/A
> 
> Name: Java Property Utility 
> Version: 1.9
> Homepage: http://software.dzhuvinov.com/cors-filter-installation.html
> License(s): Apache License, Version 2.0 (License link is broken)
> Downloaded From: http://search.maven.org/#browse%7C-89813813
> Notes: Required dependency for "CORS Filter"
> 
> See JIRA issue for details: https://issues.apache.org/jira/browse/SHINDIG-1927
> 
> Please note that this patch relies on the patch attached to the following JIRA issue:
> Remove partial implementation of CORS support
> https://issues.apache.org/jira/browse/SHINDIG-1934
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/server-resources/src/main/webapp/WEB-INF/web.xml 1527855 
>   http://svn.apache.org/repos/asf/shindig/trunk/pom.xml 1527855 
> 
> Diff: https://reviews.apache.org/r/14406/diff/
> 
> 
> Testing
> -------
> 
> * Build was successful (except for a few unrelated build hiccups)
> * Manual testing was successful
>   => Test environment: 
>         - (I) Shindig server deployed as a stand alone app hosted on its own domain; 
>         - (II) Common container utilized by another app hosted on its own domain
>   => Cross domain POST HTTP request was successful
>       - e.g. http://localhost:9082/rpc?st=ownerId%3AviewerId%3Aappid%3Ashindig%3Aurl%3A0%3Adefault
> 
> 
> Thanks,
> 
> Mike Pawlowski
> 
>


Re: Review Request 14406: Review Request: JSON RPC servlet does not support CORS properly

Posted by Rich Thompson <ri...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14406/#review26551
-----------------------------------------------------------


I think the default should be the CORS support is included, but disabled until the security issues noted are addressed.

- Rich Thompson


On Oct. 1, 2013, 2:25 a.m., Mike Pawlowski wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14406/
> -----------------------------------------------------------
> 
> (Updated Oct. 1, 2013, 2:25 a.m.)
> 
> 
> Review request for shindig.
> 
> 
> Bugs: https://issues.apache.org/jira/browse/SHINDIG-1927
>     https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/SHINDIG-1927
> 
> 
> Repository: shindig
> 
> 
> Description
> -------
> 
> Attached a patch to implement full CORS support in Apache Shindig via the open source CORS servlet filter:
> 
> Name: CORS Filter
> Version: 1.7.1
> Homepage: http://software.dzhuvinov.com/cors-filter.html
> License(s): Apache License, Version 2.0 (License link is broken)
> Downloaded From: http://search.maven.org/#browse%7C540685910
> Notes: N/A
> 
> Name: Java Property Utility 
> Version: 1.9
> Homepage: http://software.dzhuvinov.com/cors-filter-installation.html
> License(s): Apache License, Version 2.0 (License link is broken)
> Downloaded From: http://search.maven.org/#browse%7C-89813813
> Notes: Required dependency for "CORS Filter"
> 
> See JIRA issue for details: https://issues.apache.org/jira/browse/SHINDIG-1927
> 
> Please note that this patch relies on the patch attached to the following JIRA issue:
> Remove partial implementation of CORS support
> https://issues.apache.org/jira/browse/SHINDIG-1934
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/server-resources/src/main/webapp/WEB-INF/web.xml 1527855 
>   http://svn.apache.org/repos/asf/shindig/trunk/pom.xml 1527855 
> 
> Diff: https://reviews.apache.org/r/14406/diff/
> 
> 
> Testing
> -------
> 
> * Build was successful (except for a few unrelated build hiccups)
> * Manual testing was successful
>   => Test environment: 
>         - (I) Shindig server deployed as a stand alone app hosted on its own domain; 
>         - (II) Common container utilized by another app hosted on its own domain
>   => Cross domain POST HTTP request was successful
>       - e.g. http://localhost:9082/rpc?st=ownerId%3AviewerId%3Aappid%3Ashindig%3Aurl%3A0%3Adefault
> 
> 
> Thanks,
> 
> Mike Pawlowski
> 
>


Re: Review Request 14406: Review Request: JSON RPC servlet does not support CORS properly

Posted by Mike Pawlowski <mp...@ca.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14406/
-----------------------------------------------------------

(Updated Oct. 1, 2013, 2:25 a.m.)


Review request for shindig.


Changes
-------

A couple of errors on my part:
(a) I created the patch from an Eclipse workspace with Shindig checked out as a Maven project
    => Resulted in a patch with improper relative file paths.
(b) I attempted to resolve the web.xml change in the patch by specifying "http://svn.apache.org/repos/asf/shindig/trunk/java/server-resources/" as the "Base Directory" when uploading the patch to the review server.

The fix was to (a) Create a new Eclipse workspace with Shindig checked out in the normal SVN manner; (b) Apply my CORS filter changes to the workspace; (c) Re-create the patch (to generate the proper relative file paths).

Hopefully the diff file contents resolve properly against the repository this time.


Bugs: https://issues.apache.org/jira/browse/SHINDIG-1927
    https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/SHINDIG-1927


Repository: shindig


Description
-------

Attached a patch to implement full CORS support in Apache Shindig via the open source CORS servlet filter:

Name: CORS Filter
Version: 1.7.1
Homepage: http://software.dzhuvinov.com/cors-filter.html
License(s): Apache License, Version 2.0 (License link is broken)
Downloaded From: http://search.maven.org/#browse%7C540685910
Notes: N/A

Name: Java Property Utility 
Version: 1.9
Homepage: http://software.dzhuvinov.com/cors-filter-installation.html
License(s): Apache License, Version 2.0 (License link is broken)
Downloaded From: http://search.maven.org/#browse%7C-89813813
Notes: Required dependency for "CORS Filter"

See JIRA issue for details: https://issues.apache.org/jira/browse/SHINDIG-1927

Please note that this patch relies on the patch attached to the following JIRA issue:
Remove partial implementation of CORS support
https://issues.apache.org/jira/browse/SHINDIG-1934


Diffs (updated)
-----

  http://svn.apache.org/repos/asf/shindig/trunk/java/server-resources/src/main/webapp/WEB-INF/web.xml 1527855 
  http://svn.apache.org/repos/asf/shindig/trunk/pom.xml 1527855 

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


Testing
-------

* Build was successful (except for a few unrelated build hiccups)
* Manual testing was successful
  => Test environment: 
        - (I) Shindig server deployed as a stand alone app hosted on its own domain; 
        - (II) Common container utilized by another app hosted on its own domain
  => Cross domain POST HTTP request was successful
      - e.g. http://localhost:9082/rpc?st=ownerId%3AviewerId%3Aappid%3Ashindig%3Aurl%3A0%3Adefault


Thanks,

Mike Pawlowski


Re: Review Request 14406: Review Request: JSON RPC servlet does not support CORS properly

Posted by Ryan Baxter <rb...@gmail.com>.

> On Sept. 30, 2013, 9:21 p.m., Henry Saputra wrote:
> > Is this a complete patch? I am seeing "com.thetransactioncompany.cors.CORSFilter" in the web.xml changes as part of the diff

Looks like the patch for the pom.xml isn't right.


- Ryan


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


On Sept. 30, 2013, 6:02 p.m., Mike Pawlowski wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14406/
> -----------------------------------------------------------
> 
> (Updated Sept. 30, 2013, 6:02 p.m.)
> 
> 
> Review request for shindig.
> 
> 
> Bugs: https://issues.apache.org/jira/browse/SHINDIG-1927
>     https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/SHINDIG-1927
> 
> 
> Repository: shindig
> 
> 
> Description
> -------
> 
> Attached a patch to implement full CORS support in Apache Shindig via the open source CORS servlet filter:
> 
> Name: CORS Filter
> Version: 1.7.1
> Homepage: http://software.dzhuvinov.com/cors-filter.html
> License(s): Apache License, Version 2.0 (License link is broken)
> Downloaded From: http://search.maven.org/#browse%7C540685910
> Notes: N/A
> 
> Name: Java Property Utility 
> Version: 1.9
> Homepage: http://software.dzhuvinov.com/cors-filter-installation.html
> License(s): Apache License, Version 2.0 (License link is broken)
> Downloaded From: http://search.maven.org/#browse%7C-89813813
> Notes: Required dependency for "CORS Filter"
> 
> See JIRA issue for details: https://issues.apache.org/jira/browse/SHINDIG-1927
> 
> Please note that this patch relies on the patch attached to the following JIRA issue:
> Remove partial implementation of CORS support
> https://issues.apache.org/jira/browse/SHINDIG-1934
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/server-resources/pom.xml 1526722 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/server-resources/src/main/webapp/WEB-INF/web.xml 1526722 
> 
> Diff: https://reviews.apache.org/r/14406/diff/
> 
> 
> Testing
> -------
> 
> * Build was successful (except for a few unrelated build hiccups)
> * Manual testing was successful
>   => Test environment: 
>         - (I) Shindig server deployed as a stand alone app hosted on its own domain; 
>         - (II) Common container utilized by another app hosted on its own domain
>   => Cross domain POST HTTP request was successful
>       - e.g. http://localhost:9082/rpc?st=ownerId%3AviewerId%3Aappid%3Ashindig%3Aurl%3A0%3Adefault
> 
> 
> Thanks,
> 
> Mike Pawlowski
> 
>


Re: Review Request 14406: Review Request: JSON RPC servlet does not support CORS properly

Posted by Henry Saputra <hs...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14406/#review26513
-----------------------------------------------------------


Is this a complete patch? I am seeing "com.thetransactioncompany.cors.CORSFilter" in the web.xml changes as part of the diff

- Henry Saputra


On Sept. 30, 2013, 6:02 p.m., Mike Pawlowski wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14406/
> -----------------------------------------------------------
> 
> (Updated Sept. 30, 2013, 6:02 p.m.)
> 
> 
> Review request for shindig.
> 
> 
> Bugs: https://issues.apache.org/jira/browse/SHINDIG-1927
>     https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/SHINDIG-1927
> 
> 
> Repository: shindig
> 
> 
> Description
> -------
> 
> Attached a patch to implement full CORS support in Apache Shindig via the open source CORS servlet filter:
> 
> Name: CORS Filter
> Version: 1.7.1
> Homepage: http://software.dzhuvinov.com/cors-filter.html
> License(s): Apache License, Version 2.0 (License link is broken)
> Downloaded From: http://search.maven.org/#browse%7C540685910
> Notes: N/A
> 
> Name: Java Property Utility 
> Version: 1.9
> Homepage: http://software.dzhuvinov.com/cors-filter-installation.html
> License(s): Apache License, Version 2.0 (License link is broken)
> Downloaded From: http://search.maven.org/#browse%7C-89813813
> Notes: Required dependency for "CORS Filter"
> 
> See JIRA issue for details: https://issues.apache.org/jira/browse/SHINDIG-1927
> 
> Please note that this patch relies on the patch attached to the following JIRA issue:
> Remove partial implementation of CORS support
> https://issues.apache.org/jira/browse/SHINDIG-1934
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/server-resources/pom.xml 1526722 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/server-resources/src/main/webapp/WEB-INF/web.xml 1526722 
> 
> Diff: https://reviews.apache.org/r/14406/diff/
> 
> 
> Testing
> -------
> 
> * Build was successful (except for a few unrelated build hiccups)
> * Manual testing was successful
>   => Test environment: 
>         - (I) Shindig server deployed as a stand alone app hosted on its own domain; 
>         - (II) Common container utilized by another app hosted on its own domain
>   => Cross domain POST HTTP request was successful
>       - e.g. http://localhost:9082/rpc?st=ownerId%3AviewerId%3Aappid%3Ashindig%3Aurl%3A0%3Adefault
> 
> 
> Thanks,
> 
> Mike Pawlowski
> 
>