diff --git a/pre_commit_hooks/check_case_conflict.py b/pre_commit_hooks/check_case_conflict.py index 475c91c4..060b545a 100644 --- a/pre_commit_hooks/check_case_conflict.py +++ b/pre_commit_hooks/check_case_conflict.py @@ -7,6 +7,7 @@ from pre_commit_hooks.util import added_files from pre_commit_hooks.util import cmd_output +from pre_commit_hooks.util import zsplit def lower_set(iterable: Iterable[str]) -> set[str]: @@ -26,7 +27,13 @@ def directories_for(files: set[str]) -> set[str]: def find_conflicting_filenames(filenames: Sequence[str]) -> int: - repo_files = set(cmd_output('git', 'ls-files').splitlines()) + repo_files = set( + zsplit( + cmd_output( + 'git', 'ls-files', '-z', encoding_errors='surrogateescape', + ), + ), + ) repo_files |= directories_for(repo_files) relevant_files = set(filenames) | added_files() relevant_files |= directories_for(relevant_files) diff --git a/pre_commit_hooks/util.py b/pre_commit_hooks/util.py index d6c90ae0..29cb3c26 100644 --- a/pre_commit_hooks/util.py +++ b/pre_commit_hooks/util.py @@ -9,16 +9,21 @@ class CalledProcessError(RuntimeError): def added_files() -> set[str]: - cmd = ('git', 'diff', '--staged', '--name-only', '--diff-filter=A') - return set(cmd_output(*cmd).splitlines()) + cmd = ('git', 'diff', '--staged', '--name-only', '--diff-filter=A', '-z') + return set(zsplit(cmd_output(*cmd, encoding_errors='surrogateescape'))) -def cmd_output(*cmd: str, retcode: int | None = 0, **kwargs: Any) -> str: +def cmd_output( + *cmd: str, + retcode: int | None = 0, + encoding_errors: str = 'strict', + **kwargs: Any, +) -> str: kwargs.setdefault('stdout', subprocess.PIPE) kwargs.setdefault('stderr', subprocess.PIPE) proc = subprocess.Popen(cmd, **kwargs) stdout, stderr = proc.communicate() - stdout = stdout.decode() + stdout = stdout.decode(errors=encoding_errors) if retcode is not None and proc.returncode != retcode: raise CalledProcessError(cmd, retcode, proc.returncode, stdout, stderr) return stdout diff --git a/tests/check_added_large_files_test.py b/tests/check_added_large_files_test.py index 54c4e689..d8b8aa95 100644 --- a/tests/check_added_large_files_test.py +++ b/tests/check_added_large_files_test.py @@ -43,6 +43,15 @@ def test_add_something_giant(temp_git_dir): assert find_large_added_files(['f.py'], 10) == 0 +@pytest.mark.parametrize('filename', ('mañana.txt', 'snowman-☃.bin')) +def test_large_unicode_file_is_checked(temp_git_dir, filename): + with temp_git_dir.as_cwd(): + cmd_output('git', 'config', 'core.quotePath', 'true') + temp_git_dir.join(filename).write('a' * 2048) + cmd_output('git', 'add', '--', filename) + assert main(['--maxkb', '1', filename]) == 1 + + def test_enforce_all(temp_git_dir): with temp_git_dir.as_cwd(): temp_git_dir.join('f.py').write('a' * 10000) diff --git a/tests/check_case_conflict_test.py b/tests/check_case_conflict_test.py index a914f452..96019ede 100644 --- a/tests/check_case_conflict_test.py +++ b/tests/check_case_conflict_test.py @@ -1,5 +1,6 @@ from __future__ import annotations +import os import sys import pytest @@ -96,6 +97,44 @@ def test_file_conflicts_with_committed_file(temp_git_dir): assert find_conflicting_filenames(['F.py']) == 1 +@pytest.mark.parametrize( + ('old_name', 'new_name'), ( + ('café.py', 'CAFÉ.py'), + ('π.py', 'Π.py'), + ), +) +def test_unicode_file_conflicts_with_committed_file( + temp_git_dir, old_name, new_name, +): + with temp_git_dir.as_cwd(): + cmd_output('git', 'config', 'core.quotePath', 'true') + cmd_output('git', 'config', 'core.ignorecase', 'false') + temp_git_dir.join(old_name).write('content') + cmd_output('git', 'add', '--', old_name) + git_commit('-m', 'Add original file') + temp_git_dir.join(new_name).write('content') + cmd_output('git', 'add', '--', new_name) + assert find_conflicting_filenames([new_name]) == 1 + + +@skip_win32 # pragma: win32 no cover +@pytest.mark.parametrize( + ('new_name', 'expected'), ((b'normal.py', 0), (b'BAD-\xff.py', 1)), +) +def test_non_utf8_file_names(temp_git_dir, new_name, expected): + with temp_git_dir.as_cwd(): + cmd_output('git', 'config', 'core.ignorecase', 'false') + with open(b'bad-\xff.py', 'w') as file: + file.write('content') + cmd_output('git', 'add', '-A') + git_commit('-m', 'Add non-UTF-8 filename') + with open(new_name, 'w') as file: + file.write('content') + filename = os.fsdecode(new_name) + cmd_output('git', 'add', '--', filename) + assert find_conflicting_filenames([filename]) == expected + + @skip_win32 # pragma: win32 no cover def test_file_conflicts_with_committed_dir(temp_git_dir): with temp_git_dir.as_cwd(): diff --git a/tests/util_test.py b/tests/util_test.py index 92473e59..2d614cd3 100644 --- a/tests/util_test.py +++ b/tests/util_test.py @@ -1,7 +1,11 @@ from __future__ import annotations +import os +import sys + import pytest +from pre_commit_hooks.util import added_files from pre_commit_hooks.util import CalledProcessError from pre_commit_hooks.util import cmd_output from pre_commit_hooks.util import zsplit @@ -17,6 +21,56 @@ def test_output(): assert ret == 'hi\n' +def test_output_keeps_strict_decoding(): + with pytest.raises(UnicodeDecodeError): + cmd_output( + sys.executable, '-c', + 'import sys; sys.stdout.buffer.write(b"\\xff")', + ) + + +def test_output_surrogateescape(): + ret = cmd_output( + sys.executable, '-c', 'import sys; sys.stdout.buffer.write(b"\\xff")', + encoding_errors='surrogateescape', + ) + assert ret == '\udcff' + assert ret.encode('utf-8', 'surrogateescape') == b'\xff' + + +@pytest.mark.skipif( + sys.platform == 'win32', reason='POSIX filename bytes', +) # pragma: win32 no cover +def test_added_files_preserves_non_utf8_name(temp_git_dir): + raw_name = b'bad-\xff.txt' + filename = os.fsdecode(raw_name) + with temp_git_dir.as_cwd(): + with open(raw_name, 'w') as file: + file.write('content') + cmd_output('git', 'add', '--', filename) + assert added_files() == {filename} + + +skip_windows_filename = pytest.mark.skipif( + sys.platform == 'win32', reason='invalid Windows filename', +) + + +@pytest.mark.parametrize( + 'filename', ( + 'mañana.txt', + pytest.param('line\nbreak.txt', marks=skip_windows_filename), + pytest.param('tab\tfile.txt', marks=skip_windows_filename), + ), +) +def test_added_files_preserves_quoted_names(temp_git_dir, filename): + with temp_git_dir.as_cwd(): + cmd_output('git', 'config', 'core.quotePath', 'true') + temp_git_dir.join(filename).write('content') + cmd_output('git', 'add', '--', filename) + assert added_files() == {filename} + + @pytest.mark.parametrize('out', ('\0f1\0f2\0', '\0f1\0f2', 'f1\0f2\0')) def test_check_zsplits_str_correctly(out): assert zsplit(out) == ['f1', 'f2']