Refactor command quoting in PostgreSQLExecutor for consistency

- Updated the command templates in `PostgreSQLExecutor` to ensure the executable path is always double-quoted, improving compatibility with paths containing spaces.
- Modified tests in `test_windows_compatibility.py` to reflect the changes in command formatting, ensuring that single-quoted PostgreSQL options are preserved correctly within double-quoted command strings.
- Added new tests to verify that the `running()` method properly quotes the executable and datadir, enhancing robustness against shell parsing issues.

Author: tboy1337

pytest_postgresql/executor.py

@@ -61,7 +61,7 @@ class PostgreSQLExecutor(TCPExecutor):
     # On Unix, mirakuru uses shlex.split() with shell=False, so single quotes
     # inside double-quoted strings are preserved and passed to PostgreSQL's config parser.
     UNIX_PROC_START_COMMAND = (
-        '{executable} start -D "{datadir}" '
+        '"{executable}" start -D "{datadir}" '
         "-o \"-F -p {port} -c log_destination='stderr' "
         "-c logging_collector=off "
         "-c unix_socket_directories='{unixsocketdir}'{postgres_options}\" "
@@ -73,7 +73,7 @@ class PostgreSQLExecutor(TCPExecutor):
     # ignores it on Windows. On Windows, mirakuru forces shell=True so the command
     # goes through cmd.exe.
     WINDOWS_PROC_START_COMMAND = (
-        '{executable} start -D "{datadir}" '
+        '"{executable}" start -D "{datadir}" '
         '-o "-F -p {port} -c log_destination=stderr '
         '-c logging_collector=off{postgres_options}" '
         '-l "{logfile}" {startparams}'
@@ -243,7 +243,7 @@ def running(self) -> bool:
         """Check if server is running."""
         if not os.path.exists(self.datadir):
             return False
-        status_code = subprocess.getstatusoutput(f'{self.executable} status -D "{self.datadir}"')[0]
+        status_code = subprocess.getstatusoutput(f'"{self.executable}" status -D "{self.datadir}"')[0]
         return status_code == 0
 
     def _windows_terminate_process(self, _sig: Optional[int] = None) -> None:

tests/test_windows_compatibility.py

@@ -209,11 +209,14 @@ def test_postgres_options_with_single_quotes_windows(self) -> None:
         # postgres_options should be included with single quotes preserved
         assert "-c shared_buffers='128MB' -c work_mem='64MB'" in command
 
-    def test_postgres_options_with_double_quotes(self) -> None:
-        """Test postgres_options containing double quotes.
-
-        Double quotes in postgres_options need careful handling as they interact
-        with the shell's quote parsing.
+    def test_postgres_options_with_single_quoted_search_path(self) -> None:
+        """Test postgres_options with a single-quoted search_path value.
+
+        PostgreSQL GUC values are quoted with single quotes in the config syntax.
+        The Unix template wraps the whole -o argument in double quotes, so
+        inner double quotes would prematurely terminate that token and produce an
+        invalid shell command. Single-quoted values are the correct form and must
+        be preserved verbatim inside the -o argument.
         """
         with patch("pytest_postgresql.executor.platform.system", return_value="Linux"):
             executor = PostgreSQLExecutor(
@@ -225,12 +228,17 @@ def test_postgres_options_with_double_quotes(self) -> None:
                 logfile="/tmp/log",
                 startparams="-w",
                 dbname="test",
-                postgres_options='-c search_path="public,other"',
+                postgres_options="-c search_path='public,other'",
             )
 
         command = executor.command
-        # postgres_options with double quotes should be preserved
-        assert '-c search_path="public,other"' in command
+        # The single-quoted search_path value must appear verbatim inside the -o argument
+        assert "-c search_path='public,other'" in command
+        # Verify it is embedded inside the -o "..." token (no premature " termination)
+        o_start = command.index('-o "')
+        o_end = command.index('"', o_start + 4)
+        o_argument = command[o_start : o_end + 1]
+        assert "-c search_path='public,other'" in o_argument
 
     def test_postgres_options_with_paths_containing_spaces(self) -> None:
         """Test postgres_options with file paths containing spaces.
@@ -276,7 +284,7 @@ def test_empty_postgres_options(self) -> None:
 
         command = executor.command
         # Command should still be valid with empty postgres_options
-        assert "/usr/lib/postgresql/16/bin/pg_ctl start" in command
+        assert '"/usr/lib/postgresql/16/bin/pg_ctl" start' in command
         assert '-D "/tmp/data"' in command
         assert "unix_socket_directories='/tmp/socket'" in command
         # Should not have trailing space before closing quote in -o parameter
@@ -306,7 +314,7 @@ def test_empty_startparams(self) -> None:
 
         command = executor.command
         # Command should be valid with empty startparams
-        assert "/usr/lib/postgresql/16/bin/pg_ctl start" in command
+        assert '"/usr/lib/postgresql/16/bin/pg_ctl" start' in command
         assert '-l "/tmp/log"' in command
         # Command should not have trailing spaces at the end
         assert not command.endswith("  ")
@@ -332,7 +340,7 @@ def test_both_empty_postgres_options_and_startparams(self) -> None:
 
         command = executor.command
         # Command should be valid with both empty
-        assert "C:/Program Files/PostgreSQL/bin/pg_ctl.exe start" in command
+        assert '"C:/Program Files/PostgreSQL/bin/pg_ctl.exe" start' in command
         assert '-D "C:/temp/data"' in command
         assert '-l "C:/temp/log"' in command
         # Windows template should not have unix_socket_directories
@@ -621,7 +629,7 @@ def test_command_formatting_windows(self) -> None:
         # The command should be properly formatted without single quotes
         # and without unix_socket_directories (irrelevant on Windows)
         expected_parts = [
-            "C:/Program Files/PostgreSQL/bin/pg_ctl.exe start",
+            '"C:/Program Files/PostgreSQL/bin/pg_ctl.exe" start',
             '-D "C:/temp/data"',
             '-o "-F -p 5555 -c log_destination=stderr',
             "-c logging_collector=off",
@@ -723,6 +731,96 @@ def test_windows_mixed_slashes(self) -> None:
 
         command = executor.command
         # Paths with backslashes should be properly quoted
-        assert "C:\\Program Files\\PostgreSQL\\bin\\pg_ctl.exe start" in command
+        assert '"C:\\Program Files\\PostgreSQL\\bin\\pg_ctl.exe" start' in command
         assert '-D "C:\\temp\\data"' in command
         assert '-l "C:\\temp\\log.txt"' in command
+
+
+class TestRunningMethodQuoting:
+    """Test that the running() method properly quotes the executable path."""
+
+    def test_running_quotes_executable_with_spaces(self) -> None:
+        """Test that running() quotes the executable in the status command.
+
+        subprocess.getstatusoutput() uses shell=True internally. On Windows,
+        cmd.exe parses the command string and an unquoted path like
+        C:\\Program Files\\...\\pg_ctl.exe would be split at the space,
+        causing the status check to fail silently or error.
+        """
+        executor = PostgreSQLExecutor(
+            executable="C:/Program Files/PostgreSQL/17/bin/pg_ctl.exe",
+            host="localhost",
+            port=5432,
+            datadir="C:/temp/data",
+            unixsocketdir="C:/temp/socket",
+            logfile="C:/temp/log",
+            startparams="-w",
+            dbname="test",
+        )
+
+        with (
+            patch("pytest_postgresql.executor.os.path.exists", return_value=True),
+            patch("pytest_postgresql.executor.subprocess.getstatusoutput") as mock_getstatusoutput,
+        ):
+            mock_getstatusoutput.return_value = (0, "")
+            executor.running()
+
+            # The executable must be double-quoted in the shell command string
+            called_cmd = mock_getstatusoutput.call_args[0][0]
+            assert called_cmd.startswith('"C:/Program Files/PostgreSQL/17/bin/pg_ctl.exe"'), (
+                f"Executable not quoted in status command: {called_cmd!r}"
+            )
+
+    def test_running_quotes_executable_without_spaces(self) -> None:
+        """Test that running() still quotes executables without spaces.
+
+        Even when the executable path has no spaces, it should still be
+        double-quoted for consistency and correctness.
+        """
+        executor = PostgreSQLExecutor(
+            executable="/usr/lib/postgresql/17/bin/pg_ctl",
+            host="localhost",
+            port=5432,
+            datadir="/tmp/data",
+            unixsocketdir="/tmp/socket",
+            logfile="/tmp/log",
+            startparams="-w",
+            dbname="test",
+        )
+
+        with (
+            patch("pytest_postgresql.executor.os.path.exists", return_value=True),
+            patch("pytest_postgresql.executor.subprocess.getstatusoutput") as mock_getstatusoutput,
+        ):
+            mock_getstatusoutput.return_value = (0, "")
+            executor.running()
+
+            called_cmd = mock_getstatusoutput.call_args[0][0]
+            assert called_cmd.startswith('"/usr/lib/postgresql/17/bin/pg_ctl"'), (
+                f"Executable not quoted in status command: {called_cmd!r}"
+            )
+
+    def test_running_quotes_datadir_with_spaces(self) -> None:
+        """Test that running() quotes the datadir in the status command."""
+        executor = PostgreSQLExecutor(
+            executable="/usr/lib/postgresql/17/bin/pg_ctl",
+            host="localhost",
+            port=5432,
+            datadir="/tmp/my data dir",
+            unixsocketdir="/tmp/socket",
+            logfile="/tmp/log",
+            startparams="-w",
+            dbname="test",
+        )
+
+        with (
+            patch("pytest_postgresql.executor.os.path.exists", return_value=True),
+            patch("pytest_postgresql.executor.subprocess.getstatusoutput") as mock_getstatusoutput,
+        ):
+            mock_getstatusoutput.return_value = (0, "")
+            executor.running()
+
+            called_cmd = mock_getstatusoutput.call_args[0][0]
+            assert '-D "/tmp/my data dir"' in called_cmd, (
+                f"Datadir not quoted in status command: {called_cmd!r}"
+            )