From ede982cd15cf883dbfd5fb58ef8203d26e797831 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Le Borgne Date: Sun, 4 Oct 2026 13:37:49 +0100 Subject: [PATCH 1/2] Fix panic on modules declaring their encoding as e.g. latin-1 Modules can declare their encoding (PEP 263) using any name that Python accepts, such as `latin-1`, `utf_8` or `euc_jp`. But encoding_rs only knows the WHATWG labels (`latin1`, `utf-8`, `euc-jp`...), so these names weren't recognized, and building the graph panicked with: pyo3_runtime.PanicException: called `Result::unwrap()` on an `Err` value: PyErr { type: , value: TypeError('function takes exactly 5 arguments (1 given)'), traceback: None } Normalize the declared name like Python does before looking it up. Modules that really can't be decoded also caused this panic, as the error was created as a UnicodeDecodeError, which can't be created from just a message, and was then unwrapped. Raise it as a UnicodeError instead, which keeps the message naming the file. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GwUvhmzgfkiZ6vo4uGyesN --- AUTHORS.rst | 1 + CHANGELOG.rst | 3 ++ rust/src/errors.rs | 4 ++ rust/src/filesystem.rs | 48 +++++++++++++++----- rust/src/import_scanning.rs | 4 +- tests/functional/test_encoding_handling.py | 51 ++++++++++++++++++++++ tests/functional/test_error_handling.py | 34 +++++++++++++++ 7 files changed, 133 insertions(+), 12 deletions(-) diff --git a/AUTHORS.rst b/AUTHORS.rst index 9f408c53..c669f9dd 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -13,3 +13,4 @@ Authors * Nathan McDougall - https://github.com/nathanjmcdougall * Oleksandr Zaiats - https://github.com/z4y4ts * Nikhil Dabas - https://github.com/ndabas +* Pierre-Yves Le Borgne - https://github.com/pylaterreur diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 8ee3b38a..51c6c684 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -7,6 +7,9 @@ latest * Fix missing macOS wheels for regular (non-freethreaded) Python 3.14+ (https://github.com/python-grimp/grimp/issues/317). +* Fix panic when a module declares its encoding using a name that Python accepts but that isn't a + WHATWG label, such as ``latin-1`` or ``utf_8``. Raise a ``UnicodeError`` naming the file, rather + than panicking, if a module can't be decoded. 3.17 (2026-09-04) ----------------- diff --git a/rust/src/errors.rs b/rust/src/errors.rs index dee3e599..ca9eacad 100644 --- a/rust/src/errors.rs +++ b/rust/src/errors.rs @@ -39,6 +39,9 @@ pub enum GrimpError { #[error("Cache file {0} was written by a different version of Grimp.")] CacheVersionMismatch(String), + + #[error(transparent)] + FileReadError(PyErr), } pub type GrimpResult = Result; @@ -61,6 +64,7 @@ impl From for PyErr { GrimpError::CacheVersionMismatch(_) => { exceptions::CacheVersionMismatch::new_err(value.to_string()) } + GrimpError::FileReadError(error) => error, } } } diff --git a/rust/src/filesystem.rs b/rust/src/filesystem.rs index badab533..4a7d57bc 100644 --- a/rust/src/filesystem.rs +++ b/rust/src/filesystem.rs @@ -1,5 +1,5 @@ use itertools::Itertools; -use pyo3::exceptions::{PyFileNotFoundError, PyTypeError, PyUnicodeDecodeError}; +use pyo3::exceptions::{PyFileNotFoundError, PyTypeError, PyUnicodeError}; use pyo3::prelude::*; use regex::Regex; use std::collections::HashMap; @@ -14,6 +14,33 @@ use unindent::unindent; static ENCODING_RE: LazyLock = LazyLock::new(|| Regex::new(r"^[ \t\f]*#.*?coding[:=][ \t]*([-_.a-zA-Z0-9]+)").unwrap()); +/// Look up the encoding named in a Python source file's encoding declaration. +/// +/// encoding_rs only knows the WHATWG labels, but Python accepts other spellings too, such as +/// `latin-1`, `utf_8`, `utf-8-sig` or `euc_jp`. +fn lookup_encoding(name: &str) -> Option<&'static encoding_rs::Encoding> { + encoding_rs::Encoding::for_label(name.as_bytes()).or_else(|| { + // Normalize the name like Python does: see `_get_normal_name` in Lib/tokenize.py, which + // special cases UTF-8 and Latin-1. Python's codec lookup also ignores the difference + // between underscores and hyphens. + let normalized = name.to_ascii_lowercase().replace('_', "-"); + let is_spelling_of = |canonical: &str| { + normalized == canonical || normalized.starts_with(&format!("{canonical}-")) + }; + let label = if is_spelling_of("utf-8") { + "utf-8" + } else if ["latin-1", "iso-8859-1", "iso-latin-1"] + .into_iter() + .any(is_spelling_of) + { + "iso-8859-1" + } else { + &normalized + }; + encoding_rs::Encoding::for_label(label.as_bytes()) + }) +} + pub trait FileSystem: Send + Sync { fn sep(&self) -> &str; @@ -109,16 +136,17 @@ impl FileSystem for RealBasicFileSystem { } } + // Use UnicodeError rather than UnicodeDecodeError, as the latter can't be created from just + // a message. if let Some(enc_name) = detected_encoding { - let encoding = - encoding_rs::Encoding::for_label(enc_name.as_bytes()).ok_or_else(|| { - PyUnicodeDecodeError::new_err(format!( - "Failed to decode file {file_name} (unknown encoding '{enc_name}')" - )) - })?; + let encoding = lookup_encoding(&enc_name).ok_or_else(|| { + PyUnicodeError::new_err(format!( + "Failed to decode file {file_name} (unknown encoding '{enc_name}')" + )) + })?; let (decoded_s, _, had_errors) = encoding.decode(&bytes); if had_errors { - Err(PyUnicodeDecodeError::new_err(format!( + Err(PyUnicodeError::new_err(format!( "Failed to decode file {file_name} with encoding '{enc_name}'" ))) } else { @@ -127,9 +155,7 @@ impl FileSystem for RealBasicFileSystem { } else { // Default to UTF-8 if no encoding is specified String::from_utf8(bytes).map_err(|e| { - PyUnicodeDecodeError::new_err(format!( - "Failed to decode file {file_name} as UTF-8: {e}" - )) + PyUnicodeError::new_err(format!("Failed to decode file {file_name} as UTF-8: {e}")) }) } } diff --git a/rust/src/import_scanning.rs b/rust/src/import_scanning.rs index 033d3526..1f9283a9 100644 --- a/rust/src/import_scanning.rs +++ b/rust/src/import_scanning.rs @@ -130,7 +130,9 @@ fn scan_for_imports_no_py_single_module( let found_package_for_module = found_packages_by_module[module]; let module_filename = _determine_module_filename(module, found_package_for_module, file_system).unwrap(); - let module_contents = file_system.read(&module_filename).unwrap(); + let module_contents = file_system + .read(&module_filename) + .map_err(GrimpError::FileReadError)?; let imported_objects = import_parsing::parse_imports_from_code(&module_contents, &module_filename)?; diff --git a/tests/functional/test_encoding_handling.py b/tests/functional/test_encoding_handling.py index f1c148ef..d4cf556c 100644 --- a/tests/functional/test_encoding_handling.py +++ b/tests/functional/test_encoding_handling.py @@ -1,3 +1,5 @@ +import pytest + import grimp @@ -41,3 +43,52 @@ def test_build_graph_of_non_utf8_source(): "line_contents": "from .imported import π", }, ] == result + + +@pytest.mark.parametrize( + "declared_encoding, codec, imported_name", + ( + # Python special cases these spellings of UTF-8 and Latin-1. + ("utf_8", "utf-8", "π"), + ("UTF-8-sig", "utf-8", "π"), + ("latin-1", "latin-1", "jalapeño"), + ("latin_1", "latin-1", "jalapeño"), + ("iso_8859_1", "latin-1", "jalapeño"), + # Python also accepts underscores in place of hyphens in other encoding names. + ("euc_jp", "euc_jp", "ラーメン"), + ("iso8859_15", "iso8859_15", "jalapeño"), + ), +) +def test_build_graph_of_source_declaring_encoding_with_python_specific_name( + tmp_path, monkeypatch, declared_encoding, codec, imported_name +): + """ + Tests we can cope with source files that declare their encoding using a name that Python + accepts, but that isn't a WHATWG encoding label. + """ + package_directory = tmp_path / "declaredencodingpackage" + package_directory.mkdir() + (package_directory / "__init__.py").write_text("") + (package_directory / "imported.py").write_text("") + (package_directory / "importer.py").write_bytes( + f"# -*- coding: {declared_encoding} -*-\nfrom .imported import {imported_name}\n".encode( + codec + ) + ) + monkeypatch.syspath_prepend(str(tmp_path)) + + graph = grimp.build_graph("declaredencodingpackage", cache_dir=None) + + result = graph.get_import_details( + importer="declaredencodingpackage.importer", imported="declaredencodingpackage.imported" + ) + + assert [ + { + "importer": "declaredencodingpackage.importer", + "imported": "declaredencodingpackage.imported", + "is_lazy": False, + "line_number": 2, + "line_contents": f"from .imported import {imported_name}", + }, + ] == result diff --git a/tests/functional/test_error_handling.py b/tests/functional/test_error_handling.py index 955cc14f..de48d4a6 100644 --- a/tests/functional/test_error_handling.py +++ b/tests/functional/test_error_handling.py @@ -1,3 +1,4 @@ +import re from pathlib import Path import pytest @@ -19,3 +20,36 @@ def test_syntax_error_includes_module(): filename=filename, lineno=5, text="fromb . import two" ) assert expected_exception == excinfo.value + + +@pytest.mark.parametrize( + "contents, expected_problem", + ( + pytest.param(b"x = '\xff'\n", "as UTF-8", id="invalid-utf-8"), + pytest.param( + b"# -*- coding: euc-jp -*-\nx = '\xff\xff'\n", + "with encoding 'euc-jp'", + id="invalid-for-declared-encoding", + ), + pytest.param( + b"# -*- coding: nonexistent -*-\n", + "(unknown encoding 'nonexistent')", + id="unknown-encoding", + ), + ), +) +def test_undecodable_source_raises_unicode_error_including_filename( + tmp_path, monkeypatch, contents, expected_problem +): + package_directory = tmp_path / "undecodablepackage" + package_directory.mkdir() + (package_directory / "__init__.py").write_text("") + module_filename = package_directory / "undecodable.py" + module_filename.write_bytes(contents) + monkeypatch.syspath_prepend(str(tmp_path)) + + with pytest.raises( + UnicodeError, + match=re.escape(f"Failed to decode file {module_filename} {expected_problem}"), + ): + build_graph("undecodablepackage", cache_dir=None) From 06db27090f0c69bafd991a5fc57b1498ff396686 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Le Borgne Date: Sun, 4 Oct 2026 16:02:08 +0100 Subject: [PATCH 2/2] Keep Python exceptions out of GrimpError The previous commit stored file read errors in GrimpError as a PyErr. GrimpError's Debug and Display implementations then call into Python, so the Rust test binaries started linking libpython, and `just test-rust` failed wherever libpython isn't on the loader path: error while loading shared libraries: libpython3.13.so.1.0: cannot open shared object file: No such file or directory Make FileSystem::read return Rust errors instead (GrimpError::FileNotFound and GrimpError::UndecodableFile), and convert them to FileNotFoundError and UnicodeError where they reach Python, like the other GrimpError variants. The exception types and messages are unchanged. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GwUvhmzgfkiZ6vo4uGyesN --- rust/src/errors.rs | 13 +++++++++---- rust/src/filesystem.rs | 27 ++++++++++++++------------- rust/src/import_scanning.rs | 4 +--- 3 files changed, 24 insertions(+), 20 deletions(-) diff --git a/rust/src/errors.rs b/rust/src/errors.rs index ca9eacad..b2ba6a25 100644 --- a/rust/src/errors.rs +++ b/rust/src/errors.rs @@ -1,6 +1,6 @@ use crate::exceptions; use pyo3::PyErr; -use pyo3::exceptions::PyValueError; +use pyo3::exceptions::{PyFileNotFoundError, PyUnicodeError, PyValueError}; use ruff_python_parser::ParseError as RuffParseError; use thiserror::Error; @@ -40,8 +40,11 @@ pub enum GrimpError { #[error("Cache file {0} was written by a different version of Grimp.")] CacheVersionMismatch(String), - #[error(transparent)] - FileReadError(PyErr), + #[error("{0}")] + FileNotFound(String), + + #[error("{0}")] + UndecodableFile(String), } pub type GrimpResult = Result; @@ -64,7 +67,9 @@ impl From for PyErr { GrimpError::CacheVersionMismatch(_) => { exceptions::CacheVersionMismatch::new_err(value.to_string()) } - GrimpError::FileReadError(error) => error, + GrimpError::FileNotFound(_) => PyFileNotFoundError::new_err(value.to_string()), + // Not UnicodeDecodeError, as that can't be created from just a message. + GrimpError::UndecodableFile(_) => PyUnicodeError::new_err(value.to_string()), } } } diff --git a/rust/src/filesystem.rs b/rust/src/filesystem.rs index 4a7d57bc..2b562f18 100644 --- a/rust/src/filesystem.rs +++ b/rust/src/filesystem.rs @@ -1,5 +1,6 @@ +use crate::errors::{GrimpError, GrimpResult}; use itertools::Itertools; -use pyo3::exceptions::{PyFileNotFoundError, PyTypeError, PyUnicodeError}; +use pyo3::exceptions::PyTypeError; use pyo3::prelude::*; use regex::Regex; use std::collections::HashMap; @@ -50,7 +51,7 @@ pub trait FileSystem: Send + Sync { fn exists(&self, file_name: &str) -> bool; - fn read(&self, file_name: &str) -> PyResult; + fn read(&self, file_name: &str) -> GrimpResult; fn write(&mut self, file_name: &str, contents: &str) -> PyResult<()>; } @@ -110,7 +111,7 @@ impl FileSystem for RealBasicFileSystem { Path::new(file_name).is_file() } - fn read(&self, file_name: &str) -> PyResult { + fn read(&self, file_name: &str) -> GrimpResult { // Python files are assumed UTF-8 by default (PEP 686), but they can specify an alternative // encoding, which we need to take into account here. // See https://peps.python.org/pep-0263/ @@ -119,7 +120,7 @@ impl FileSystem for RealBasicFileSystem { let path = Path::new(file_name); let bytes = fs::read(path).map_err(|e| { - PyFileNotFoundError::new_err(format!("Failed to read file {file_name}: {e}")) + GrimpError::FileNotFound(format!("Failed to read file {file_name}: {e}")) })?; let s = String::from_utf8_lossy(&bytes); @@ -136,17 +137,15 @@ impl FileSystem for RealBasicFileSystem { } } - // Use UnicodeError rather than UnicodeDecodeError, as the latter can't be created from just - // a message. if let Some(enc_name) = detected_encoding { let encoding = lookup_encoding(&enc_name).ok_or_else(|| { - PyUnicodeError::new_err(format!( + GrimpError::UndecodableFile(format!( "Failed to decode file {file_name} (unknown encoding '{enc_name}')" )) })?; let (decoded_s, _, had_errors) = encoding.decode(&bytes); if had_errors { - Err(PyUnicodeError::new_err(format!( + Err(GrimpError::UndecodableFile(format!( "Failed to decode file {file_name} with encoding '{enc_name}'" ))) } else { @@ -155,7 +154,9 @@ impl FileSystem for RealBasicFileSystem { } else { // Default to UTF-8 if no encoding is specified String::from_utf8(bytes).map_err(|e| { - PyUnicodeError::new_err(format!("Failed to decode file {file_name} as UTF-8: {e}")) + GrimpError::UndecodableFile(format!( + "Failed to decode file {file_name} as UTF-8: {e}" + )) }) } } @@ -199,7 +200,7 @@ impl PyRealBasicFileSystem { } fn read(&self, file_name: &str) -> PyResult { - self.inner.read(file_name) + Ok(self.inner.read(file_name)?) } fn write(&mut self, file_name: &str, contents: &str) -> PyResult<()> { @@ -279,11 +280,11 @@ impl FileSystem for FakeBasicFileSystem { self.contents.lock().unwrap().contains_key(file_name) } - fn read(&self, file_name: &str) -> PyResult { + fn read(&self, file_name: &str) -> GrimpResult { let contents = self.contents.lock().unwrap(); match contents.get(file_name) { Some(file_contents) => Ok(file_contents.clone()), - None => Err(PyFileNotFoundError::new_err(format!( + None => Err(GrimpError::FileNotFound(format!( "No such file: {file_name}" ))), } @@ -327,7 +328,7 @@ impl PyFakeBasicFileSystem { } fn read(&self, file_name: &str) -> PyResult { - self.inner.read(file_name) + Ok(self.inner.read(file_name)?) } fn write(&mut self, file_name: &str, contents: &str) -> PyResult<()> { diff --git a/rust/src/import_scanning.rs b/rust/src/import_scanning.rs index 1f9283a9..ca5ae99d 100644 --- a/rust/src/import_scanning.rs +++ b/rust/src/import_scanning.rs @@ -130,9 +130,7 @@ fn scan_for_imports_no_py_single_module( let found_package_for_module = found_packages_by_module[module]; let module_filename = _determine_module_filename(module, found_package_for_module, file_system).unwrap(); - let module_contents = file_system - .read(&module_filename) - .map_err(GrimpError::FileReadError)?; + let module_contents = file_system.read(&module_filename)?; let imported_objects = import_parsing::parse_imports_from_code(&module_contents, &module_filename)?;