Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions usr/lib/python3/dist-packages/sanitize_string/sanitize_string.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,23 @@ def main() -> int:
## Read untrusted_string from stdin if needed
if untrusted_string is None:
if sys.stdin is not None:
if "pytest" not in sys.modules:
sys.stdin.reconfigure(errors="ignore") # type: ignore
untrusted_string = sys.stdin.read()
if "pytest" not in sys.modules and hasattr(sys.stdin, "reconfigure"):
sys.stdin.reconfigure(errors="ignore") # type: ignore[arg-type]
try:
read_fn = sys.stdin.read
except AttributeError:
print(
"sanitize-string: standard input is unreadable", file=sys.stderr
)
return 1
try:
untrusted_string = read_fn()
except (OSError, ValueError):
print(
"sanitize-string: failed to read from standard input",
file=sys.stderr,
)
return 1
Comment on lines -75 to +91

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is again guarding against conditions that must not occur in real-world use, so this isn't useful.

As described above though, the if "pytest" not in sys.modules bit can probably be lost if we patch stdin with an actual TextIOWrapper.

else:
## No way to get an untrusted string, print nothing and
## exit successfully
Expand Down
153 changes: 146 additions & 7 deletions usr/lib/python3/dist-packages/sanitize_string/tests/sanitize_string.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@

# pylint: disable=missing-module-docstring,fixme

import sys
from io import StringIO
from unittest import mock

from strip_markup.tests.strip_markup import TestStripMarkupBase
from stdisplay.tests.stdisplay import simple_escape_cases

Expand All @@ -19,32 +23,167 @@ class TestSanitizeString(TestStripMarkupBase):
maxDiff = None

argv0: str = "sanitize-string"
help_str: str = """\
sanitize-string: Usage: sanitize-string [--help] max_length [string]
If no string is provided as an argument, the string is read from standard input.
Set max_length to 'nolimit' to allow arbitrarily long strings.
"""

def _test_args_with_exit(
self,
stdout_string: str,
stderr_string: str,
args: list[str],
exit_code: int,
) -> None:
stdout_buf: StringIO = StringIO()
stderr_buf: StringIO = StringIO()
args_arr: list[str] = [self.argv0, *args]
with (
mock.patch.object(sys, "argv", args_arr),
mock.patch.object(sys, "stdout", stdout_buf),
mock.patch.object(sys, "stderr", stderr_buf),
):
result: int = sanitize_string_main()
self.assertEqual(stdout_buf.getvalue(), stdout_string)
self.assertEqual(stderr_buf.getvalue(), stderr_string)
self.assertEqual(result, exit_code)
stdout_buf.close()
stderr_buf.close()

def test_help(self) -> None:
"""
Ensure sanitize_string.py's help output is as expected.
"""

help_str: str = """\
sanitize-string: Usage: sanitize-string [--help] max_length [string]
If no string is provided as an argument, the string is read from standard input.
Set max_length to 'nolimit' to allow arbitrarily long strings.
"""
self._test_args(
main_func=sanitize_string_main,
argv0=self.argv0,
stdout_string="",
stderr_string=help_str,
stderr_string=self.help_str,
args=["--help"],
)
self._test_args(
main_func=sanitize_string_main,
argv0=self.argv0,
stdout_string="",
stderr_string=help_str,
stderr_string=self.help_str,
args=["-h"],
)

def test_usage_errors(self) -> None:
"""Ensure argument validation errors emit usage and exit non-zero."""

error_cases: list[list[str]] = [
[],
["-5"],
["not-a-number"],
["1", "2", "3"],
]

for args in error_cases:
self._test_args_with_exit(
stdout_string="",
stderr_string=self.help_str,
args=args,
exit_code=1,
)

def test_missing_stdin(self) -> None:
"""Verify sanitize_string exits cleanly when stdin is unavailable."""

stdout_buf: StringIO = StringIO()
stderr_buf: StringIO = StringIO()
args_arr: list[str] = [self.argv0, "nolimit"]
with (
mock.patch.object(sys, "argv", args_arr),
mock.patch.object(sys, "stdin", None),
mock.patch.object(sys, "stdout", stdout_buf),
mock.patch.object(sys, "stderr", stderr_buf),
):
exit_code: int = sanitize_string_main()
self.assertEqual(stdout_buf.getvalue(), "")
self.assertEqual(stderr_buf.getvalue(), "")
self.assertEqual(exit_code, 0)
stdout_buf.close()
stderr_buf.close()
Comment on lines +92 to +109

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already test this in run-test itself. Maybe moving that logic out of the Bash script and into the Python scripts might be a good future goal, but for now we don't need this.


