Merge ~techalchemy/ubuntu-qa-tools:feature/in-memory-cookiedb into ubuntu-qa-tools:master

[Dan Ryan]

Use in-memory sqlite database for cookie storage and cookiejar generation

- Also addresses an issue converting launchpadlib.__version__ to and from integers due to byte/unicode encoding changes which prevented importing on python 3

[Steve Beattie (sbeattie)]

Did you actually try to use this? The sql portion of the change fails for me like so:

  $UCT/scripts/sis-changes --ppa ubuntu --pocket Proposed -r bionic --download ${HOME}/tmp/prepare-kernel-usn-6r8qbh5h/usn-bionic-linux-azure-5.3 linux-azure-5.3
  Traceback (most recent call last):
    File "/home/steve/git/ubuntu-cve-tracker/scripts/sis-changes", line 158, in <module>
      opener = lpl_common.opener_with_cookie(cve_lib.config["plb_authentication"])
    File "/home/steve/git/ubuntu-cve-tracker/scripts/lpl_common.py", line 148, in opener_with_cookie
      for line in cookie_db.iterdump():
    File "/usr/lib/python3.7/sqlite3/dump.py", line 30, in _iterdump
      schema_res = cu.execute(q)
  sqlite3.OperationalError: database is locked

which is why the shutil copy is performed in the original code.

Also, would prefer two distinct fixes as two distinct commits, please.

Thanks.

[Steve Beattie (sbeattie)]

That said, the changes around the launchpadlib version check looked fine; I pulled that change out and have merged that into master.

Thanks!

[Steve Beattie (sbeattie)]

The real solution, by the by, is to kill the remaining users of opener_with_cookie(), and convert them to using the proper auth mechanism from python-launchpadlib, either via lpl_common.py:connect() or directly.

(The opener_with_cookie() function is a hysterical/historical artifact dating back before python-launchpadlib existed and launchpad supported application auth tokens, and the only way you could interact was via web-scraping.)

[Dan Ryan (techalchemy)]

First of all thanks for taking the time to try this out, I have no idea how the code actually gets invoked so I wasn't able to verify that there wouldn't be any issues with competitive locks on the db in question. I included the encoding change because while attempting to import the function in question, I found I couldn't. I'll be sure to make more granular commits so that they can be cherry picked as needed.

I anticipated that, if necessary, I'd iterate on this change (e.g. by loading the db in memory instead of accessing it directly). This was a change we discussed (and there is a 'todo' in the code about it), but I am happy to either 'close' this merge request if I can figure out how to do that, or to make the changes you are suggesting to the remaining callers of this function.

Unmerged commits

1d490ec... by Dan Ryan on 2020-01-30

Use in-memory sqlite database for cookie storage and cookiejar generation

Signed-off-by: Dan Ryan <email address hidden>

diff --git a/common/lpl_common.py b/common/lpl_common.py
index de9281f..6b57854 100644
--- a/common/lpl_common.py
+++ b/common/lpl_common.py
@@ -7,7 +7,7 @@
 
 from __future__ import print_function
 
-import os, sys, tempfile, time, shutil, launchpadlib
+import os, sys, tempfile, time, shutil, launchpadlib, io
 from launchpadlib.launchpad import Launchpad
 from launchpadlib.credentials import Credentials
 import launchpadlib.errors
@@ -20,23 +20,26 @@ except ImportError:
 # as of 16.04, launchpadlib supports python3
 # so make code support both python2 and python3
 if sys.version_info > (3, 0):
+    string_types = (str,)
     from http.cookiejar import LoadError, MozillaCookieJar
+    from io import StringIO
     from urllib.request import HTTPCookieProcessor, HTTPError, build_opener
     from urllib.parse import unquote
 else:
+    string_types = (basestring,)
     from cookielib import LoadError, MozillaCookieJar
+    from cStringIO import StringIO
     from urllib2 import HTTPCookieProcessor, HTTPError, build_opener
     from urllib import unquote
 
 version_min = [1,5,7]
