Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix double firing of check_request_enabled #228

Open
adamchainz opened this issue May 15, 2017 · 2 comments
Open

Fix double firing of check_request_enabled #228

adamchainz opened this issue May 15, 2017 · 2 comments

Comments

@adamchainz
Copy link
Owner

The check_request_enabled signal can fire twice during process_response, but really we should only do it once. Handlers could be expensive, e.g. doing database queries, and there's no reason they should be expected to cache their results.

Reported by @MarkusH.

@adamchainz adamchainz self-assigned this May 15, 2017
@adamchainz adamchainz removed their assignment Sep 12, 2017
@romantomjak
Copy link

romantomjak commented Nov 4, 2017

Correct me if I'm wrong, but I think it already executes just once.

Before process_response Django runs process_request which sets therequest._cors_enabled and that makes these lines redundant, because it will always be set and the "second" call to self.is_enabled(request) will not be made. Or am I missing something blatantly obvious?

@jaap3
Copy link
Contributor

jaap3 commented Sep 21, 2021

The signal fires each time CorsMiddleware.check_signal is called:

def check_signal(self, request: HttpRequest) -> bool:
signal_responses = check_request_enabled.send(sender=None, request=request)
return any(return_value for function, return_value in signal_responses)

The CorsMiddleware.is_enabled method calls self.check_signal:

def is_enabled(self, request: HttpRequest) -> bool:
return bool(
re.match(conf.CORS_URLS_REGEX, request.path_info)
) or self.check_signal(request)

CorsMiddleware.process_request calls self.is_enabled and caches the result on the request object as request._cors_enabled:

request._cors_enabled = self.is_enabled(request)

The CorsMiddleware.process_response may have another call to self.is_enabled, but only if request._cors_enabled is not set (or set to None) (However I believe @romantomjak analysis is correct and that call can never be made since request._cors_enabled is always set to some boolean value at that point):

enabled = getattr(request, "_cors_enabled", None)
if enabled is None:
enabled = self.is_enabled(request)

Finally there's a second call to self.check_signal in process_response:

if (
not conf.CORS_ALLOW_ALL_ORIGINS
and not self.origin_found_in_white_lists(origin, url)
and not self.check_signal(request)
):

So technically check_request_enabled could be called 3 times during the course of a request/response cycle. In reality it can be called twice.

For the double firing to happen CORS_URLS_REGEX needs to be configured to a non-permissive value (i.e. not the default), CORS_ALLOW_ALL_ORIGINS needs to be set to False (the default) and the origin should not be included in either CORS_ALLOWED_ORIGINS or match any of the CORS_ALLOWED_ORIGIN_REGEXES.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants