Skip to content

Commit a24cfd7

Browse files
Harden file path and message normalization error boundaries
Co-authored-by: Shri Sukhani <shrisukhani@users.noreply.github.com>
1 parent 9fe6096 commit a24cfd7

File tree

2 files changed

+228
-25
lines changed

2 files changed

+228
-25
lines changed

hyperbrowser/client/file_utils.py

Lines changed: 74 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -5,31 +5,53 @@
55
from hyperbrowser.exceptions import HyperbrowserError
66

77

8+
def _validate_error_message_text(message_value: str, *, field_name: str) -> None:
9+
if not isinstance(message_value, str):
10+
raise HyperbrowserError(f"{field_name} must be a string")
11+
try:
12+
normalized_message = message_value.strip()
13+
if not isinstance(normalized_message, str):
14+
raise TypeError(f"normalized {field_name} must be a string")
15+
is_empty = len(normalized_message) == 0
16+
except HyperbrowserError:
17+
raise
18+
except Exception as exc:
19+
raise HyperbrowserError(
20+
f"Failed to normalize {field_name}",
21+
original_error=exc,
22+
) from exc
23+
if is_empty:
24+
raise HyperbrowserError(f"{field_name} must not be empty")
25+
try:
26+
contains_control_character = any(
27+
ord(character) < 32 or ord(character) == 127
28+
for character in message_value
29+
)
30+
except HyperbrowserError:
31+
raise
32+
except Exception as exc:
33+
raise HyperbrowserError(
34+
f"Failed to validate {field_name} characters",
35+
original_error=exc,
36+
) from exc
37+
if contains_control_character:
38+
raise HyperbrowserError(f"{field_name} must not contain control characters")
39+
40+
841
def ensure_existing_file_path(
942
file_path: Union[str, PathLike[str]],
1043
*,
1144
missing_file_message: str,
1245
not_file_message: str,
1346
) -> str:
14-
if not isinstance(missing_file_message, str):
15-
raise HyperbrowserError("missing_file_message must be a string")
16-
if not missing_file_message.strip():
17-
raise HyperbrowserError("missing_file_message must not be empty")
18-
if any(
19-
ord(character) < 32 or ord(character) == 127
20-
for character in missing_file_message
21-
):
22-
raise HyperbrowserError(
23-
"missing_file_message must not contain control characters"
24-
)
25-
if not isinstance(not_file_message, str):
26-
raise HyperbrowserError("not_file_message must be a string")
27-
if not not_file_message.strip():
28-
raise HyperbrowserError("not_file_message must not be empty")
29-
if any(
30-
ord(character) < 32 or ord(character) == 127 for character in not_file_message
31-
):
32-
raise HyperbrowserError("not_file_message must not contain control characters")
47+
_validate_error_message_text(
48+
missing_file_message,
49+
field_name="missing_file_message",
50+
)
51+
_validate_error_message_text(
52+
not_file_message,
53+
field_name="not_file_message",
54+
)
3355
try:
3456
normalized_path = os.fspath(file_path)
3557
except HyperbrowserError:
@@ -43,17 +65,44 @@ def ensure_existing_file_path(
4365
raise HyperbrowserError("file_path is invalid", original_error=exc) from exc
4466
if not isinstance(normalized_path, str):
4567
raise HyperbrowserError("file_path must resolve to a string path")
46-
if not normalized_path.strip():
68+
try:
69+
stripped_normalized_path = normalized_path.strip()
70+
if not isinstance(stripped_normalized_path, str):
71+
raise TypeError("normalized file_path must be a string")
72+
except HyperbrowserError:
73+
raise
74+
except Exception as exc:
75+
raise HyperbrowserError("file_path is invalid", original_error=exc) from exc
76+
if not stripped_normalized_path:
4777
raise HyperbrowserError("file_path must not be empty")
48-
if normalized_path != normalized_path.strip():
78+
try:
79+
has_surrounding_whitespace = normalized_path != stripped_normalized_path
80+
except HyperbrowserError:
81+
raise
82+
except Exception as exc:
83+
raise HyperbrowserError("file_path is invalid", original_error=exc) from exc
84+
if has_surrounding_whitespace:
4985
raise HyperbrowserError(
5086
"file_path must not contain leading or trailing whitespace"
5187
)
52-
if "\x00" in normalized_path:
88+
try:
89+
contains_null_byte = "\x00" in normalized_path
90+
except HyperbrowserError:
91+
raise
92+
except Exception as exc:
93+
raise HyperbrowserError("file_path is invalid", original_error=exc) from exc
94+
if contains_null_byte:
5395
raise HyperbrowserError("file_path must not contain null bytes")
54-
if any(
55-
ord(character) < 32 or ord(character) == 127 for character in normalized_path
56-
):
96+
try:
97+
contains_control_character = any(
98+
ord(character) < 32 or ord(character) == 127
99+
for character in normalized_path
100+
)
101+
except HyperbrowserError:
102+
raise
103+
except Exception as exc:
104+
raise HyperbrowserError("file_path is invalid", original_error=exc) from exc
105+
if contains_control_character:
57106
raise HyperbrowserError("file_path must not contain control characters")
58107
try:
59108
path_exists = bool(os.path.exists(normalized_path))

tests/test_file_utils.py

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,3 +336,157 @@ def __fspath__(self) -> str:
336336
)
337337

