From 8b51e40d6330b1191bbd3b57a7977553f78c80df Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michael=20P=C3=B6hn?= <michael.poehn@fsfe.org>
Date: Sun, 2 Apr 2017 12:08:01 +0200
Subject: [PATCH] avoid duplicate value assignments when updating config files

---
 fdroidserver/common.py | 59 ++++++++++++++++++++++++++++++++----------
 tests/common.TestCase  | 50 ++++++++++++++++++++++++++++++++++-
 2 files changed, 94 insertions(+), 15 deletions(-)

diff --git a/fdroidserver/common.py b/fdroidserver/common.py
index c1111939..a9778943 100644
--- a/fdroidserver/common.py
+++ b/fdroidserver/common.py
@@ -2213,24 +2213,55 @@ def get_cert_fingerprint(pubkey):
     return " ".join(ret)
 
 
-def write_to_config(thisconfig, key, value=None):
-    '''write a key/value to the local config.py'''
+def write_to_config(thisconfig, key, value=None, config_file=None):
+    '''write a key/value to the local config.py
+
+    NOTE: only supports writing string variables.
+
+    :param thisconfig: config dictionary
+    :param key: variable name in config.py to be overwritten/added
+    :param value: optional value to be written, instead of fetched
+        from 'thisconfig' dictionary.
+    '''
     if value is None:
         origkey = key + '_orig'
         value = thisconfig[origkey] if origkey in thisconfig else thisconfig[key]
-    with open('config.py', 'r', encoding='utf8') as f:
-        data = f.read()
-    pattern = '\n[\s#]*' + key + '\s*=\s*"[^"]*"'
-    repl = '\n' + key + ' = "' + value + '"'
-    data = re.sub(pattern, repl, data)
-    # if this key is not in the file, append it
-    if not re.match('\s*' + key + '\s*=\s*"', data):
-        data += repl
+    cfg = config_file if config_file else 'config.py'
+
+    # load config file
+    with open(cfg, 'r', encoding="utf-8") as f:
+        lines = f.readlines()
+
     # make sure the file ends with a carraige return
-    if not re.match('\n$', data):
-        data += '\n'
-    with open('config.py', 'w', encoding='utf8') as f:
-        f.writelines(data)
+    if len(lines) > 0:
+        if not lines[-1].endswith('\n'):
+            lines[-1] += '\n'
+
+    # regex for finding and replacing python string variable
+    # definitions/initializations
+    pattern = re.compile('^[\s#]*' + key + '\s*=\s*"[^"]*"')
+    repl = key + ' = "' + value + '"'
+    pattern2 = re.compile('^[\s#]*' + key + "\s*=\s*'[^']*'")
+    repl2 = key + " = '" + value + "'"
+
+    # If we replaced this line once, we make sure won't be a
+    # second instance of this line for this key in the document.
+    didRepl = False
+    # edit config file
+    with open(cfg, 'w', encoding="utf-8") as f:
+        for line in lines:
+            if pattern.match(line) or pattern2.match(line):
+                if not didRepl:
+                    line = pattern.sub(repl, line)
+                    line = pattern2.sub(repl2, line)
+                    f.write(line)
+                    didRepl = True
+            else:
+                f.write(line)
+        if not didRepl:
+            f.write('\n')
+            f.write(repl)
+            f.write('\n')
 
 
 def parse_xml(path):
diff --git a/tests/common.TestCase b/tests/common.TestCase
index 50cbd320..d5978405 100755
--- a/tests/common.TestCase
+++ b/tests/common.TestCase
@@ -10,9 +10,9 @@ import shutil
 import sys
 import tempfile
 import unittest
+import textwrap
 from zipfile import ZipFile
 
-import fdroidserver.signindex
 
 localmodule = os.path.realpath(
     os.path.join(os.path.dirname(inspect.getfile(inspect.currentframe())), '..'))
@@ -20,6 +20,7 @@ print('localmodule: ' + localmodule)
 if localmodule not in sys.path:
     sys.path.insert(0, localmodule)
 
+import fdroidserver.signindex
 import fdroidserver.common
 import fdroidserver.metadata
 
@@ -233,6 +234,53 @@ class CommonTest(unittest.TestCase):
         self.assertFalse(fdroidserver.common.verify_apk_signature(twosigapk))
         self.assertIsNone(fdroidserver.common.verify_apks(sourceapk, twosigapk, tmpdir))
 
+    def test_write_to_config(self):
+        with tempfile.TemporaryDirectory() as tmpPath:
+            cfgPath = os.path.join(tmpPath, 'config.py')
+            with open(cfgPath, 'w', encoding='utf-8') as f:
+                f.write(textwrap.dedent("""\
+                    # abc
+                    # test = 'example value'
+                    default_me= '%%%'
+
+                    # comment
+                    do_not_touch = "good value"
+                    default_me="!!!"
+
+                    key="123"    # inline"""))
+
+            cfg = {'key': '111', 'default_me_orig': 'orig'}
+            fdroidserver.common.write_to_config(cfg, 'key', config_file=cfgPath)
+            fdroidserver.common.write_to_config(cfg, 'default_me', config_file=cfgPath)
+            fdroidserver.common.write_to_config(cfg, 'test', value='test value', config_file=cfgPath)
+            fdroidserver.common.write_to_config(cfg, 'new_key', value='new', config_file=cfgPath)
+
+            with open(cfgPath, 'r', encoding='utf-8') as f:
+                self.assertEqual(f.read(), textwrap.dedent("""\
+                    # abc
+                    test = 'test value'
+                    default_me = 'orig'
+
+                    # comment
+                    do_not_touch = "good value"
+
+                    key = "111"    # inline
+
+                    new_key = "new"
+                    """))
+
+    def test_write_to_config_when_empty(self):
+        with tempfile.TemporaryDirectory() as tmpPath:
+            cfgPath = os.path.join(tmpPath, 'config.py')
+            with open(cfgPath, 'w') as f:
+                pass
+            fdroidserver.common.write_to_config({}, 'key', 'val', cfgPath)
+            with open(cfgPath, 'r', encoding='utf-8') as f:
+                self.assertEqual(f.read(), textwrap.dedent("""\
+
+                key = "val"
+                """))
+
 
 if __name__ == "__main__":
     parser = optparse.OptionParser()