-if sys.version_info > (3, 0):
-    if [int(x) for x in launchpadlib.__version__.split('.')] < version_min:
-        raise ValueError("Requires launchpadlib version %s or later (%s in use)" \
-            % (".".join(version_min), launchpadlib.__version__))
-else:
-    if [int(x) for x in launchpadlib.__version__.decode().split('.')] < version_min:
-        raise ValueError("Requires launchpadlib version %s or later (%s in use)" \
-            % (".".join(version_min), launchpadlib.__version__))
+lp_version = launchpadlib.__version__
+if not isinstance(lp_version, string_types):
+    lp_version = lp_version.decode()
+
+if version_min > list(map(int, lp_version.split('.'))):
+    raise ValueError("Requires launchpadlib version %s or later (%s in use)" \
+        % (".".join(map(str, version_min)), lp_version))
 
 def connect(use_edge=False, beta=False, version=None, uri=None, bot=None):
 
@@ -136,36 +139,47 @@ def opener_with_cookie(cookie_file):
     old_umask = os.umask(0o077)
 
     # Work around Firefox 3.5's dumb sqlite locking problems by copying cookies out:
-    tmp = None
+    opener = None
+    cj = MozillaCookieJar()
     if cookie_file.endswith('.sqlite'):
-        (cookie_path, cookie_name) = os.path.split(cookie_file)
-        with tempfile.NamedTemporaryFile(prefix='cookies-XXXXXX', suffix='.sqlite') as sql_handle:
-            sql = sql_handle.name
-        shutil.copyfile(cookie_file, sql)
 
-        match = '%launchpad.net'
-        con = sqlite.connect(sql)
-        cur = con.cursor()
-        cur.execute("select host, path, isSecure, expiry, name, value from moz_cookies where host like ?", [match])
-        ftstr = ["FALSE","TRUE"]
-        tmp = tempfile.NamedTemporaryFile(prefix='cookies-XXXXXX', suffix='.mozilla', mode='w+')
-        cookie_file = tmp.name
-        tmp.write("# HTTP Cookie File\n")
-        for item in cur.fetchall():
-            str = "%s\t%s\t%s\t%s\t%s\t%s\t%s\n" % ( item[0], \
-                   ftstr[item[0].startswith('.')], item[1], \
-                   ftstr[item[2]], item[3], item[4], item[5])
-            tmp.write(str)
-        sql = None
-        tmp.flush()
+        sql_script = StringIO()
+        with sqlite.connect(cookie_file) as cookie_db:
+            for line in cookie_db.iterdump():
+                sql_script.write("{0}\n".format(line))
+        sql_script.seek(0)
 
-    cj = MozillaCookieJar()
-    try:
-        cj.load(cookie_file)
-    except LoadError as e:
-        print("Failed to load cookie from file (%s): %s - continuing anyway..." % (cookie_file, e.strerror))
+        match = '%launchpad.net'
+        query = "select host, path, isSecure, expiry, name, value from moz_cookies where host like ?"
+        with tempfile.NamedTemporaryFile(prefix='cookies-XXXXXX', suffix='.mozilla', mode='w+') as cookie_jar:
+            with sqlite.connect(":memory:") as con:
+                cur = con.cursor()
+                try:
+                    cur.executescript(sql_script.read())
+                    cur.commit()
+                except sqlite.Error:
+                    print("Failed loading cookied DB...")
+                    raise
+
+                try:
+                    cur.execute(query, [match])
+                except sqlite.Error:
+                    print("Failed reading database...")
+                    raise
+                ftstr = ["FALSE","TRUE"]
+                cookie_jar.write("# HTTP Cookie File\n")
+                for item in cur.fetchall():
+                    str_ = "%s\t%s\t%s\t%s\t%s\t%s\t%s\n" % ( item[0], \
+                        ftstr[item[0].startswith('.')], item[1], \
+                        ftstr[item[2]], item[3], item[4], item[5])
+                    cookie_jar.write(str_)
+
+            cookie_jar.flush()
+            try:
+                cj.load(cookie_jar.name)
+            except LoadError as e:
+                print("Failed to load cookie from file ({0}): {1!s} - continuing anyway...".format(cookie_jar.name, e))
     opener = build_opener(HTTPCookieProcessor(cj))
-    tmp = None
     os.umask(old_umask)
     return opener