def test_stdin_without_reconfigure(self) -> None:
"""Ensure sanitize_string tolerates stdin objects lacking reconfigure."""

stdout_buf: StringIO = StringIO()
stderr_buf: StringIO = StringIO()
stdin_buf: StringIO = StringIO("Sample input")
args_arr: list[str] = [self.argv0, "nolimit"]
original_pytest_module = sys.modules.pop("pytest", None)
try:
with (
mock.patch.object(sys, "argv", args_arr),
mock.patch.object(sys, "stdin", stdin_buf),
mock.patch.object(sys, "stdout", stdout_buf),
mock.patch.object(sys, "stderr", stderr_buf),
):
exit_code: int = sanitize_string_main()
finally:
if original_pytest_module is not None:
sys.modules["pytest"] = original_pytest_module
self.assertEqual(stdout_buf.getvalue(), "Sample input")
self.assertEqual(stderr_buf.getvalue(), "")
self.assertEqual(exit_code, 0)
stdout_buf.close()
stderr_buf.close()
Comment on lines +111 to +134

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this actually useful? In real-world use stdin will either have reconfigure(), or it will be None, unless something changes in the future so that sys.stdin is no longer a TextIOWrapper. When using StringIO under pytest, that isn't the case, but that's why the sanitize_string main function skips the reconfigure step if pytest is in sys.modules. Testing a case that can only happen in the test suite seems unnecessary.

What would be better is if the test suite actually provided a TextIOWrapper to stdin so that we could always use reconfigure... that's actually possible according to https://stackoverflow.com/questions/44702487/how-to-create-and-write-to-a-textiowrapper-and-readlines, so that's probably what I'll do. Then this whole block can be skipped.


def test_unreadable_stdin_raises_error(self) -> None:
"""Ensure unreadable stdin streams fail gracefully."""

class ExplodingStdin:
def read(self) -> str: # pragma: no cover - invoked via main
raise ValueError("boom")

stdout_buf: StringIO = StringIO()
stderr_buf: StringIO = StringIO()
args_arr: list[str] = [self.argv0, "nolimit"]
with (
mock.patch.object(sys, "argv", args_arr),
mock.patch.object(sys, "stdin", ExplodingStdin()),
mock.patch.object(sys, "stdout", stdout_buf),
mock.patch.object(sys, "stderr", stderr_buf),
):
exit_code: int = sanitize_string_main()
self.assertEqual(stdout_buf.getvalue(), "")
self.assertEqual(
stderr_buf.getvalue(),
"sanitize-string: failed to read from standard input\n",
)
self.assertEqual(exit_code, 1)
stdout_buf.close()
stderr_buf.close()
Comment on lines +136 to +160

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If stdin raises an error on read, something is very, very wrong most likely in the kernel. Reading from stdin should never fail, it should In that case exiting gracefully would actually be a bad thing.


def test_missing_read_attribute_returns_error(self) -> None:
"""Validate stdin objects without read cause a clean failure."""

class NoReadStdin: # pragma: no cover - exercised indirectly
pass

stdout_buf: StringIO = StringIO()
stderr_buf: StringIO = StringIO()
args_arr: list[str] = [self.argv0, "nolimit"]
with (
mock.patch.object(sys, "argv", args_arr),
mock.patch.object(sys, "stdin", NoReadStdin()),
mock.patch.object(sys, "stdout", stdout_buf),
mock.patch.object(sys, "stderr", stderr_buf),
):
exit_code: int = sanitize_string_main()
self.assertEqual(stdout_buf.getvalue(), "")
self.assertEqual(
stderr_buf.getvalue(),
"sanitize-string: standard input is unreadable\n",
)
self.assertEqual(exit_code, 1)
stdout_buf.close()
stderr_buf.close()
Comment on lines +162 to +185

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, this can only ever happen in tests, at least if sanitize_string is used properly. This test is unnecessary.


def test_safe_strings(self) -> None:
"""
Wrapper for _test_safe_strings (from TestStripMarkup) specific to
Expand Down