Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix AttributeError on shortcut responses
Added in the single 'is enabled' check from #150 / 7c6cfaa.
  • Loading branch information
suligap authored and Adam Chainz committed Nov 9, 2016
1 parent 68bbee5 commit 762591a
Show file tree
Hide file tree
Showing 6 changed files with 133 additions and 37 deletions.
6 changes: 6 additions & 0 deletions HISTORY.rst
Expand Up @@ -5,6 +5,12 @@ Pending
-------

* New release notes go here.
* Fix a bug with the single check if CORS enabled added in 1.3.0: on Django
< 1.10 shortcut responses could be generated by middleware above
``CorsMiddleware``, before it processed the request, failing with an
``AttributeError`` for ``request._cors_enabled``. Also clarified the docs
that ``CorsMiddleware`` should be kept as high as possible in your middleware
stack, above any middleware that can generate such responses.

1.3.0 (2016-11-06)
------------------
Expand Down
13 changes: 8 additions & 5 deletions README.rst
Expand Up @@ -44,17 +44,20 @@ You will also need to add a middleware class to listen in on responses:

.. code-block:: python
MIDDLEWARE_CLASSES = [
MIDDLEWARE = [ # Or MIDDLEWARE_CLASSES on Django < 1.10
...
'corsheaders.middleware.CorsMiddleware',
'django.middleware.common.CommonMiddleware',
...
]
Note that ``CorsMiddleware`` needs to come before Django's
``CommonMiddleware`` if you are using Django's ``USE_ETAGS = True``
setting, otherwise the CORS headers will be lost from 304 Not-Modified
responses, causing errors in some browsers.
``CorsMiddleware`` should be placed as high as possible, especially before any
middleware that can generate responses such as Django's ``CommonMiddleware`` or
Whitenoise's ``WhiteNoiseMiddleware``. If it is not before, it will not be able
to add the CORS headers to these responses.

Also if you are using ``CORS_REPLACE_HTTPS_REFERER`` it should be placed before
Django's ``CsrfViewMiddleware`` (see more below).

Configuration
-------------
Expand Down
65 changes: 37 additions & 28 deletions corsheaders/middleware.py
Expand Up @@ -103,39 +103,48 @@ def process_response(self, request, response):
Add the respective CORS headers
"""
origin = request.META.get('HTTP_ORIGIN')
if request._cors_enabled and origin:
# todo: check hostname from db instead
url = urlparse(origin)

if conf.CORS_MODEL is not None:
model = apps.get_model(*conf.CORS_MODEL.split('.'))
if model.objects.filter(cors=url.netloc).exists():
response[ACCESS_CONTROL_ALLOW_ORIGIN] = origin
if not origin:
return response

if (
not conf.CORS_ORIGIN_ALLOW_ALL and
self.origin_not_found_in_white_lists(origin, url) and
not self.check_signal(request)
):
return response
enabled = getattr(request, '_cors_enabled', None)
if enabled is None:
enabled = self.is_enabled(request)

if conf.CORS_ORIGIN_ALLOW_ALL and not conf.CORS_ALLOW_CREDENTIALS:
response[ACCESS_CONTROL_ALLOW_ORIGIN] = "*"
else:
response[ACCESS_CONTROL_ALLOW_ORIGIN] = origin
patch_vary_headers(response, ['Origin'])
if not enabled:
return response

if len(conf.CORS_EXPOSE_HEADERS):
response[ACCESS_CONTROL_EXPOSE_HEADERS] = ', '.join(conf.CORS_EXPOSE_HEADERS)
# todo: check hostname from db instead
url = urlparse(origin)

if conf.CORS_ALLOW_CREDENTIALS:
response[ACCESS_CONTROL_ALLOW_CREDENTIALS] = 'true'
if conf.CORS_MODEL is not None:
model = apps.get_model(*conf.CORS_MODEL.split('.'))
if model.objects.filter(cors=url.netloc).exists():
response[ACCESS_CONTROL_ALLOW_ORIGIN] = origin

if request.method == 'OPTIONS':
response[ACCESS_CONTROL_ALLOW_HEADERS] = ', '.join(conf.CORS_ALLOW_HEADERS)
response[ACCESS_CONTROL_ALLOW_METHODS] = ', '.join(conf.CORS_ALLOW_METHODS)
if conf.CORS_PREFLIGHT_MAX_AGE:
response[ACCESS_CONTROL_MAX_AGE] = conf.CORS_PREFLIGHT_MAX_AGE
if (
not conf.CORS_ORIGIN_ALLOW_ALL and
self.origin_not_found_in_white_lists(origin, url) and
not self.check_signal(request)
):
return response

if conf.CORS_ORIGIN_ALLOW_ALL and not conf.CORS_ALLOW_CREDENTIALS:
response[ACCESS_CONTROL_ALLOW_ORIGIN] = "*"
else:
response[ACCESS_CONTROL_ALLOW_ORIGIN] = origin
patch_vary_headers(response, ['Origin'])

if len(conf.CORS_EXPOSE_HEADERS):
response[ACCESS_CONTROL_EXPOSE_HEADERS] = ', '.join(conf.CORS_EXPOSE_HEADERS)

if conf.CORS_ALLOW_CREDENTIALS:
response[ACCESS_CONTROL_ALLOW_CREDENTIALS] = 'true'

if request.method == 'OPTIONS':
response[ACCESS_CONTROL_ALLOW_HEADERS] = ', '.join(conf.CORS_ALLOW_HEADERS)
response[ACCESS_CONTROL_ALLOW_METHODS] = ', '.join(conf.CORS_ALLOW_METHODS)
if conf.CORS_PREFLIGHT_MAX_AGE:
response[ACCESS_CONTROL_MAX_AGE] = conf.CORS_PREFLIGHT_MAX_AGE

return response

Expand Down
67 changes: 65 additions & 2 deletions tests/test_middleware.py
@@ -1,13 +1,30 @@
import django
from django.http import HttpResponse
from django.test import TestCase
from django.test.utils import override_settings

from corsheaders.middleware import (
ACCESS_CONTROL_ALLOW_CREDENTIALS, ACCESS_CONTROL_ALLOW_HEADERS, ACCESS_CONTROL_ALLOW_METHODS,
ACCESS_CONTROL_ALLOW_ORIGIN, ACCESS_CONTROL_EXPOSE_HEADERS, ACCESS_CONTROL_MAX_AGE
ACCESS_CONTROL_ALLOW_ORIGIN, ACCESS_CONTROL_EXPOSE_HEADERS, ACCESS_CONTROL_MAX_AGE,
)
from corsheaders.models import CorsModel

from .utils import append_middleware, temporary_check_request_hander
from .utils import (
append_middleware,
prepend_middleware,
temporary_check_request_hander,
)

try:
from django.utils.deprecation import MiddlewareMixin
except ImportError: # pragma: no cover
MiddlewareMixin = object # pragma: no cover


class ShortCircuitMiddleware(MiddlewareMixin):

def process_request(self, request):
return HttpResponse('short-circuit-middleware-response')


class CorsMiddlewareTests(TestCase):
Expand Down Expand Up @@ -249,6 +266,52 @@ def allow_all(sender, request, **kwargs):

assert allow_all.calls == 1

@override_settings(CORS_ORIGIN_WHITELIST=['example.com'])
@prepend_middleware('tests.test_middleware.ShortCircuitMiddleware')
def test_get_short_circuit(self):
"""
Test a scenario when a middleware that returns a response is run before
the ``CorsMiddleware``. In this case
``CorsMiddleware.process_response()`` should ignore the request if
MIDDLEWARE setting is used (new mechanism in Django 1.10+) and process
the request and add CORS response headers if MIDDLEWARE_CLASSES is
used in a backward compatible fashion with django-cors-headers pre 1.3.0.
"""
resp = self.client.get('/', HTTP_ORIGIN='http://example.com')
if django.VERSION[:2] >= (1, 10):
assert ACCESS_CONTROL_ALLOW_ORIGIN not in resp
else:
assert ACCESS_CONTROL_ALLOW_ORIGIN in resp

@override_settings(
CORS_URLS_REGEX=r'^/foo$',
CORS_ORIGIN_WHITELIST=['example.com'],
)
@prepend_middleware('tests.test_middleware.ShortCircuitMiddleware')
def test_get_short_circuit_no_origin(self):
resp = self.client.get('/', HTTP_ORIGIN='http://example.com')
assert ACCESS_CONTROL_ALLOW_ORIGIN not in resp

@override_settings(
CORS_URLS_REGEX=r'^/foo$',
CORS_ORIGIN_WHITELIST=['example.com'],
)
def test_get_no_origin_not_enabled(self):
resp = self.client.get('/', HTTP_ORIGIN='http://example.com')
assert ACCESS_CONTROL_ALLOW_ORIGIN not in resp

@override_settings(CORS_ORIGIN_WHITELIST=['example.com'])
def test_works_if_view_deletes_is_enabled(self):
"""
Just in case something crazy happens in the view or other middleware,
check that get_response doesn't fall over if `is_enabled` is removed
"""
resp = self.client.get(
'/delete-is-enabled/',
HTTP_ORIGIN='http://example.com',
)
assert ACCESS_CONTROL_ALLOW_ORIGIN in resp


@override_settings(
CORS_REPLACE_HTTPS_REFERER=True,
Expand Down
7 changes: 7 additions & 0 deletions tests/urls.py
Expand Up @@ -12,7 +12,14 @@ def test_view_http401(request):
return HttpResponse('Unauthorized', status=401)


def test_view_that_deletes_is_enabled(request):
if hasattr(request, '_cors_enabled'):
del request._cors_enabled
return HttpResponse()


urlpatterns = [
url(r'^$', test_view, name='test-view'),
url(r'^test-401/$', test_view_http401),
url(r'^delete-is-enabled/$', test_view_that_deletes_is_enabled),
]
12 changes: 10 additions & 2 deletions tests/utils.py
Expand Up @@ -6,19 +6,27 @@
from corsheaders.signals import check_request_enabled


def append_middleware(path):
def add_middleware(action, path):
if django.VERSION[:2] >= (1, 10):
middleware_setting = 'MIDDLEWARE'
else:
middleware_setting = 'MIDDLEWARE_CLASSES'

return modify_settings(**{
middleware_setting: {
'append': path
action: path,
}
})


def append_middleware(path):
return add_middleware('append', path)


def prepend_middleware(path):
return add_middleware('prepend', path)


@contextmanager
def temporary_check_request_hander(handler):
check_request_enabled.connect(handler)
Expand Down

0 comments on commit 762591a

Please sign in to comment.