Closed sriddell closed 11 years ago
I just published 1.1.0 with your changes. Thanks.
On Wed, Apr 24, 2013 at 5:43 PM, sriddell notifications@github.com wrote:
I'd like to thank you for this plugin; I was hitting the grails bug that wouldn't allow OPTIONS methods through to my controller when I found your solution.
For the work I'm doing, I needed to support the Access-Control-Expose-Headers in the response; this pull request contains my simple solution to add support as well as tighten the origin checking to be closer to my reading of the W3C spec, along with additional unit tests.
I'd greatly appreciate any feedback/discussion on the proposed changes (whether you agree with them or not). If you disagree with them, or would like to see them implemented differently, please let me know, as I'd much prefer to see the feature added to your plugin than to maintain my own fork.
The only potential non-backward compatible change I have introduced is that I am checking that the Origin header exists and is a match on all requests (per my read of the W3C CORS spec), where as previously, the filter would add CORS headers to the response for any request if the origin regex was not being used. The filter will now always set the value of the Origin header (if accepted as a match) in the Access-Control-Allow-Origin response header. (Again, my read of the W3C spec that requires that behavior when returning Access-Control-Allow-Credentials with a true value.)
Note that I set the version to 1.0.5-SNAPSHOT for my own development.
Looking forward to your comments.
Thanks again for the plugin, Shane Riddell
You can merge this Pull Request by running
git pull https://github.com/sriddell/grails-cors expose-headers
Or view, comment on, or merge it at:
https://github.com/davidtinker/grails-cors/pull/10
Commit Summary
Added support for Access-Control-Expose-Headers in the response.
File Changes
M CorsGrailsPlugin.groovy (8) M README.md (9) M src/java/com/brandseye/cors/CorsFilter.java (56) M test/unit/com/brandseye/cors/CorsFilterTest.groovy (57)
Patch Links:
https://github.com/davidtinker/grails-cors/pull/10.patch https://github.com/davidtinker/grails-cors/pull/10.diff
http://twoogleplus.com/ Online service to cross-post tweets to Google+ http://engage.calibreapps.com/ Engage - Web based goal setting and performance management
I'd like to thank you for this plugin; I was hitting the grails bug that wouldn't allow OPTIONS methods through to my controller when I found your solution.
For the work I'm doing, I needed to support the Access-Control-Expose-Headers in the response; this pull request contains my simple solution to add support as well as tighten the origin checking to be closer to my reading of the W3C spec, along with additional unit tests.
I'd greatly appreciate any feedback/discussion on the proposed changes (whether you agree with them or not). If you disagree with them, or would like to see them implemented differently, please let me know, as I'd much prefer to see the feature added to your plugin than to maintain my own fork.
The only potential non-backward compatible change I have introduced is that I am checking that the Origin header exists and is a match on all requests (per my read of the W3C CORS spec), where as previously, the filter would add CORS headers to the response for any request if the origin regex was not being used. The filter will now always set the value of the Origin header (if accepted as a match) in the Access-Control-Allow-Origin response header. (Again, my read of the W3C spec that requires that behavior when returning Access-Control-Allow-Credentials with a true value.)
Note that I set the version to 1.0.5-SNAPSHOT for my own development.
Looking forward to your comments.
Thanks again for the plugin, Shane Riddell