From 5705f4dd84042d811a87b093ed12a9891d42d898 Mon Sep 17 00:00:00 2001 From: RP2 Date: Sat, 11 Jul 2026 13:45:31 -0700 Subject: [PATCH] fix: sanitize non-ASCII characters in EXIF metadata strings _decode_ascii's str branch returned non-ASCII characters unchanged. When this string reached tifffile.imwrite(description=...), tifffile's .encode('ascii') raised UnicodeEncodeError, crashing export. The bytes branch had the same issue -- decode('ascii', 'replace') produces U+FFFD (replacement character), which is also non-ASCII and would crash tifffile. Fix both branches by round-tripping through encode/decode ASCII with replacement, producing pure ASCII output with '?' as fallback. Adds 7 tests covering both branches, edge cases, and the exact crash scenario (non-ASCII in EXIF ImageDescription through embed_metadata). Fixes #452 --- negpy/features/metadata/writer.py | 4 +-- tests/metadata/test_writer.py | 45 ++++++++++++++++++++++++++++++- 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/negpy/features/metadata/writer.py b/negpy/features/metadata/writer.py index f7b9157c..684e5003 100644 --- a/negpy/features/metadata/writer.py +++ b/negpy/features/metadata/writer.py @@ -477,9 +477,9 @@ def _fits() -> Optional[bytes]: def _decode_ascii(value: object) -> str | None: if isinstance(value, bytes): - return value.rstrip(b"\x00").decode("ascii", "replace") + value = value.rstrip(b"\x00").decode("ascii", "replace") if isinstance(value, str): - return value + return value.encode("ascii", "replace").decode("ascii") return None diff --git a/tests/metadata/test_writer.py b/tests/metadata/test_writer.py index 13cf2691..5202ae61 100644 --- a/tests/metadata/test_writer.py +++ b/tests/metadata/test_writer.py @@ -7,7 +7,7 @@ import tifffile from negpy.features.metadata.models import MetadataConfig -from negpy.features.metadata.writer import _sanitize_exif, embed_metadata, preserve_source_metadata +from negpy.features.metadata.writer import _decode_ascii, _sanitize_exif, embed_metadata, preserve_source_metadata def _make_tiff_bytes() -> bytes: @@ -235,3 +235,46 @@ def test_does_not_normalize_orientation(self) -> None: loaded = piexif.load(out) assert loaded["0th"].get(piexif.ImageIFD.Orientation) == 6 assert loaded["0th"].get(piexif.ImageIFD.Make) == b"Nikon" + + +class TestDecodeAscii: + """_decode_ascii must always return pure-ASCII str (#452).""" + + def test_bytes_with_non_ascii(self) -> None: + assert _decode_ascii(b"4\xd75 negative") == "4?5 negative" + + def test_str_with_non_ascii(self) -> None: + assert _decode_ascii("4\u00d75 negative") == "4?5 negative" + + def test_pure_ascii_bytes_unchanged(self) -> None: + assert _decode_ascii(b"Portra 400") == "Portra 400" + + def test_pure_ascii_str_unchanged(self) -> None: + assert _decode_ascii("Portra 400") == "Portra 400" + + def test_null_terminated_bytes_stripped(self) -> None: + assert _decode_ascii(b"Hello\x00World\x00") == "Hello\x00World" + + def test_none_returns_none(self) -> None: + assert _decode_ascii(None) is None + assert _decode_ascii(42) is None + + def test_non_ascii_exif_does_not_crash_tiff_metadata_embed(self) -> None: + """Regression: non-ASCII EXIF description must not crash tifffile (#452).""" + image_bytes = _make_tiff_bytes() + source_exif = { + "0th": { + piexif.ImageIFD.Make: b"Nikon", + piexif.ImageIFD.ImageDescription: "4\u00d75 - Portra 400", + }, + "Exif": {}, + "GPS": {}, + "Interop": {}, + "1st": {}, + } + out = embed_metadata(image_bytes, MetadataConfig(), source_exif) + assert out != image_bytes + with tifffile.TiffFile(io.BytesIO(out)) as tf: + desc = tf.pages[0].tags.get("ImageDescription") + # ASCII-safe -- tifffile would reject non-ASCII + desc.value.encode("ascii")