Skip to content

test(yara): write the non-ASCII malformed-rule fixture as UTF-8 - #504

Open
kevin9327 wants to merge 1 commit into
NVIDIA:mainfrom
kevin9327:fix/static-yara-explicit-encoding
Open

test(yara): write the non-ASCII malformed-rule fixture as UTF-8#504
kevin9327 wants to merge 1 commit into
NVIDIA:mainfrom
kevin9327:fix/static-yara-explicit-encoding

Conversation

@kevin9327

Copy link
Copy Markdown
Contributor

Problem

TestHelpers::test_malformed_extra_encoded_rule_does_not_block_builtin_rules is parametrized
with an ASCII payload and a non-ASCII one, then writes the fixture with no encoding:

@pytest.mark.parametrize("payload", ["not base64", "not base64 é"])
def test_malformed_extra_encoded_rule_does_not_block_builtin_rules(self, tmp_path, payload):
    (tmp_path / "bad.yar.b64").write_text(payload)

Path.write_text with no encoding uses locale.getpreferredencoding(False). That is UTF-8
on the CI image, but it is the ANSI code page on Windows and on any non-UTF-8 locale, and many
of those code pages cannot represent é. The second parameter then raises
UnicodeEncodeError while writing the fixture, so the case never reaches its assertion — that
one malformed extra rule file does not stop the builtin rules from loading and matching. The
ASCII parameter passes, so the regression signal is silently halved wherever the locale is not
UTF-8.

The analyzer itself is explicit about the encoding it expects:

def _read_rule_source(rule_file: Path, data: bytes | None = None) -> str:
    ...
    encoded_source = data.decode("utf-8")
    return base64.b64decode("".join(encoded_source.split())).decode("utf-8")

so the fixture should be written as UTF-8 rather than as whatever the ambient locale happens to
be.

Fix

Pass encoding="utf-8" on that one write. The payload stays non-ASCII and stays invalid
base64, so what the case tests is unchanged; it just becomes byte-identical on every locale and
matches how _read_rule_source reads it. Every other write_text in this module writes pure
ASCII and is unaffected either way, so none of them are touched.

Reproduction

Windows 11, Python 3.12.10, locale.getpreferredencoding(False) is cp949. Against unmodified
main (704bc95):

$ python -m pytest -p no:randomly \
    "tests/nodes/analyzers/test_static_yara.py::TestHelpers::test_malformed_extra_encoded_rule_does_not_block_builtin_rules" \
    -q --no-header

        with self.open(mode='w', encoding=encoding, errors=errors, newline=newline) as f:
>           return f.write(data)
                   ^^^^^^^^^^^^^
E           UnicodeEncodeError: 'cp949' codec can't encode character '\xe9' in position 11:
            illegal multibyte sequence

..\Python312\Lib\pathlib.py:1048: UnicodeEncodeError
=========================== short test summary info ===========================
FAILED tests/nodes/analyzers/test_static_yara.py::TestHelpers::test_malformed_extra_encoded_rule_does_not_block_builtin_rules[not base64 \xe9]
1 failed, 1 passed in 7.45s

cp949 is this machine's code page; any ANSI code page without é reproduces it. With this
change, same command:

..                                                                       [100%]
2 passed in 3.94s

Both parameters now execute the assertion.

What must still hold

tests/nodes/analyzers/test_static_yara.py in full:

failed passed
before 1 78
after 0 79

ruff check tests/All checks passed!
ruff format --check tests/126 files already formatted

Diff is 1 file changed, 1 insertion(+), 1 deletion(-).

The malformed encoded-rule case is parametrized with a non-ASCII payload
but writes it with Path.write_text and no encoding, so the file is
encoded with the locale codec. Where that codec cannot represent the
character the case raises UnicodeEncodeError during setup and never
reaches the assertion that a malformed extra rule leaves the builtin
rules loadable.

_read_rule_source decodes rule bytes as UTF-8, so writing the fixture as
UTF-8 also matches how the analyzer reads it.

Signed-off-by: kevin9327 <kevin9327@users.noreply.github.com>

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[SkillSpector Review]

Reviewed head 43a88ed955ee3fce022c4c020e5dcd940b16679f — APPROVE.

Writing the non-ASCII malformed-rule fixture explicitly as UTF-8 makes the test reach the intended invalid-base64 assertion on non-UTF-8 Windows locales and matches the production decoder. I found no required changes.

Required checks pass, but GitHub currently reports mergeStateStatus=BEHIND; update against current main and re-run required checks before merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants