fix(text-injection): remove unnecessary shell escaping for subprocess calls (#149)
Removes the _escape_text method and its usage since text is passed to subprocess.run() via list arguments, not shell strings. When using subprocess with a list (without shell=True), arguments are passed directly to the command without shell interpretation, making shell escaping unnecessary and actually harmful. This fixes the bug where apostrophes and other special characters were typed with backslash escapes (e.g., "Let\'s" instead of "Let's"). Changes: - Remove _escape_text() method from TextInjector class - Remove escaped_text variable from inject_text() method - Pass text directly to _inject_with_xdotool() and _inject_with_wayland_tool() - Update docstrings to remove references to escaped text - Update tests to verify text is NOT escaped when passed to subprocess Closes #119
This commit is contained in:
@@ -197,19 +197,19 @@ class TextInjector:
|
||||
# Get information about the current window/application
|
||||
self._log_current_window_info()
|
||||
|
||||
# Escape special characters for shell
|
||||
escaped_text = self._escape_text(text)
|
||||
logger.debug(f"Escaped text: '{escaped_text}'")
|
||||
# Note: No shell escaping needed - subprocess is called with list arguments,
|
||||
# which passes text directly without shell interpretation
|
||||
logger.debug(f"Text to inject: '{text}'")
|
||||
|
||||
try:
|
||||
if (
|
||||
self.environment == DesktopEnvironment.X11
|
||||
or self.environment == DesktopEnvironment.WAYLAND_XDOTOOL
|
||||
):
|
||||
self._inject_with_xdotool(escaped_text)
|
||||
self._inject_with_xdotool(text)
|
||||
else:
|
||||
try:
|
||||
self._inject_with_wayland_tool(escaped_text)
|
||||
self._inject_with_wayland_tool(text)
|
||||
except subprocess.CalledProcessError as e:
|
||||
stderr_msg = e.stderr.strip() if e.stderr else "No stderr output"
|
||||
logger.warning(
|
||||
@@ -220,7 +220,7 @@ class TextInjector:
|
||||
).lower() + " " + stderr_msg.lower() and shutil.which("xdotool"):
|
||||
logger.info("Automatically switching to XWayland fallback permanently")
|
||||
self.environment = DesktopEnvironment.WAYLAND_XDOTOOL
|
||||
self._inject_with_xdotool(escaped_text)
|
||||
self._inject_with_xdotool(text)
|
||||
else:
|
||||
raise
|
||||
logger.info("Text injection completed successfully")
|
||||
@@ -235,37 +235,12 @@ class TextInjector:
|
||||
logger.warning("Could not import audio feedback module")
|
||||
return False
|
||||
|
||||
def _escape_text(self, text: str) -> str:
|
||||
"""
|
||||
Escape special characters in the text for shell commands.
|
||||
|
||||
Args:
|
||||
text: The original text
|
||||
|
||||
Returns:
|
||||
The escaped text
|
||||
"""
|
||||
# Replace common special characters
|
||||
replacements = {
|
||||
"'": r"\'",
|
||||
'"': r"\"",
|
||||
"`": r"\`",
|
||||
"\\": r"\\",
|
||||
"$": r"\$",
|
||||
}
|
||||
|
||||
result = text
|
||||
for char, replacement in replacements.items():
|
||||
result = result.replace(char, replacement)
|
||||
|
||||
return result
|
||||
|
||||
def _inject_with_xdotool(self, text: str):
|
||||
"""
|
||||
Inject text using xdotool for X11 environments.
|
||||
|
||||
Args:
|
||||
text: The text to inject (already escaped)
|
||||
text: The text to inject
|
||||
"""
|
||||
# Create environment with explicit X11 settings for Wayland compatibility
|
||||
env = os.environ.copy()
|
||||
@@ -384,7 +359,7 @@ class TextInjector:
|
||||
Inject text using a Wayland-compatible tool (wtype or ydotool).
|
||||
|
||||
Args:
|
||||
text: The text to inject (already escaped)
|
||||
text: The text to inject
|
||||
|
||||
Raises:
|
||||
subprocess.CalledProcessError: If the tool fails, with stderr captured
|
||||
|
||||
+18
-26
@@ -229,21 +229,33 @@ class TestTextInjector(unittest.TestCase):
|
||||
# Inject text
|
||||
injector.inject_text(special_text)
|
||||
|
||||
# Verify xdotool was called with proper escaping
|
||||
# Find calls that contain xdotool and check they contain escaped text
|
||||
# Verify xdotool was called with the original text (no escaping needed)
|
||||
# Find calls that contain xdotool and check they contain the original text
|
||||
found_unescaped = False
|
||||
found_escaped = False
|
||||
for args, _ in calls:
|
||||
if len(args) > 0 and isinstance(args[0], list):
|
||||
cmd = args[0]
|
||||
if "xdotool" in cmd and "type" in cmd:
|
||||
# Join the command to check for escaped characters
|
||||
# Join the command to check for characters
|
||||
cmd_str = " ".join(cmd)
|
||||
# Look for escaped quotes and dollar signs
|
||||
if "'" in cmd_str or '\\"' in cmd_str or "\\$" in cmd_str:
|
||||
# Check for unescaped special characters
|
||||
if "'" in cmd_str:
|
||||
found_unescaped = True
|
||||
if "\\'" in cmd_str or '\\"' in cmd_str or "\\$" in cmd_str:
|
||||
# Found escaped characters - this is bad!
|
||||
found_escaped = True
|
||||
break
|
||||
|
||||
self.assertTrue(found_escaped, "Special characters were not properly escaped")
|
||||
# Should find unescaped characters and NOT find escaped ones
|
||||
self.assertTrue(
|
||||
found_unescaped,
|
||||
"Text was not passed correctly to xdotool",
|
||||
)
|
||||
self.assertFalse(
|
||||
found_escaped,
|
||||
"Text should not be shell-escaped when passed to xdotool",
|
||||
)
|
||||
|
||||
def test_empty_text_injection(self):
|
||||
"""Test injecting empty text (should do nothing)."""
|
||||
@@ -291,26 +303,6 @@ class TestTextInjector(unittest.TestCase):
|
||||
# Check that error sound was triggered
|
||||
audio_feedback.play_error_sound.assert_called_once()
|
||||
|
||||
def test_escape_text(self):
|
||||
"""Test the _escape_text method."""
|
||||
injector = TextInjector()
|
||||
|
||||
# Test various special characters
|
||||
result = injector._escape_text("'single'")
|
||||
self.assertIn("\\'", result)
|
||||
|
||||
result = injector._escape_text('"double"')
|
||||
self.assertIn('\\"', result)
|
||||
|
||||
result = injector._escape_text("$var")
|
||||
self.assertIn("\\$", result)
|
||||
|
||||
result = injector._escape_text("`backtick`")
|
||||
self.assertIn("\\`", result)
|
||||
|
||||
result = injector._escape_text("back\\slash")
|
||||
self.assertIn("\\\\", result)
|
||||
|
||||
def test_detect_environment_unknown(self):
|
||||
"""Test environment detection when no indicators are present."""
|
||||
with patch.dict("os.environ", {}, clear=True):
|
||||
|
||||
Reference in New Issue
Block a user