Skip to content

gh-145966: Fix _csv DIALECT_GETATTR macro silently masking non-AttributeError exceptions #145974

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
raminfp wants to merge 3 commits into
base: main
Choose a base branch
from
+28 −7
Open

gh-145966: Fix _csv DIALECT_GETATTR macro silently masking non-AttributeError exceptions #145974

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6f3ae87
gh-145966: Fix _csv DIALECT_GETATTR macro silently masking non-Attrib…
raminfp Mar 15, 2026
c9b4479
gh-145966: Address review feedback: fix PEP-7 alignment, simplify NEW…
raminfp Mar 15, 2026
fbd8b14
Update Modules/_csv.c
raminfp Mar 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions Lib/test/test_csv.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1280,6 +1280,19 @@ class mydialect(csv.Dialect):
self.assertRaises(ValueError, create_invalid, field_name, " ",
skipinitialspace=True)

def test_dialect_getattr_non_attribute_error_propagates(self):
# gh-145966: non-AttributeError exceptions raised by __getattr__
# during dialect attribute lookup must propagate, not be silenced.
class BadDialect:
def __getattr__(self, name):
raise RuntimeError("boom")

with self.assertRaises(RuntimeError):
csv.reader([], dialect=BadDialect())

with self.assertRaises(RuntimeError):
csv.writer(StringIO(), dialect=BadDialect())


class TestSniffer(unittest.TestCase):
sample1 = """\
Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Library/2026-03-15-00-00-00.gh-issue-145966.tCI0uD4I.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Non-:exc:`AttributeError` exceptions raised during dialect attribute lookup
in :mod:`csv` are no longer silently suppressed.
20 changes: 13 additions & 7 deletions Modules/_csv.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,13 +497,19 @@ dialect_new(PyTypeObject *type, PyObject *args, PyObject *kwargs)
Py_XINCREF(skipinitialspace);
Py_XINCREF(strict);
if (dialect != NULL) {
#define DIALECT_GETATTR(v, n) \
do { \
if (v == NULL) { \
v = PyObject_GetAttrString(dialect, n); \
if (v == NULL) \
PyErr_Clear(); \
} \
#define DIALECT_GETATTR(v, n) \
do { \
if (v == NULL) { \
v = PyObject_GetAttrString(dialect, n); \
if (v == NULL) { \
if (PyErr_ExceptionMatches(PyExc_AttributeError)) { \
PyErr_Clear(); \
} \
else { \
goto err; \
} \
} \
} \
Comment on lines +500 to +512
Copy link
Member

@picnixz picnixz Mar 16, 2026

Choose a reason for hiding this comment

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

Suggested change
#define DIALECT_GETATTR(v, n) \
do { \
if (v == NULL) { \
v = PyObject_GetAttrString(dialect, n); \
if (v == NULL) { \
if (PyErr_ExceptionMatches(PyExc_AttributeError)) { \
PyErr_Clear(); \
} \
else { \
goto err; \
} \
} \
} \
#define DIALECT_GETATTR(v, n) \
do { \
if (v == NULL) { \
if (PyObject_GetOptionalAttrString(dialect, n, &v) < 0) { \
goto err; \
} \
} \

Copy link
Member

@picnixz picnixz Mar 16, 2026

Choose a reason for hiding this comment

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

@encukou Something like that?

} while (0)
DIALECT_GETATTR(delimiter, "delimiter");
DIALECT_GETATTR(doublequote, "doublequote");
Expand Down
Loading