kill phantomjs process if timeout (#14606)

mandayejs/mandaye/utils.py

@@ -17,6 +17,7 @@ import os
 import json
 import subprocess
 import logging
+import multiprocessing
 
 from django.conf import settings
 
@@ -28,29 +29,43 @@ logger = logging.getLogger(__name__)
 
 
 def exec_phantom(data, script='do_login.js'):
-    phantom = subprocess.Popen([
-        settings.PHANTOM_JS_BINARY,
-        '--ignore-ssl-errors=yes', '--ssl-protocol=any',
-        os.path.join(settings.BASE_DIR, 'mandayejs', script)],
-        close_fds=True,
-        stdin=subprocess.PIPE,
-        stdout=subprocess.PIPE
-    )
 
-    stdout, stderr = phantom.communicate(json.dumps(data))
+    def run(send_end):
+        phantom = subprocess.Popen([
+            settings.PHANTOM_JS_BINARY,
+            '--ignore-ssl-errors=yes', '--ssl-protocol=any',
+            os.path.join(settings.BASE_DIR, 'mandayejs', script)],
+            close_fds=True,
+            stdin=subprocess.PIPE,
+            stdout=subprocess.PIPE
+        )
+        stdout, stderr = phantom.communicate(json.dumps(data))
 
-    try:
-        result = json.loads(stdout)
-    except (ValueError,):
-        result = {"result": "failure, couldn't decode JSON"}
-        logger.error(stdout)
+        try:
+            result = json.loads(stdout)
+        except (ValueError,):
+            result = {"result": "failure, couldn't decode JSON"}
+            logger.error(stdout)
 
-        # only kill process if it's still running
-        if phantom.poll() is None:
-            phantom.terminate()
+        if result.get('stderr'):
+            logger.warning(result['stderr'])
+
+        send_end.send(result)
+
+    recv_end, send_end = multiprocessing.Pipe(False)
+    process = multiprocessing.Process(target=run, args=(send_end,))
+    process.start()
+    process.join(settings.PHANTOM_JS_TIMEOUT)
+
+    if process.is_alive():
+        process.terminate()
+        # Don't log locators, they may contain credentials (passwords)
+        context = {k: v for k, v in data.items() if k != 'locators'}
+        logger.error("PhantomJS process timeout, context: %s" % context)
+        result = {'result': 'timeout'}
+    else:
+        result = recv_end.recv()
 
-    if result.get('stderr'):
-        logger.warning(result['stderr'])
     return result
 
 

mandayejs/mandaye/views.py

@@ -155,6 +155,9 @@ def post_login_do(request, *args, **kwargs):
         credentials.delete()
         messages.error(request, _('wrong user credentials'))
         url = resolve_url('associate')
+    elif result.get('result') == 'timeout':
+        messages.error(request, _('server took too long to respond'))
+        url = resolve_url('associate')
     elif result.get('result') == 'redirect':
         url = result.get('url', '/')
     else:

mandayejs/settings.py

@@ -164,6 +164,8 @@ SITE_APP = None
 
 PHANTOM_JS_BINARY = '/usr/bin/phantomjs'
 
+# Default timeout before killing Phantomjs process
+PHANTOM_JS_TIMEOUT = 10
 
 JSONFIELD_ENCODER_CLASS = 'django.core.serializers.json.DjangoJSONEncoder'
 

tests/settings.py

@@ -47,3 +47,6 @@ else:
 api_settings.user_settings.update({
     'DEFAULT_AUTHENTICATION_CLASSES' : AUTHENTICATION_CLASSES,
 })
+
+
+PHANTOM_JS_TIMEOUT = 0.25

tests/test_mandayejs.py

@@ -1,6 +1,7 @@
 # -*- coding: utf-8 -*-
 
 import json
+import time
 import mock
 import pytest
 
@@ -47,8 +48,11 @@ MOCKED_SITE_LOCATORS = [
 
 
 class MockedPopen(mock.Mock):
+    stall_for = False
 
     def communicate(self, data):
+        if self.stall_for:
+            time.sleep(13)
         return self.expected_output
 
 
@@ -272,6 +276,20 @@ def test_phantom_js_errors(mocked_popen, caplog):
     assert result['url'] == 'https://whatever.com/someurl'
 
 
+@mock.patch('mandayejs.mandaye.utils.subprocess.Popen')
+def test_phantom_js_timeout(mocked_popen, caplog):
+    mocked_popen.return_value = MockedPopen(expected_output=json.dumps({'whatever': 'whatever'}), stall_for=True)
+    result = exec_phantom(LOGIN_INFO)
+
+    for record in caplog.records():
+        assert record.levelname == 'ERROR'
+        assert 'https://whatever.com' in record.message
+        assert 'tenants/static/js/auth.checker.js' in record.message
+        assert 'input[type=submit], button' in record.message
+
+    assert result['result'] == 'timeout'
+
+
 @mock.patch('mandayejs.applications.Test.SITE_LOCATORS', MOCKED_SITE_LOCATORS)
 def test_credentials_json_encoding(user_john):