launchpad-reviewers team mailing list archive

[Kit Randel <kit.randel@xxxxxxxxxxxxx>]

Kit Randel has proposed merging lp:~blr/rutabaga/auth-helper-no-exit into lp:rutabaga.

Commit message:
Prevent rutabaga exiting, unless exception raised.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~blr/rutabaga/auth-helper-no-exit/+merge/299638

I noticed on Dogfood that squid complained of rutabaga's auth helper exiting unexpectedly.

e.g.

2016/07/11 02:50:44.703| helper.cc(1180) GetFirstAvailable: GetFirstAvailable: Running servers 1
2016/07/11 02:50:44.733| helper.cc(901) helperHandleRead: helperHandleRead: 0 bytes from basicauthenticator #1
2016/07/11 02:50:44.733| WARNING: basicauthenticator #1 exited

This is not particularly well documented, but I think the expectation is that the helper should not exit, unless under exceptional circumstances. I've confirmed this by looking at examples of other auth helpers, none of which are authoritative unfortunately.

I have tested this change on Dogfood however, and the error is suppressed and all appears well.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~blr/rutabaga/auth-helper-no-exit into lp:rutabaga.

=== modified file 'rutabaga/scripts/rutabaga_auth_helper.py'
--- rutabaga/scripts/rutabaga_auth_helper.py	2015-11-10 00:59:00 +0000
+++ rutabaga/scripts/rutabaga_auth_helper.py	2016-07-11 03:05:42 +0000
@@ -1,4 +1,7 @@
 #!/srv/rutabaga/code/env/bin/python3
+#
+# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
 
 import sys
 
@@ -16,41 +19,43 @@
 AUTH_WS_URL = 'http://localhost:8080/validate'
 
 # squid 'debug_options ALL,1 82,9 84,9' can be potentially
-# useful for debugging this helper.
-
-
-def read_stdin():
-    line = sys.stdin.readline()
-    validate(line)
-
-
-def validate(squid_stdin):
-    data = squid_stdin.split()
-    if len(data) != 2:
-        # squid can potentially return channel_id in position 0
-        # if concurrency is enabled.
-        raise IOError
-
-    username = data[0]  # username
-    secret = data[1]  # password
-
+# useful for debugging this
+
+
+def validate(username, secret):
     s = Session()
     s.mount('http://', HTTPAdapter(max_retries=5))
 
     try:
-        r = s.post(AUTH_WS_URL, json={'secret': secret,
-                                      'username': username})
+        r = s.post(AUTH_WS_URL, json={'secret': secret, 'username': username})
+        if r.status_code == codes.ok and r.json()['username'] == username:
+            return True
+        else:
+            return False
     except (ConnectionError, Timeout):
         # squid 3.4 implements a more informative helper failure code
         # 'BH'. If we upgrade, consider using.
         # http://wiki.squid-cache.org/Features/AddonHelpers#Basic_Scheme
         sys.exit(1)
 
-    if r.status_code == codes.ok and r.json()['username'] == username:
-        sys.stdout.write('OK\n')
-    else:
-        sys.stderr.write('ERR\n')
+
+def main():
+    while True:
+        data = sys.stdin.readline().split()
+        if len(data) != 2:
+            # squid can potentially return channel_id in position 0
+            # if concurrency is enabled.
+            raise IOError
+
+        username = data[0]
+        secret = data[1]
+
+        if validate(username, secret):
+            sys.stdout.write('OK\n')
+        else:
+            sys.stderr.write('ERR\n')
+        sys.stdout.flush()
 
 
 if __name__ == '__main__':
-    read_stdin()
+    main()