From 22bb5a583005e2346ff21ac0d239097a67288bb1 Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Wed, 31 Oct 2012 15:57:45 -0400 Subject: [PATCH] Fix longpolling on messages to nobody. This is what caused our server to hang when receiving certain messages over the last couple days. It was introduced by me making in the assumption that doing the same thing we did after validate_notify failed was a correct way to immediately return from notify_new_message, which it was not. The code of validate_notify actually finished the handler in the event that validation failed, which isn't "correct", but did not manifest in a visible problem. The correct way to trigger an immediate response from a tornado view is to just return the value, not call handler.finish() and then return None. Similarly, the correct way to trigger longpolling from a tornado view is to either return None (or equivalently, / drop off the end of the function) or return a generator. (imported from commit 5b931248b4650fc88d5d68f5936a95f19e097af9) --- zephyr/views.py | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/zephyr/views.py b/zephyr/views.py index f815121f51..222fba9a3f 100644 --- a/zephyr/views.py +++ b/zephyr/views.py @@ -677,29 +677,25 @@ def send_message_backend(request, user_profile, sender, client_name=None): return json_success() -def validate_notify(request, handler): +def validate_notify(request): # Check the shared secret. # Also check the originating IP, at least for now. - if ((request.META['REMOTE_ADDR'] not in ('127.0.0.1', '::1')) - or (request.POST.get('secret') != settings.SHARED_SECRET)): + return (request.META['REMOTE_ADDR'] in ('127.0.0.1', '::1') + and request.POST.get('secret') == settings.SHARED_SECRET) - handler.set_status(403) - handler.finish('Access denied') - return False - return True @asynchronous @csrf_exempt @require_post def notify_new_message(request, handler): - if not validate_notify(request, handler): - return + if not validate_notify(request): + return json_error("Access denied", status=403) # If a message for some reason has no recipients (e.g. it is sent # by a bot to a stream that nobody is subscribed to), just skip # the message gracefully if request.POST["users"] == "": - return + return json_success() # FIXME: better query users = [UserProfile.objects.get(id=user) @@ -709,14 +705,14 @@ def notify_new_message(request, handler): for user in users: user.receive(message) - handler.finish() + return json_success() @asynchronous @csrf_exempt @require_post def notify_pointer_update(request, handler): - if not validate_notify(request, handler): - return + if not validate_notify(request): + return json_error("Access denied", status=403) # FIXME: better query user_profile = UserProfile.objects.get(id=request.POST['user']) @@ -725,7 +721,7 @@ def notify_pointer_update(request, handler): user_profile.update_pointer(new_pointer, pointer_updater) - handler.finish() + return json_success() @login_required_api_view def api_get_public_streams(request, user_profile):