338338
assert exc_info.value.original_error is None
339+
340+
341+
def test_ensure_existing_file_path_wraps_missing_message_strip_runtime_errors(
342+
tmp_path: Path,
343+
):
344+
class _BrokenMissingMessage(str):
345+
def strip(self, chars=None): # type: ignore[override]
346+
_ = chars
347+
raise RuntimeError("missing message strip exploded")
348+
349+
file_path = tmp_path / "file.txt"
350+
file_path.write_text("content")
351+
352+
with pytest.raises(
353+
HyperbrowserError, match="Failed to normalize missing_file_message"
354+
) as exc_info:
355+
ensure_existing_file_path(
356+
str(file_path),
357+
missing_file_message=_BrokenMissingMessage("missing"),
358+
not_file_message="not-file",
359+
)
360+
361+
assert isinstance(exc_info.value.original_error, RuntimeError)
362+
363+
364+
def test_ensure_existing_file_path_wraps_missing_message_character_validation_failures(
365+
tmp_path: Path,
366+
):
367+
class _BrokenMissingMessage(str):
368+
def strip(self, chars=None): # type: ignore[override]
369+
_ = chars
370+
return self
371+
372+
def __iter__(self):
373+
raise RuntimeError("missing message iteration exploded")
374+
375+
file_path = tmp_path / "file.txt"
376+
file_path.write_text("content")
377+
378+
with pytest.raises(
379+
HyperbrowserError, match="Failed to validate missing_file_message characters"
380+
) as exc_info:
381+
ensure_existing_file_path(
382+
str(file_path),
383+
missing_file_message=_BrokenMissingMessage("missing"),
384+
not_file_message="not-file",
385+
)
386+
387+
assert isinstance(exc_info.value.original_error, RuntimeError)
388+
389+
390+
def test_ensure_existing_file_path_preserves_hyperbrowser_missing_message_strip_errors(
391+
tmp_path: Path,
392+
):
393+
class _BrokenMissingMessage(str):
394+
def strip(self, chars=None): # type: ignore[override]
395+
_ = chars
396+
raise HyperbrowserError("custom missing message strip failure")
397+
398+
file_path = tmp_path / "file.txt"
399+
file_path.write_text("content")
400+
401+
with pytest.raises(
402+
HyperbrowserError, match="custom missing message strip failure"
403+
) as exc_info:
404+
ensure_existing_file_path(
405+
str(file_path),
406+
missing_file_message=_BrokenMissingMessage("missing"),
407+
not_file_message="not-file",
408+
)
409+
410+
assert exc_info.value.original_error is None
411+
412+
413+
def test_ensure_existing_file_path_wraps_not_file_message_strip_runtime_errors(
414+
tmp_path: Path,
415+
):
416+
class _BrokenNotFileMessage(str):
417+
def strip(self, chars=None): # type: ignore[override]
418+
_ = chars
419+
raise RuntimeError("not-file message strip exploded")
420+
421+
file_path = tmp_path / "file.txt"
422+
file_path.write_text("content")
423+
424+
with pytest.raises(
425+
HyperbrowserError, match="Failed to normalize not_file_message"
426+
) as exc_info:
427+
ensure_existing_file_path(
428+
str(file_path),
429+
missing_file_message="missing",
430+
not_file_message=_BrokenNotFileMessage("not-file"),
431+
)
432+
433+
assert isinstance(exc_info.value.original_error, RuntimeError)
434+
435+
436+
def test_ensure_existing_file_path_wraps_file_path_strip_runtime_errors():
437+
class _BrokenPath(str):
438+
def strip(self, chars=None): # type: ignore[override]
439+
_ = chars
440+
raise RuntimeError("path strip exploded")
441+
442+
with pytest.raises(HyperbrowserError, match="file_path is invalid") as exc_info:
443+
ensure_existing_file_path(
444+
_BrokenPath("/tmp/path.txt"),
445+
missing_file_message="missing",
446+
not_file_message="not-file",
447+
)
448+
449+
assert isinstance(exc_info.value.original_error, RuntimeError)
450+
451+
452+
def test_ensure_existing_file_path_wraps_file_path_contains_runtime_errors():
453+
class _BrokenPath(str):
454+
def strip(self, chars=None): # type: ignore[override]
455+
_ = chars
456+
return self
457+
458+
def __contains__(self, item): # type: ignore[override]
459+
_ = item
460+
raise RuntimeError("path contains exploded")
461+
462+
with pytest.raises(HyperbrowserError, match="file_path is invalid") as exc_info:
463+
ensure_existing_file_path(
464+
_BrokenPath("/tmp/path.txt"),
465+
missing_file_message="missing",
466+
not_file_message="not-file",
467+
)
468+
469+
assert isinstance(exc_info.value.original_error, RuntimeError)
470+
471+
472+
def test_ensure_existing_file_path_wraps_file_path_character_iteration_runtime_errors():
473+
class _BrokenPath(str):
474+
def strip(self, chars=None): # type: ignore[override]
475+
_ = chars
476+
return self
477+
478+
def __contains__(self, item): # type: ignore[override]
479+
_ = item
480+
return False
481+
482+
def __iter__(self):
483+
raise RuntimeError("path iteration exploded")
484+
485+
with pytest.raises(HyperbrowserError, match="file_path is invalid") as exc_info:
486+
ensure_existing_file_path(
487+
_BrokenPath("/tmp/path.txt"),
488+
missing_file_message="missing",
489+
not_file_message="not-file",
490+
)
491+
492+
assert isinstance(exc_info.value.original_error, RuntimeError)

0 commit comments

Comments
 (0)