launchpad-reviewers team mailing list archive

[Robert Collins <robertc@xxxxxxxxxxxxxxxxx>]

Robert Collins has proposed merging lp:~lifeless/js-oopsd/post into lp:js-oopsd.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~lifeless/js-oopsd/post/+merge/123474

keep browsers happier - implement CORS rules to permit any browser to use
POST or GET, and permit preflight checks.
-- 
https://code.launchpad.net/~lifeless/js-oopsd/post/+merge/123474
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/js-oopsd/post into lp:js-oopsd.

=== modified file 'NEWS'
--- NEWS	2012-09-09 04:08:16 +0000
+++ NEWS	2012-09-10 04:21:20 +0000
@@ -7,3 +7,6 @@
 ----
 
 * GET and POST support added.
+
+* CORS headers added to the responses, and CORS preflight OPTIONS check added.
+  (Robert Collins)

=== modified file 'js_oopsd/main.py'
--- js_oopsd/main.py	2012-09-09 04:08:16 +0000
+++ js_oopsd/main.py	2012-09-10 04:21:20 +0000
@@ -29,6 +29,14 @@
 import oops_wsgi 
 from paste.request import parse_formvars
 
+cors_headers = (
+    ('Access-Control-Allow-Origin', '*'),
+    ('Access-Control-Max-Age', '3628800'),
+    ('Access-Control-Allow-Methods', 'PUT, DELETE'),
+    ('Access-Control-Expose-Headers', 'X-OOPS-ID'),
+    )
+
+
 def make_app(jsoops_config):
     def app(environ, start_response):
         # NB: probably want oops on_create hooks and to create the context, not the
@@ -53,6 +61,9 @@
         elif method == 'GET':
             query = environ.get('QUERY_STRING', '')
             json_content = unquote(query)
+        elif method == 'OPTIONS':
+            start_response('200 OK', list(cors_headers))
+            return
         else:
             # Not supported
             start_response('405 Method Not Allowed', [('Allow', 'GET, POST')])
@@ -77,7 +88,8 @@
         # future.
         report.update(report_extra)
         jsoops_config.publish(report)
-        start_response('200 OK', [('X-OOPS-ID', str(report['id']))])
+        start_response('200 OK',
+            list(cors_headers) + [('X-OOPS-ID', str(report['id']))])
         yield ''
     return app
 

=== modified file 'js_oopsd/tests/test_main.py'
--- js_oopsd/tests/test_main.py	2012-09-09 04:08:16 +0000
+++ js_oopsd/tests/test_main.py	2012-09-10 04:21:20 +0000
@@ -20,7 +20,11 @@
 from testtools import TestCase
 from testtools.matchers import Equals
 
-from js_oopsd.main import make_app
+from js_oopsd.main import (
+    cors_headers,
+    make_app,
+    )
+
 
 def capture():
     results = []
@@ -56,7 +60,19 @@
         self.assertThat(output, Equals([]))
         self.assertThat(events,
             Equals([('405 Method Not Allowed', [('Allow', 'GET, POST')])]))
-    
+
+    def test_options(self):
+        # CORS requires us to tell browsers they are allowed to use the
+        # service.
+        app, events, start_response = test_app()
+        environ = {
+            'PATH_INFO': '/',
+            'REQUEST_METHOD': 'OPTIONS',
+            }
+        output = list(app(environ, start_response))
+        self.assertThat(output, Equals([]))
+        self.assertThat(events, Equals([('200 OK', list(cors_headers))]))
+
     def test_post_empty(self):
         # Empty generates a error response.
         app, events, start_response = test_app()
@@ -97,5 +113,5 @@
         self.assertThat(output, Equals(['']))
         self.assertThat(events,
             Equals([{'id': '1', u'reporter': u'fred'},
-                    ('200 OK', [('X-OOPS-ID', '1')])]))
+                    ('200 OK', list(cors_headers) + [('X-OOPS-ID', '1')])]))