popen() function strips the quotes from the arguments, so backup fails

for the schema name that needs quoting.

Code is changed to add escapers.

Fixes #4590

docs/en_US/release_notes_4_14.rst

@@ -24,6 +24,7 @@ Bug fixes
 
 | `Issue #3386 <https://redmine.postgresql.org/issues/3386>`_ -  Ensure backup a partition table should not backup the whole database.
 | `Issue #4199 <https://redmine.postgresql.org/issues/4199>`_ -  Ensure that 'ENTER' key in the data filter should not run the query.
+| `Issue #4590 <https://redmine.postgresql.org/issues/4590>`_ -  Fix issue where backup fails for schema name that needs quoting.
 | `Issue #4728 <https://redmine.postgresql.org/issues/4728>`_ -  Highlighted the color of closing or opening parenthesis when user select them in CodeMirror.
 | `Issue #4751 <https://redmine.postgresql.org/issues/4751>`_ -  Fix issue where export job fails when deselecting all the columns.
 | `Issue #4753 <https://redmine.postgresql.org/issues/4753>`_ -  Fix an error where 'false' string is displayed when we add a new parameter in the Parameters tab, also clear the old value when the user changes the parameter name.

web/pgadmin/misc/bgprocess/__init__.py

@@ -120,3 +120,18 @@ def stop_process(pid):
         return success_return()
     except LookupError as lerr:
         return gone(errormsg=str(lerr))
+
+
+def escape_dquotes_process_arg(arg):
+    # Double quotes has special meaning for shell command line and they are
+    # run without the double quotes. Add extra quotes to save our double
+    # quotes from stripping.
+
+    # This cannot be at common place as this file executes
+    # separately from pgadmin
+    dq_id = "#DQ#"
+
+    if arg.startswith('"') and arg.endswith('"'):
+        return r'{0}{1}{0}'.format(dq_id, arg)
+    else:
+        return arg

web/pgadmin/misc/bgprocess/process_executor.py

@@ -60,6 +60,22 @@ else:
             )
 
 
+def unescape_dquotes_process_arg(arg):
+    # Double quotes has special meaning for shell command line and they are
+    # run without the double quotes.
+    #
+    # Remove the saviour #DQ#
+
+    # This cannot be at common place as this file executes
+    # separately from pgadmin
+    dq_id = "#DQ#"
+
+    if arg.startswith(dq_id) and arg.endswith(dq_id):
+        return '{0}'.format(arg[len(dq_id):-len(dq_id)])
+    else:
+        return arg
+
+
 def _log_exception():
     type_, value_, traceback_ = info = sys.exc_info()
 
@@ -274,14 +290,14 @@ def update_status(**kw):
         raise ValueError("Please verify pid and db_file arguments.")
 
 
-def execute():
+def execute(argv):
     """
     This function will execute the background process
 
     Returns:
         None
     """
-    command = sys.argv[1:]
+    command = argv[1:]
     args = dict()
     _log('Initialize the process execution: {0}'.format(command))
 
@@ -363,7 +379,7 @@ def execute():
                 process_stderr.log(data[1])
 
     # If executable not found or invalid arguments passed
-    except OSError:
+    except OSError as e:
         info = _log_exception()
         args.update({'exit_code': 500})
         if process_stderr:
@@ -421,6 +437,10 @@ def convert_environment_variables(env):
 
 if __name__ == '__main__':
 
+    argv = [
+        unescape_dquotes_process_arg(arg) for arg in sys.argv
+    ]
+
     _sys_encoding = sys.getdefaultencoding()
     if not _sys_encoding or _sys_encoding == 'ascii':
         # Fall back to 'utf-8', if we couldn't determine the default encoding,
@@ -466,7 +486,7 @@ if __name__ == '__main__':
             # Let's do the job assigning to it.
             try:
                 _log('Executing the command now from the detached child...')
-                execute()
+                execute(argv)
             except Exception:
                 _log_exception()
         else:
@@ -500,7 +520,7 @@ if __name__ == '__main__':
             }
 
             cmd = [sys.executable]
-            cmd.extend(sys.argv)
+            cmd.extend(argv)
 
             _log('[PARENT] Command executings: {0}'.format(cmd))
 
@@ -549,7 +569,7 @@ if __name__ == '__main__':
                 w.close()
 
                 _log('[CHILD] Start executing the background process...')
-                execute()
+                execute(argv)
             except Exception:
                 _log_exception()
                 sys.exit(1)

web/pgadmin/setup/tests/test_export_import_servers.py

@@ -36,13 +36,13 @@ class ImportExportServersTestCase(BaseTestGenerator):
 
         # Load the servers
         os.system(
-            "python %s --load-servers %s 2> %s" %
+            "python \"%s\" --load-servers \"%s\" 2> %s" %
             (setup, os.path.join(path, "servers.json"), os.devnull)
         )
 
         # And dump them again
         tf = tempfile.NamedTemporaryFile(delete=False)
-        os.system("python %s --dump-servers %s 2> %s" %
+        os.system("python \"%s\" --dump-servers \"%s\" 2> %s" %
                   (setup, tf.name, os.devnull))
 
         # Compare the JSON files, ignoring servers that exist in our

web/pgadmin/tools/backup/__init__.py

@@ -24,6 +24,7 @@ from pgadmin.utils.ajax import make_json_response, bad_request
 
 from config import PG_DEFAULT_DRIVER
 from pgadmin.model import Server
+from pgadmin.misc.bgprocess import escape_dquotes_process_arg
 
 # set template path for sql scripts
 MODULE_NAME = 'backup'
@@ -419,17 +420,23 @@ def create_backup_objects_job(sid):
 
     if 'schemas' in data:
         for s in data['schemas']:
-            args.extend(['--schema', s])
+            args.extend(['--schema', r'{0}'.format(
+                driver.qtIdent(conn, s).replace('"', '\"'))])
 
     if 'tables' in data:
         for s, t in data['tables']:
             args.extend([
-                '--table', driver.qtIdent(conn, s, t)
+                '--table', r'{0}'.format(
+                    driver.qtIdent(conn, s, t).replace('"', '\"'))
             ])
 
+    escaped_args = [
+        escape_dquotes_process_arg(arg) for arg in args
+    ]
     try:
         if backup_obj_type == 'objects':
             args.append(data['database'])
+            escaped_args.append(data['database'])
             p = BatchProcess(
                 desc=BackupMessage(
                     BACKUP.OBJECT, sid,
@@ -439,7 +446,7 @@ def create_backup_objects_job(sid):
                     *args,
                     database=data['database']
                 ),
-                cmd=utility, args=args
+                cmd=utility, args=escaped_args
             )
         else:
             p = BatchProcess(
@@ -452,7 +459,7 @@ def create_backup_objects_job(sid):
                     ) else data['file'],
                     *args
                 ),
-                cmd=utility, args=args
+                cmd=utility, args=escaped_args
             )
 
         manager.export_password_env(p.id)