diff --git a/changelog/50903.fixed.md b/changelog/50903.fixed.md new file mode 100644 index 000000000000..912dd06030c2 --- /dev/null +++ b/changelog/50903.fixed.md @@ -0,0 +1 @@ +Fixed the file.comment, file.append, and file.prepend states failing with UnicodeDecodeError when the target file contains bytes that are not valid in the system encoding. diff --git a/salt/states/file.py b/salt/states/file.py index 96bc5cab9141..3fa4899eceea 100644 --- a/salt/states/file.py +++ b/salt/states/file.py @@ -6306,7 +6306,7 @@ def comment(name, regex, char="#", backup=".bak", ignore_missing=False): with salt.utils.files.fopen(name, "rb") as fp_: slines = fp_.read() - slines = slines.decode(__salt_system_encoding__) + slines = slines.decode(__salt_system_encoding__, errors="replace") slines = slines.splitlines(True) # Perform the edit @@ -6314,7 +6314,7 @@ def comment(name, regex, char="#", backup=".bak", ignore_missing=False): with salt.utils.files.fopen(name, "rb") as fp_: nlines = fp_.read() - nlines = nlines.decode(__salt_system_encoding__) + nlines = nlines.decode(__salt_system_encoding__, errors="replace") nlines = nlines.splitlines(True) # Check the result @@ -6643,7 +6643,7 @@ def append( with salt.utils.files.fopen(name, "rb") as fp_: slines = fp_.read() - slines = slines.decode(__salt_system_encoding__) + slines = slines.decode(__salt_system_encoding__, errors="replace") slines = slines.splitlines() append_lines = [] @@ -6689,7 +6689,7 @@ def append( with salt.utils.files.fopen(name, "rb") as fp_: nlines = fp_.read() - nlines = nlines.decode(__salt_system_encoding__) + nlines = nlines.decode(__salt_system_encoding__, errors="replace") nlines = nlines.splitlines() if slines != nlines: @@ -6928,7 +6928,7 @@ def prepend( with salt.utils.files.fopen(name, "rb") as fp_: slines = fp_.read() - slines = slines.decode(__salt_system_encoding__) + slines = slines.decode(__salt_system_encoding__, errors="replace") slines = slines.splitlines(True) count = 0 @@ -6976,7 +6976,7 @@ def prepend( with salt.utils.files.fopen(name, "rb") as fp_: # read as many lines of target file as length of user input contents = fp_.read() - contents = contents.decode(__salt_system_encoding__) + contents = contents.decode(__salt_system_encoding__, errors="replace") contents = contents.splitlines(True) target_head = contents[0 : len(preface)] target_lines = [] @@ -6995,7 +6995,7 @@ def prepend( with salt.utils.files.fopen(name, "rb") as fp_: nlines = fp_.read() - nlines = nlines.decode(__salt_system_encoding__) + nlines = nlines.decode(__salt_system_encoding__, errors="replace") nlines = nlines.splitlines(True) if slines != nlines: diff --git a/tests/pytests/unit/states/file/test_append.py b/tests/pytests/unit/states/file/test_append.py new file mode 100644 index 000000000000..af788fee8165 --- /dev/null +++ b/tests/pytests/unit/states/file/test_append.py @@ -0,0 +1,89 @@ +import builtins + +import pytest + +import salt.states.file as filestate +import salt.utils.files +from tests.support.mock import MagicMock, patch + + +@pytest.fixture +def configure_loader_modules(): + return { + filestate: { + "__env__": "base", + "__salt__": {}, + "__opts__": {"test": False, "cachedir": ""}, + "__instance_id__": "", + "__low__": {}, + "__utils__": {}, + } + } + + +def test_append_file_encoding_mismatch(tmp_path): + """ + file.append must not raise UnicodeDecodeError when the target file + contains bytes that are not valid in the system encoding. The decoded + contents are only used to build the diff, so undecodable bytes should + be tolerated rather than aborting the state. + + Regression test for #50903. + """ + name = tmp_path / "bugfile" + # 0xed is not valid ASCII and not valid UTF-8 on its own + name.write_bytes(b"abc\xedxyz\n") + + salt_mock = { + "file.search": MagicMock(return_value=False), + "file.append": MagicMock(return_value=None), + } + utils_mock = {"files.is_text": MagicMock(return_value=True)} + + with patch.object(builtins, "__salt_system_encoding__", "ascii"), patch.dict( + filestate.__salt__, salt_mock + ), patch.dict(filestate.__utils__, utils_mock): + result = filestate.append(name=str(name), text="cheese") + + assert result["result"] is True + salt_mock["file.append"].assert_called_once() + + +def test_append_clean_encoding_unaffected_50903(tmp_path): + """ + Guard against overcorrection of the #50903 fix: decoding with + errors="replace" must not change behaviour for files that decode + cleanly in the system encoding. The diff must still be generated, + contain the original non-ASCII text unmangled, and hold no U+FFFD + replacement characters. This test passes with and without the fix. + """ + name = tmp_path / "cleanfile" + # Valid UTF-8 content that decodes cleanly with the utf-8 system encoding + name.write_bytes("h\u00e9llo\n".encode("utf-8")) + + def fake_append(fname, args=None): + # Perform the real append so the state's second read sees a change + with salt.utils.files.fopen(fname, "a", encoding="utf-8") as fp_: + for line in args: + fp_.write(line + "\n") + + salt_mock = { + "file.search": MagicMock(return_value=False), + "file.append": MagicMock(side_effect=fake_append), + } + utils_mock = {"files.is_text": MagicMock(return_value=True)} + + # Production callers (the state compiler running an SLS file.append) + # pass name and text; __salt_system_encoding__ is the locale-derived + # builtin read by the decode sites, so it is patched rather than passed. + with patch.object(builtins, "__salt_system_encoding__", "utf-8"), patch.dict( + filestate.__salt__, salt_mock + ), patch.dict(filestate.__utils__, utils_mock): + result = filestate.append(name=str(name), text="cheese") + + assert result["result"] is True + assert result["comment"] == "Appended 1 lines" + diff = result["changes"]["diff"] + assert "+cheese" in diff + assert "h\u00e9llo" in diff + assert "\ufffd" not in diff diff --git a/tests/pytests/unit/states/file/test_comment.py b/tests/pytests/unit/states/file/test_comment.py index 6ef8b72de28c..82e67b67337e 100644 --- a/tests/pytests/unit/states/file/test_comment.py +++ b/tests/pytests/unit/states/file/test_comment.py @@ -1,3 +1,4 @@ +import builtins import logging import os @@ -115,6 +116,38 @@ def test_comment(): assert filestate.comment(name, regex) == ret +def test_comment_file_encoding_mismatch_50903(tmp_path): + """ + file.comment must not raise UnicodeDecodeError when the target file + contains bytes that are not valid in the system encoding. The decoded + contents are only used to build the diff, so undecodable bytes should + be tolerated rather than aborting the state. + + Regression test for #50903. + """ + name = tmp_path / "fstab" + # 0xed is not valid ASCII and not valid UTF-8 on its own + name.write_bytes(b"bind 127.0.0.1\nabc\xedxyz\n") + + salt_mock = { + # First search: uncommented pattern found; second: commented after edit + "file.search": MagicMock(side_effect=[True, True]), + "file.comment_line": MagicMock(return_value=True), + } + + # Production callers (the state compiler running an SLS file.comment) + # pass name and regex; __salt_system_encoding__ is the locale-derived + # builtin read by the decode sites, so it is patched rather than passed. + with patch.object(builtins, "__salt_system_encoding__", "ascii"), patch.dict( + filestate.__salt__, salt_mock + ): + result = filestate.comment(str(name), "^bind 127.0.0.1") + + assert result["result"] is True + assert result["comment"] == "Commented lines successfully" + salt_mock["file.comment_line"].assert_called_once() + + # 'uncomment' function tests: 1 def test_uncomment(): """ diff --git a/tests/pytests/unit/states/file/test_prepend.py b/tests/pytests/unit/states/file/test_prepend.py index db528519ec0f..60f66a30eff3 100644 --- a/tests/pytests/unit/states/file/test_prepend.py +++ b/tests/pytests/unit/states/file/test_prepend.py @@ -1,3 +1,4 @@ +import builtins import logging import os @@ -114,3 +115,34 @@ def test_prepend(): comt = "Prepended 1 lines" ret.update({"comment": comt, "result": True, "changes": {}}) assert filestate.prepend(name, text=text) == ret + + +def test_prepend_file_encoding_mismatch_50903(tmp_path): + """ + file.prepend must not raise UnicodeDecodeError when the target file + contains bytes that are not valid in the system encoding. The decoded + contents are only used to build the diff, so undecodable bytes should + be tolerated rather than aborting the state. + + Regression test for #50903. + """ + name = tmp_path / "motd" + # 0xed is not valid ASCII and not valid UTF-8 on its own + name.write_bytes(b"abc\xedxyz\n") + + salt_mock = { + "file.search": MagicMock(return_value=False), + "file.prepend": MagicMock(return_value=True), + } + + # Production callers (the state compiler running an SLS file.prepend) + # pass name and text; __salt_system_encoding__ is the locale-derived + # builtin read by the decode sites, so it is patched rather than passed. + with patch.object(builtins, "__salt_system_encoding__", "ascii"), patch.dict( + filestate.__salt__, salt_mock + ): + result = filestate.prepend(name=str(name), text="Trust no one") + + assert result["result"] is True + assert result["comment"] == "Prepended 1 lines" + salt_mock["file.prepend"].assert_called_once()