From a7fdd9ab640c1fb91401766770f4a3eb3b539f16 Mon Sep 17 00:00:00 2001 From: Ehsan Rahman Date: Sat, 31 Oct 2026 00:37:24 +0000 Subject: [PATCH 1/2] =?UTF-8?q?Add=2014=20leaky-abstraction=20pitfalls=20(?= =?UTF-8?q?#60=E2=80=93#73),=20100%=20example=20coverage,=20and=20linter?= =?UTF-8?q?=20guardrails?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - New pitfalls & verified examples (#60–#73): - Fundamentals (README.md): #60 all([]) vacuous truth, #61 itertools.groupby unsorted runs & shared cursor, #62 chained comparisons with in/is/==, #63 match/case bare-name capture. - Concurrency (docs/concurrency.md): #64 async for early-break cleanup leaks (contextlib.aclosing), #65 asyncio.gather() orphaned sibling tasks, #66 subprocess.Popen.wait() pipe buffer deadlocks, #67 GIL dict compound-op & free-threaded assumptions. - Object model (docs/object-model.md): #68 built-in dict/list C fast-path method bypasses, #69 __slots__ lost in subclasses, #70 __len__ falsiness. - Interpreter (docs/interpreter.md): #71 deferred/stringified annotations vs runtime introspection. - Data systems (docs/data-systems.md): #72 urllib.parse.urljoin path/origin pitfalls, #73 Unicode normalization & .casefold() vs .lower(). - Improvements to existing entries & repo tooling: - Add benign stdlib runnable examples for #58 and #59 (73/73 coverage). - Highlight isinstance(True, int) JSON validation trap in #14. - Add set/dict un-findable NaN key growth to #44. - Link #53 safe_join TOCTOU discussion to filelock advisories & clarify CWE-22 counts in docs/security.md. - Add copy-paste Ruff/type-checker/runtime guardrail section to README.md. - Add free-threaded CPython 3.13t and 3.14t to .github/workflows/verify.yml. --- .github/workflows/verify.yml | 2 +- CONTRIBUTING.md | 2 +- README.md | 128 ++++++++++++++++++-- docs/concurrency.md | 87 ++++++++++++- docs/data-systems.md | 42 ++++++- docs/interpreter.md | 22 ++++ docs/object-model.md | 76 ++++++++++++ docs/security.md | 10 +- examples/14_bool_is_int.py | 10 +- examples/44_nan_breaks_containers.py | 8 +- examples/58_pickle_deserialization_exec.py | 60 +++++++++ examples/59_key_algorithm_confusion.py | 62 ++++++++++ examples/60_all_empty_vacuous_truth.py | 23 ++++ examples/61_groupby_unsorted_and_shared.py | 28 +++++ examples/62_chained_comparison_trap.py | 20 +++ examples/63_match_case_bare_name.py | 39 ++++++ examples/64_async_generator_cleanup.py | 51 ++++++++ examples/65_asyncio_gather_orphans.py | 55 +++++++++ examples/66_subprocess_pipe_deadlock.py | 26 ++++ examples/67_freethreaded_gil_assumptions.py | 77 ++++++++++++ examples/68_builtin_subclass_bypass.py | 51 ++++++++ examples/69_slots_inheritance_dict.py | 44 +++++++ examples/70_bool_falls_back_to_len.py | 40 ++++++ examples/71_stringified_annotations.py | 49 ++++++++ examples/72_urljoin_drops_path.py | 40 ++++++ examples/73_unicode_normalization_casing.py | 28 +++++ 26 files changed, 1055 insertions(+), 25 deletions(-) create mode 100644 examples/58_pickle_deserialization_exec.py create mode 100644 examples/59_key_algorithm_confusion.py create mode 100644 examples/60_all_empty_vacuous_truth.py create mode 100644 examples/61_groupby_unsorted_and_shared.py create mode 100644 examples/62_chained_comparison_trap.py create mode 100644 examples/63_match_case_bare_name.py create mode 100644 examples/64_async_generator_cleanup.py create mode 100644 examples/65_asyncio_gather_orphans.py create mode 100644 examples/66_subprocess_pipe_deadlock.py create mode 100644 examples/67_freethreaded_gil_assumptions.py create mode 100644 examples/68_builtin_subclass_bypass.py create mode 100644 examples/69_slots_inheritance_dict.py create mode 100644 examples/70_bool_falls_back_to_len.py create mode 100644 examples/71_stringified_annotations.py create mode 100644 examples/72_urljoin_drops_path.py create mode 100644 examples/73_unicode_normalization_casing.py diff --git a/.github/workflows/verify.yml b/.github/workflows/verify.yml index 8a2713b..32ff796 100644 --- a/.github/workflows/verify.yml +++ b/.github/workflows/verify.yml @@ -11,7 +11,7 @@ jobs: strategy: fail-fast: false matrix: - python-version: ["3.10", "3.11", "3.12", "3.13", "3.14"] + python-version: ["3.10", "3.11", "3.12", "3.13", "3.13t", "3.14", "3.14t"] steps: - uses: actions/checkout@v4 - uses: actions/setup-python@v5 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2a2e19a..6ffb09e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -20,6 +20,6 @@ Found a Python behavior that surprised you? Please add it. ``` 4. If there's a documented real-world bug, CVE or fix caused by the pitfall, add an **In the wild** paragraph linking to it. -5. Open a pull request. CI runs the examples on CPython 3.10 to 3.14. +5. Open a pull request. CI runs the examples on CPython 3.10 to 3.14 (including free-threaded builds). If behavior differs between Python versions, say which versions in the README and guard the example with `sys.version_info`. diff --git a/README.md b/README.md index 5f23cdd..1b77a25 100644 --- a/README.md +++ b/README.md @@ -6,13 +6,13 @@ Examining the gap between Python's readable interfaces and the underlying complexity, implicit state, and unexpected behavior they can conceal. -A catalog of 59 Python gotchas, traps and surprising behaviors, from beginner-level surprises to asyncio, threading, CPython internals, data-engineering bugs and security vulnerabilities. Each comes with a minimal reproduction, an explanation of *why* it happens, the fix, and, where one exists, the real-world bug or CVE it caused. +A catalog of 73 Python gotchas, traps and surprising behaviors, from beginner-level surprises to asyncio, threading, CPython internals, data-engineering bugs and security vulnerabilities. Each comes with a minimal reproduction, an explanation of *why* it happens, the fix, and, where one exists, the real-world bug or CVE it caused. Python's syntax prioritizes readability, but readable code does not always make runtime behavior predictable. Some pitfalls arise from implicit state, others from surprising semantics or abstractions that leak underlying details. John Ousterhout's *A Philosophy of Software Design* calls a particularly troublesome symptom of complexity *unknown unknowns*: behavior you cannot predict from the interface in front of you. When the language's mechanics don't match its readable syntax, developers may discover those differences only through runtime bugs. Here is a collection of Python pitfalls developers often discover the hard way, and the evidence that they don't stay in tutorials. -> Every pitfall except two security entries has a runnable, self-checking example in [`examples/`](examples/). CI runs them on CPython 3.10 to 3.14, so the outputs shown here are verified, not remembered. +> Every pitfall has a runnable, self-checking example in [`examples/`](examples/). CI runs them on CPython 3.10 to 3.14 (including free-threaded `3.13t` and `3.14t`), so the outputs shown here are verified, not remembered. ## How the complexity shows up in production @@ -20,7 +20,7 @@ Each row is a kind of hidden complexity, a pitfall that demonstrates the mechani | Hidden complexity | Pitfalls | PyPI advisories since 2023 | Real-world cases | |---|---|---:|---| -| A path join silently discards your prefix | [#53](docs/security.md#53-ospathjoin-discards-your-prefix), [#54](docs/security.md#54-tarfile-trusted-member-paths-until-python-314) | 507 (path traversal, the #1 weakness) | `tarfile` CVE-2007-4559, open for 15 years | +| A path or URL join silently discards your prefix | [#53](docs/security.md#53-ospathjoin-discards-your-prefix), [#54](docs/security.md#54-tarfile-trusted-member-paths-until-python-314), [#72](docs/data-systems.md#72-urllibparseurljoin-drops-base-paths-and-resets-hosts) | 507 (path traversal, the #1 weakness) | `tarfile` CVE-2007-4559, open for 15 years | | Parsing cost grows with the attacker's input | [#56](docs/security.md#56-nested-quantifiers-backtrack-exponentially), [#57](docs/security.md#57-decompression-bombs), [#42](docs/interpreter.md#42-strint-fails-above-4300-digits) | 549 (denial of service) | `urllib3` CVE-2021-33503, `idna` CVE-2024-3651, CPython CVE-2020-10735 | | Loading data runs code | [#58](docs/security.md#58-loading-data-can-run-code) | 210 (deserialization) | `torch.load` CVE-2025-32434; 58 bypasses of `picklescan` | | A convenience hides a protocol decision | [#55](docs/security.md#55-redirects-forward-credentials-to-another-host) | 60 (redirect-related leaks) | `requests` CVE-2018-18074, `urllib3` CVE-2023-43804 | @@ -40,6 +40,8 @@ Each row is a kind of hidden complexity, a pitfall that demonstrates the mechani | 1 | [Iterator exhaustion](#1-iterator-exhaustion-the-observer-effect) | Second `list(it)` is `[]` | Materialize once with `list()` | | 2 | [`zip()` drops data](#2-zip-silently-truncates-and-swallows-an-element) | Missing rows, a lost element | `zip(..., strict=True)` | | 3 | [Generators delay errors](#3-generators-delay-their-errors) | Exception far from the bug | Validate in a normal function | +| 60 | [`all([])` is `True`](#60-all-is-true-vacuous-truth) | Empty list passes every check | Check non-empty + materialize | +| 61 | [`itertools.groupby` splits unsorted runs](#61-itertoolsgroupby-only-groups-consecutive-runs-and-shares-its-iterator) | Duplicate keys, empty groups | Sort first + materialize groups | | | **Mutability & aliasing** | | | | 4 | [Mutable default arguments](#4-mutable-default-arguments) | Data bleeds across calls | `None` sentinel | | 5 | [List multiplication aliasing](#5-list-multiplication-aliasing) | Edit one row, all rows change | List comprehension | @@ -57,6 +59,8 @@ Each row is a kind of hidden complexity, a pitfall that demonstrates the mechani | | **Syntax traps** | | | | 15 | [Implicit string concatenation](#15-implicit-string-concatenation) | Two items become one | Trailing commas + linter | | 16 | [Trailing comma makes a tuple](#16-a-trailing-comma-makes-a-tuple) | `(30,)` instead of `30` | Delete the comma | +| 62 | [Chained comparisons (`in` + `==`)](#62-all-comparisons-chain-including-in-and-is) | `"a" in d == True` is `False` | Drop `== True` / parenthesize | +| 63 | [`match`/`case` bare names capture](#63-bare-names-in-matchcase-capture-everything) | Constant check matches `500` | Use `Enum` dotted names | | | **Standard library surprises** | | | | 17 | [`strip()` takes a character set](#17-strip-takes-a-set-of-characters-not-a-substring) | `"report.txt"` becomes `"repor"` | `removesuffix()` | | 18 | [Banker's rounding](#18-round-uses-bankers-rounding) | `round(2.5) == 2` | `Decimal` + `ROUND_HALF_UP` | @@ -66,13 +70,13 @@ Each row is a kind of hidden complexity, a pitfall that demonstrates the mechani | Page | # | Covers | |------|---|--------| -| [Concurrency: asyncio & threading](docs/concurrency.md) | 20–30 | Garbage-collected tasks, forgotten `await`, blocking the event loop, swallowed cancellation, cross-thread loop calls, non-atomic `+=`, vanishing thread exceptions, `Lock` re-entry, `threading.local` under asyncio, `fork()` deadlocks, daemon threads | -| [Object model & imports](docs/object-model.md) | 31–37 | Properties vs `__getattr__`, `@lru_cache` leaks, `__eq__` vs `__hash__`, `__exit__` swallowing errors, weak refs to bound methods, `mock.patch` targets, double-imported `__main__` | -| [Interpreter internals](docs/interpreter.md) | 38–42 | `python -O` stripping asserts, class-body comprehension scope, `locals()` writes, `id()` reuse, the 4300-digit `int`/`str` limit | -| [Data systems: time, numbers, bytes](docs/data-systems.md) | 43–52 | `hash()` sharding, NaN in containers, nanosecond timestamps in floats and `datetime`, float prices, DST arithmetic, fixed UTC offsets, `utcnow()`, `struct` alignment, non-standard JSON | +| [Concurrency: asyncio, threading & subprocesses](docs/concurrency.md) | 20–30, 64–67 | Garbage-collected tasks, forgotten `await`, blocking the event loop, swallowed cancellation, cross-thread loop calls, `async for` cleanup leaks, `gather()` orphaned siblings, non-atomic `+=`, vanishing thread exceptions, `Lock` re-entry, `threading.local` under asyncio, `fork()` deadlocks, daemon threads, `Popen.wait()` pipe deadlocks, GIL `dict` assumptions | +| [Object model & imports](docs/object-model.md) | 31–37, 68–70 | Properties vs `__getattr__`, `@lru_cache` leaks, `__eq__` vs `__hash__`, `__exit__` swallowing errors, weak refs to bound methods, `mock.patch` targets, double-imported `__main__`, built-in `dict` C fast-path bypasses, `__slots__` lost in subclasses, `__len__` falsiness | +| [Interpreter internals](docs/interpreter.md) | 38–42, 71 | `python -O` stripping asserts, class-body comprehension scope, `locals()` writes, `id()` reuse, the 4300-digit `int`/`str` limit, deferred annotations vs runtime checks | +| [Data systems: time, numbers, bytes & text](docs/data-systems.md) | 43–52, 72–73 | `hash()` sharding, NaN in containers, nanosecond timestamps in floats and `datetime`, float prices, DST arithmetic, fixed UTC offsets, `utcnow()`, `struct` alignment, non-standard JSON, `urljoin` path/origin traps, Unicode normalization & casefolding | | [Security: where complexity becomes a CVE](docs/security.md) | 53–59 | `os.path.join` with absolute paths, `tarfile` extraction, credentials forwarded on redirect, regex backtracking, decompression bombs, unsafe deserialization, signing-key confusion | -See also [In the wild](#in-the-wild-home-assistant) for how a large production codebase hits and guards against these. +See also [In the wild](#in-the-wild-home-assistant) and [Making it fail loudly](#making-it-fail-loudly-linter--runtime-guardrails) for how production codebases detect and prevent these automatically. ### Part III: Research @@ -170,6 +174,41 @@ def _chunks(data, size): [Example](examples/03_lazy_generator_errors.py) +### 60. `all([])` Is `True` (Vacuous Truth) + +`all()` checks that *no* element is falsy, so on an empty sequence it returns `True`. In validation or authorization logic, an empty list—or worse, an iterator that was already consumed earlier (see [#1](#1-iterator-exhaustion-the-observer-effect))—silently passes every check. + +```python +# The Trap +users = [] +if all(u["is_admin"] for u in users): + grant_admin_access() # Runs when users is empty! +``` + +**Why:** Standard mathematical logic defines universal quantification over the empty set as true (`all(xs) == (not any(not x for x in xs))`). + +**The fix:** Materialize iterators once and explicitly require at least one element when emptiness is invalid: `bool(users) and all(u["is_admin"] for u in users)`. + +[Example](examples/60_all_empty_vacuous_truth.py) + +### 61. `itertools.groupby` Only Groups Consecutive Runs (and Shares Its Iterator) + +`itertools.groupby()` looks like SQL's `GROUP BY`, but it behaves like Unix `uniq`: it starts a new group whenever the key changes, without sorting. On top of that, every group shares one underlying iterator—if you save `(key, group)` tuples to inspect later, advancing to the next group drains the previous one, leaving earlier groups empty. + +```python +# The Trap +events = [("alice", 10), ("bob", 20), ("alice", 5)] +print([k for k, _ in groupby(events, key=lambda r: r[0])]) +# Output: ['alice', 'bob', 'alice'] (two 'alice' groups!) + +saved = list(groupby(sorted(events), key=lambda r: r[0])) +print(list(saved[0][1])) # Output: [] (drained when groupby moved to 'bob') +``` + +**The fix:** Sort with the exact same key function first and materialize each group inside the comprehension (`[(k, list(g)) for k, g in groupby(sorted(data, key=fn), key=fn)]`), or group in a single O(n) pass without sorting using `collections.defaultdict(list)`. + +[Example](examples/61_groupby_unsorted_and_shared.py) + --- ## Mutability & aliasing @@ -422,9 +461,9 @@ print({0, False}) # Output: {0} print(True + True) # Output: 2 ``` -Note the result keeps the *first* key (`1`) but the *last* value. This matters when you mix flags and counts in one mapping, `Counter`, or JSON-like structure. +Note the result keeps the *first* key (`1`) but the *last* value. This matters when you mix flags and counts in one mapping, `Counter`, or JSON-like structure. It also bites runtime validation: `isinstance(True, int)` is `True`, so `isinstance(payload["limit"], int)` silently accepts JSON `{"limit": true}`. -**The fix:** Don't mix `bool` and numeric keys. If the type genuinely matters, include it in the key: `(type(k), k)`. +**The fix:** Don't mix `bool` and numeric keys; if the type genuinely matters, include it in the key: `(type(k), k)`. When validating integers at runtime, exclude booleans explicitly: `isinstance(x, int) and not isinstance(x, bool)`. [Example](examples/14_bool_is_int.py) @@ -467,6 +506,42 @@ Meanwhile `(30)` is just `30`. You need `(30,)` for a one-element tuple. [Example](examples/16_trailing_comma_tuple.py) +### 62. All Comparisons Chain (Including `in` and `is`) + +`0 < x < 10` is one of Python's nicest features, because Python expands `a op1 b op2 c` into `(a op1 b) and (b op2 c)`. Less obviously, `in`, `not in`, `is`, `is not` and `==` are *all* in the same comparison precedence group, so they chain with each other too. + +```python +# The Trap +d = {"a": 1} +print("a" in d == True) # Output: False (("a" in d) and (d == True)) +print(False == False in [False]) # Output: True ((False == False) and (False in [False])) +``` + +**The fix:** Never compare boolean expressions with `== True` or `is True`—write `if "a" in d:` directly (Ruff's `E712` flags equality comparisons to `True`/`False`). When you genuinely need to compare the result of a comparison, add parentheses: `("a" in d) == expected`. + +[Example](examples/62_chained_comparison_trap.py) + +### 63. Bare Names in `match`/`case` Capture Everything + +In a `match` statement, a bare identifier like `HTTP_OK` does **not** compare against the variable `HTTP_OK`. It acts as an unconditional capture pattern that matches *any* value and overwrites `HTTP_OK` in the local scope. Inside a tuple or mapping pattern it doesn't even trigger an unreachable-branch `SyntaxError`. + +```python +# The Trap +HTTP_OK = 200 + +match (500, "database down"): + case (HTTP_OK, body): # Matches 500 and rebinds HTTP_OK = 500! + print(f"ok: {body}") # Output: ok: database down + case _: + print("error") +``` + +**Why:** Structural pattern matching ([PEP 634](https://peps.python.org/pep-0634/)) treats bare names like assignment targets (`x = ...`) and reserves value lookup for dotted names (`Status.OK`). + +**The fix:** Put constants on an `Enum` (or module namespace) so the pattern uses a dotted name (`case (Status.OK, body):`), or add an explicit guard (`case (code, body) if code == HTTP_OK:`). + +[Example](examples/63_match_case_bare_name.py) + --- ## Standard library surprises @@ -546,6 +621,39 @@ print(save()) # Output: saved The lesson generalizes: when a pitfall can't be designed away, make it fail loudly. Every guard rail above turns a silent, intermittent bug into an immediate error with a clear message. +## Making it fail loudly: linter & runtime guardrails + +Many of the pitfalls in this catalog can be caught statically in CI before code ever runs. Adding the rules below to `pyproject.toml` flags them automatically with [Ruff](https://docs.astral.sh/ruff/): + +```toml +[tool.ruff.lint] +extend-select = [ + "B005", # #17: .strip() with multi-character strings + "B006", # #4: mutable default arguments + "B007", # #10: unused loop control variables (shadowing risk) + "B012", # #19: return/break/continue inside finally + "B019", # #32: @lru_cache on methods (instance memory leak) + "B020", # #10: loop variable overrides iterable + "B023", # #9: function/lambda in loop doesn't bind loop variable + "B905", # #2: zip() without explicit strict= + "COM818", # #16: trailing comma creating an accidental bare tuple + "DTZ", # #48–50: naive datetimes and datetime.utcnow() + "E712", # #62: == True / == False (triggers chained-comparison traps) + "ISC001", # #15: implicit string concatenation + "RUF006", # #20: asyncio.create_task() without storing a reference + "S101", # #38: assert used for runtime validation + "S301", # #58: suspicious pickle usage + "S324", # #43: insecure/non-cryptographic hash usage + "ASYNC", # #22: blocking I/O and time.sleep() inside async functions + "TC", # #71: imports behind TYPE_CHECKING needed at runtime +] +``` + +Pair static linting with: + +- **Type checking (`pyright` / `mypy`):** catches unawaited coroutines ([#21](docs/concurrency.md#21-forgetting-await-silently-does-nothing)), unreachable `match` branches ([#63](#63-bare-names-in-matchcase-capture-everything)), and missing `None` checks ([#70](docs/object-model.md#70-defining-__len__-makes-empty-objects-falsy)). +- **Development mode (`python -X dev`) in tests:** enables asyncio debug checks for slow callbacks ([#22](docs/concurrency.md#22-blocking-calls-freeze-the-event-loop)), non-thread-safe loop calls ([#24](docs/concurrency.md#24-loop-methods-arent-thread-safe)), and `ResourceWarning` on unclosed pipes and files ([#66](docs/concurrency.md#66-subprocesspopenwait-deadlocks-when-a-pipe-fills)). + ## Running the examples Each file in [`examples/`](examples/) prints the surprising output and `assert`s it, so a script that runs without error confirms the behavior described above. diff --git a/docs/concurrency.md b/docs/concurrency.md index 96b1a0b..84736dc 100644 --- a/docs/concurrency.md +++ b/docs/concurrency.md @@ -14,13 +14,17 @@ Concurrency bugs are the purest form of *unknown unknowns*. The code reads top t | 22 | [Blocking calls freeze the loop](#22-blocking-calls-freeze-the-event-loop) | Every task stalls at once | `asyncio.to_thread()` | | 23 | [Catching `BaseException`](#23-catching-baseexception-breaks-cancellation) | `cancel()` and timeouts do nothing | Catch `Exception` | | 24 | [Loop methods aren't thread-safe](#24-loop-methods-arent-thread-safe) | Callbacks run late, or never | `call_soon_threadsafe()` | -| | **Threading** | | | +| 64 | [Breaking out of `async for`](#64-breaking-out-of-async-for-delays-cleanup) | Connections stay open after `break` | `contextlib.aclosing()` | +| 65 | [`asyncio.gather()` orphans siblings](#65-asynciogather-leaves-sibling-tasks-running-on-error) | Failed call still mutates state | `asyncio.TaskGroup` | +| | **Threading & Subprocesses** | | | | 25 | [`+=` is not atomic](#25--is-not-atomic) | Lost updates | `threading.Lock` | | 26 | [Exceptions in threads vanish](#26-exceptions-in-threads-vanish) | Failures with no traceback | Call `future.result()` | | 27 | [`Lock` is not reentrant](#27-lock-is-not-reentrant) | Thread deadlocks on itself | `threading.RLock` | | 28 | [`threading.local` in asyncio](#28-threadinglocal-leaks-between-asyncio-tasks) | One request sees another's data | `contextvars` | | 29 | [`fork()` with threads running](#29-fork-copies-locked-locks) | Child process hangs forever | `spawn` / `forkserver` | | 30 | [Daemon threads skip cleanup](#30-daemon-threads-skip-cleanup) | Unflushed data at exit | Stop event + `join()` | +| 66 | [`Popen.wait()` with `PIPE`](#66-subprocesspopenwait-deadlocks-when-a-pipe-fills) | Process hangs past ~64 KB output | `subprocess.run()` / `communicate()` | +| 67 | ["The GIL makes `dict` safe"](#67-the-gil-makes-dict-safe-fails-on-compound-operations) | Duplicate work, `RuntimeError` | Lock compound access | --- @@ -135,9 +139,57 @@ In the example, the callback is delayed from 100 ms to 500 ms. With `loop.set_de [Example](../examples/24_asyncio_cross_thread_calls.py) +### 64. Breaking Out of `async for` Delays Cleanup + +In CPython, breaking out of a synchronous generator loop drops its reference count to zero and runs its `finally` block right away. An async generator's `finally` may `await`, which synchronous `break` and garbage collection cannot do immediately. Breaking early out of `async for` leaves connections, locks or transactions inside the generator open until the loop shuts down (or a later GC pass schedules cleanup). + +```python +# The Trap +async def stream_rows(db): + conn = await db.acquire() + try: + async for row in conn.cursor(): + yield row + finally: + await conn.release() # Not run when the caller breaks! + +async for row in stream_rows(db): + if row.id == target: + break # conn stays checked out of the pool. +``` + +**The fix:** Wrap any async generator you might exit early in `contextlib.aclosing()` (Python 3.10+), which awaits `agen.aclose()` on block exit: + +```python +async with contextlib.aclosing(stream_rows(db)) as rows: + async for row in rows: + if row.id == target: + break +``` + +[Example](../examples/64_async_generator_cleanup.py) + +### 65. `asyncio.gather()` Leaves Sibling Tasks Running on Error + +`await asyncio.gather(validate(), charge_card())` looks like a single unit of work. Yet when `validate()` raises, `gather()` immediately propagates the exception to the caller *without cancelling* `charge_card()`. The sibling task keeps running in the background and can commit writes after your error handler has already returned a failure response. + +```python +# The Trap +try: + await asyncio.gather(validate(), charge_card()) +except ValueError: + return "order rejected" # charge_card() is still running in the background! +``` + +**Why:** `gather()` waits on futures rather than owning task lifetimes, so raising early merely detaches the caller from the remaining tasks. + +**The fix:** Use `asyncio.TaskGroup` (Python 3.11+), which cancels every remaining task and awaits their termination before exiting the `async with` block. + +[Example](../examples/65_asyncio_gather_orphans.py) + --- -## Threading +## Threading & Subprocesses ### 25. `+=` Is Not Atomic @@ -257,3 +309,34 @@ t.join() ``` [Example](../examples/30_daemon_threads_skip_cleanup.py) + +### 66. `subprocess.Popen.wait()` Deadlocks When a Pipe Fills + +`Popen(..., stdout=subprocess.PIPE)` redirects the child's output into an OS kernel buffer (typically 64 KB on Linux and macOS). Calling `proc.wait()` waits for the child to exit *without draining that pipe*. Small outputs fit in the buffer so every unit test passes; in production, a verbose response fills the pipe, the child blocks on `write()`, and the parent blocks on `wait()` forever. + +```python +# The Trap +proc = subprocess.Popen(cmd, stdout=subprocess.PIPE) +proc.wait() # Hangs once stdout exceeds ~64 KB. +out = proc.stdout.read() +``` + +**The fix:** Use `subprocess.run(cmd, capture_output=True)` (or `out, err = proc.communicate()`), which drains `stdout` and `stderr` concurrently while waiting for exit. If you don't need the output, redirect to `subprocess.DEVNULL` instead of `PIPE`. + +[Example](../examples/66_subprocess_pipe_deadlock.py) + +### 67. "The GIL Makes `dict` Safe" Fails on Compound Operations + +Because individual C-level `dict` lookups and stores happen while holding the GIL, Python lore often treats `dict` as "thread-safe without a lock". That abstraction leaks in three ways: check-then-set (`if k not in cache: cache[k] = ...`) races between the check and the store; mutating a `dict` while another thread iterates it raises `RuntimeError: dictionary changed size during iteration` even with the GIL on; and under free-threaded builds (`python3.13t` / `python3.14t`), the GIL is gone altogether. + +```python +# The Trap +def get_user(uid): + if uid not in cache: # Thread B checks before Thread A stores... + cache[uid] = fetch(uid) # ...both run fetch(uid). + return cache[uid] +``` + +**The fix:** Protect compound read-modify-write sequences and iteration snapshots (`list(shared.items())`) with a `threading.Lock`, regardless of whether the GIL is enabled. + +[Example](../examples/67_freethreaded_gil_assumptions.py) diff --git a/docs/data-systems.md b/docs/data-systems.md index 3b0360c..510f7e6 100644 --- a/docs/data-systems.md +++ b/docs/data-systems.md @@ -18,9 +18,11 @@ These are the pitfalls that matter when Python moves real data: market feeds, se | 48 | [Aware datetime arithmetic is wall-clock](#48-aware-datetime-arithmetic-is-wall-clock-arithmetic) | Jobs fire an hour off, or in a loop | Do arithmetic in UTC | | 49 | [A UTC offset is not a time zone](#49-a-utc-offset-is-not-a-time-zone) | Schedule drifts after DST | Convert to a `ZoneInfo` | | 50 | [`datetime.utcnow()`](#50-datetimeutcnow-is-off-by-your-utc-offset) | Timestamps off by hours | `datetime.now(timezone.utc)` | -| | **Wire formats** | | | +| | **Wire formats & text** | | | | 51 | [`struct` pads and byte-swaps](#51-struct-pads-and-byte-swaps-by-default) | Garbage fields, wrong sizes | Always use `<`, `>` or `!` | | 52 | [`json.dumps` emits invalid JSON](#52-jsondumps-emits-invalid-json) | Other languages reject the payload | `allow_nan=False` | +| 72 | [`urljoin` drops paths and hosts](#72-urllibparseurljoin-drops-base-paths-and-resets-hosts) | `/v1` vanishes, `//host` swaps origin | Normalize `/` + verify origin | +| 73 | [Unicode normalization & casing](#73-visually-identical-unicode-strings-arent-equal) | `"café" != "café"`, `"ß"` mismatches | `normalize("NFC")` + `.casefold()` | --- @@ -57,9 +59,12 @@ float("nan") in [nan] # False (a different NaN object) max([nan, 1, 2]) # nan max([1, nan, 2]) # 2 (depends on position!) sorted([3, nan, 1, 2]) # [3, nan, 1, 2] (not sorted) + +s = {float("nan") for _ in range(5)} +len(s), float("nan") in s # (5, False): 5 ghost entries you can never look up ``` -**Why:** Sorting and `max()` assume that if `a < b` is false then `a >= b`. Every comparison with NaN is false, so that assumption breaks and the result depends on the order in which elements were compared. +**Why:** Sorting and `max()` assume that if `a < b` is false then `a >= b`. Every comparison with NaN is false, so that assumption breaks and the result depends on the order in which elements were compared. Meanwhile `float("nan")` is still hashable, so sets and dicts accept every distinct NaN object as a separate key that never compares equal on lookup. **The fix:** Remove or replace NaNs before aggregating: `[x for x in xs if not math.isnan(x)]`. Market data formats often use a sentinel for a missing price; Databento's [DBN](https://docs.rs/dbn) uses `i64::MAX` (`UNDEF_PRICE`), which its float conversion helpers turn into NaN. Handle that case at the boundary, before it reaches your analytics. @@ -176,7 +181,7 @@ datetime.utcnow().timestamp() - time.time() # +14,400 or +18,000 s (4 or 5 hou --- -## Wire formats +## Wire formats & text ### 51. `struct` Pads and Byte-Swaps by Default @@ -208,3 +213,34 @@ A related trap: Python serializes huge integers exactly, but JavaScript reads JS **The fix:** Pass `allow_nan=False` to fail fast, and convert missing values to `None` (`null`) yourself. Send large integers such as nanosecond timestamps as strings when the consumer might be JavaScript. [Example](../examples/52_json_nan_infinity.py) + +### 72. `urllib.parse.urljoin` Drops Base Paths and Resets Hosts + +Developers reach for `urljoin(base_url, endpoint)` expecting path concatenation like `f"{base}/{endpoint}"`. Instead, `urljoin` implements RFC 3986 relative-reference resolution (how browsers resolve ``). Three surprises follow: a base URL without a trailing `/` loses its last path segment; a leading `/` in `endpoint` wipes out the entire base path; and a scheme-relative `//host` replaces the origin altogether (turning a URL builder into an SSRF or credential-leak primitive; see [#53](security.md#53-ospathjoin-discards-your-prefix) and [#55](security.md#55-redirects-forward-credentials-to-another-host)). + +```python +# The Trap +urljoin("https://api.example.com/v1", "users") # 'https://api.example.com/users' (/v1 gone!) +urljoin("https://api.example.com/v1/", "/users") # 'https://api.example.com/users' +urljoin("https://api.example.com/v1/", "//evil.test/x") # 'https://evil.test/x' +``` + +**The fix:** Reject endpoints that carry their own scheme or host (`urlsplit(endpoint).netloc`), ensure the base ends with `/`, strip a single leading slash with `endpoint.removeprefix("/")`, and verify the joined URL's origin and path prefix still match the base. + +[Example](../examples/72_urljoin_drops_path.py) + +### 73. Visually Identical Unicode Strings Aren't Equal + +Python `str` is a sequence of Unicode code points, not user-perceived characters. `"café"` can be encoded as four code points (`"caf\u00e9"`, NFC) or five (`"cafe\u0301"`, NFD, standard for macOS filenames). They render identically, yet `==` is `False`, `len()` differs, and a `dict` or database lookup misses. Casing has a matching trap: `"ß".lower()` stays `"ß"`, while `"STRASSE".lower()` is `"strasse"`, so `.lower()` fails to match them. + +```python +# The Trap +nfc, nfd = "caf\u00e9", "cafe\u0301" +print(nfc, nfd, nfc == nfd) # café café False +print(len({nfc, nfd})) # 2 separate set entries +print("straße".lower() == "STRASSE".lower()) # False +``` + +**The fix:** Normalize untrusted text at system boundaries with `unicodedata.normalize("NFC", text)`, and use `.casefold()` rather than `.lower()` for case-insensitive matching and deduplication. + +[Example](../examples/73_unicode_normalization_casing.py) diff --git a/docs/interpreter.md b/docs/interpreter.md index e67f0a2..0969d6e 100644 --- a/docs/interpreter.md +++ b/docs/interpreter.md @@ -13,6 +13,7 @@ Some Python behavior only makes sense once you know what the compiler and CPytho | 40 | [Writing to `locals()`](#40-writing-to-locals-does-nothing) | Assignment ignored | Use an explicit dict | | 41 | [`id()` values get reused](#41-id-values-get-reused) | Stale cache hits | Key on the value | | 42 | [4300-digit `int`/`str` limit](#42-strint-fails-above-4300-digits) | `ValueError` on big numbers | `sys.set_int_max_str_digits()` | +| 71 | [Deferred annotations vs runtime checks](#71-deferred-annotations-break-naive-runtime-introspection) | `__annotations__` holds strings, not types | `typing.get_type_hints()` | --- @@ -113,3 +114,24 @@ json.loads("9" * 5000) # ValueError too. **The fix:** Use `sys.set_int_max_str_digits(n)` (or `PYTHONINTMAXSTRDIGITS`), ideally only around code that handles trusted input. Hex, octal and binary conversions are unaffected. [Example](../examples/42_int_str_digit_limit.py) + +### 71. Deferred Annotations Break Naive Runtime Introspection + +Adding `from __future__ import annotations` ([PEP 563](https://peps.python.org/pep-0563/)) stops Python from evaluating type hints at definition time and stores their source text as strings instead. Any decorator, CLI builder or validator that reads `fn.__annotations__` expecting real classes breaks at runtime, and `typing.get_type_hints()` fails with `NameError` if an annotation references a class defined inside a function. + +```python +# The Trap +from __future__ import annotations + +def parse_port(port: int) -> int: + return port + +parse_port.__annotations__["port"] # 'int' (a str, not ) +isinstance(8080, parse_port.__annotations__["port"]) # TypeError! +``` + +**Why:** Static type checkers wanted forward references without runtime import cost, while runtime frameworks (FastAPI, Pydantic, `dataclasses`, dependency injectors) treat annotations as executable metadata. Python 3.14 introduced lazy annotation functions via `annotationlib` ([PEP 649](https://peps.python.org/pep-0649/)) to replace stringification when `__future__` is *not* imported, meaning libraries now encounter eager types, PEP 563 strings, and PEP 649 evaluators side by side. + +**The fix:** Never read `__annotations__` directly for types; call `typing.get_type_hints(fn)` (or `annotationlib.get_annotations()` on 3.14+), and keep runtime-inspected types at module scope where `get_type_hints()` can resolve them (Ruff's `TC001`–`TC003` flag imports moved behind `if TYPE_CHECKING:` that runtime consumers still need). + +[Example](../examples/71_stringified_annotations.py) diff --git a/docs/object-model.md b/docs/object-model.md index 3d8caa4..c2d9bc9 100644 --- a/docs/object-model.md +++ b/docs/object-model.md @@ -15,6 +15,9 @@ Python's attribute lookup, descriptors and import system are a stack of indirect | 35 | [Weak refs to bound methods](#35-weak-references-to-bound-methods-die-instantly) | Callbacks never fire | `weakref.WeakMethod` | | 36 | [`mock.patch` in the wrong place](#36-mockpatch-in-the-wrong-place) | Test hits the real service | Patch where it's used | | 37 | [A script imported twice](#37-a-script-can-be-imported-twice) | `isinstance` fails on your own class | Thin entry point | +| 68 | [Built-in subclasses skip overrides](#68-built-in-subclasses-bypass-overridden-methods) | `.get()` and `.update()` ignore hooks | `collections.UserDict` | +| 69 | [`__slots__` lost in subclasses](#69-subclassing-a-__slots__-class-silently-restores-__dict__) | Subclass silently regains `__dict__` | Declare `__slots__` on child | +| 70 | [`__len__` makes empty objects falsy](#70-defining-__len__-makes-empty-objects-falsy) | `if node:` skips valid leaf nodes | Test `is not None` / `__bool__` | --- @@ -157,3 +160,76 @@ if __name__ == "__main__": **The fix:** Keep the entry-point file thin. Put the real code in modules, and have the entry point import them and call `main()`. Running a package with `python -m package` (with a `__main__.py`) follows the same idea. [Example](../examples/37_main_module_imported_twice.py) + +### 68. Built-in Subclasses Bypass Overridden Methods + +Subclassing `dict`, `list` or `set` and overriding `__getitem__` or `__setitem__` looks like normal object-oriented Python. `d[k]` calls your override, while `d.__init__()`, `d.get()`, `d.update()`, `d.setdefault()` and `{**d}` completely ignore it. + +```python +# The Trap +class LowerKeyDict(dict): + def __setitem__(self, key, value): + super().__setitem__(key.lower(), value) + + def __getitem__(self, key): + return super().__getitem__(key.lower()) + +d = LowerKeyDict(Accept="text/html") +d["X-Token"] = "secret" +d.update({"Cache-Control": "no-store"}) +print(list(d.keys())) # ['Accept', 'x-token', 'Cache-Control'] +print(d.get("X-TOKEN")) # None (d["X-TOKEN"] is 'secret'!) +``` + +**Why:** CPython's built-in types are written in C for speed, and their C methods access internal `PyDict` storage directly rather than dispatching back through Python's method table. C-level performance optimization leaks right through the inheritance model. + +**The fix:** Subclass `collections.UserDict`, `collections.UserList`, or `collections.abc.MutableMapping`, whose methods are written in Python on top of `__getitem__` and `__setitem__`. + +[Example](../examples/68_builtin_subclass_bypass.py) + +### 69. Subclassing a `__slots__` Class Silently Restores `__dict__` + +`__slots__` replaces per-instance `__dict__` storage with fixed C-struct slots, saving memory and catching misspelled attributes. But `__slots__` is not inherited in the way most developers expect: if a subclass omits `__slots__`, Python silently creates a `__dict__` and `__weakref__` on every subclass instance anyway. + +```python +# The Trap +class Point: + __slots__ = ("x", "y") + +class LabeledPoint(Point): + pass # Forgot __slots__! + +p = LabeledPoint() +p.typo_attr = 99 # Succeeds silently, stored in p.__dict__. +``` + +**Why:** Each class body independently controls whether its instances get a `__dict__` descriptor unless `__slots__` suppresses it on *that* class too. + +**The fix:** Declare `__slots__ = ()` on subclasses that add no new attributes (or list only the *new* slot names—repeating parent slot names wastes space). With `@dataclass(slots=True)`, put `slots=True` on every subclass as well. + +[Example](../examples/69_slots_inheritance_dict.py) + +### 70. Defining `__len__` Makes Empty Objects Falsy + +When `bool(obj)` (or `if obj:`) evaluates an instance that doesn't define `__bool__`, Python falls back to `len(obj) != 0`. Any domain object that defines `__len__`—a tree node counting its children, a batch wrapper, a response page—becomes falsy whenever its count happens to be zero, so `if obj:` conflates "empty" with `None`. + +```python +# The Trap +class Node: + def __init__(self, name, children=()): + self.name = name + self.children = list(children) + + def __len__(self): + return len(self.children) + +root = Node("root") +if root: + process(root) # Skipped! len(root) == 0 makes bool(root) False. +``` + +**In the wild:** `xml.etree.ElementTree.Element` hit this so often—`if elem:` evaluated to `False` for valid leaf tags like `ok` because an element's `len()` is its child-tag count—that Python added a multi-release warning urging callers to write `if elem is not None:` instead. + +**The fix:** Always check `if obj is not None:` when testing for optional objects (Ruff's `SIM108` / explicit `None` checks help), and define `def __bool__(self): return True` on entity classes where zero length doesn't mean "absent". + +[Example](../examples/70_bool_falls_back_to_len.py) diff --git a/docs/security.md b/docs/security.md index a29cc7e..1f88be4 100644 --- a/docs/security.md +++ b/docs/security.md @@ -10,7 +10,7 @@ Counts are unique PyPI advisories published since 2023, from the [OSV](https://o | # | Pitfall | Weakness (CWE) | PyPI advisories since 2023 | The fix | |---|---------|----------------|---------------------------:|---------| -| 53 | [`os.path.join` discards your prefix](#53-ospathjoin-discards-your-prefix) | Path traversal (22) | 444, the most common weakness on PyPI | Resolve, then check containment | +| 53 | [`os.path.join` discards your prefix](#53-ospathjoin-discards-your-prefix) | Path traversal (22) | 444 (507 incl. variants) | Resolve, then check containment | | 54 | [`tarfile` trusted member paths](#54-tarfile-trusted-member-paths-until-python-314) | Path traversal (22) | (CVE-2007-4559, in CPython) | `filter="data"` | | 55 | [Redirects forward credentials](#55-redirects-forward-credentials-to-another-host) | Information exposure (200) | 60 redirect-related | Strip headers across origins | | 56 | [Regex backtracking](#56-nested-quantifiers-backtrack-exponentially) | ReDoS (1333) | 80 | Remove nested quantifiers | @@ -18,7 +18,7 @@ Counts are unique PyPI advisories published since 2023, from the [OSV](https://o | 58 | [Loading data runs code](#58-loading-data-can-run-code) | Deserialization (502) | 210 | Use data-only formats | | 59 | [One key parameter, two meanings](#59-one-key-parameter-two-meanings) | Signature verification (347) | 41 | Pin the algorithm | -Entries 53–57 have runnable examples. Entries 58 and 59 are documented without examples, because demonstrating them means writing a working attack. +Every entry has a runnable example (`#58` and `#59` demonstrate the mechanism with benign stdlib callables and local HMAC checks rather than exploit payloads). --- @@ -48,7 +48,7 @@ def safe_join(base, name): return target ``` -`.resolve()` also follows symlinks, so a link planted inside the directory can't point outside it. Better still, don't use client-supplied names as paths at all: generate the filename yourself and store the original as metadata. +`.resolve()` also follows symlinks, so a link planted inside the directory can't point outside it. Note that if a local attacker has write access to the directory while your process runs, they can swap a file for a symlink *between* `safe_join()` and `open()`—the check-then-use race behind [`filelock`'s 20 advisories](dependency-risk.md#4-one-parameter-two-meanings); defend shared local directories with `os.open(..., os.O_NOFOLLOW, dir_fd=...)`. Better still, don't use client-supplied names as paths at all: generate the filename yourself and store the original as metadata. (For the URL equivalent of this trap, see [#72 `urllib.parse.urljoin`](data-systems.md#72-urllibparseurljoin-drops-base-paths-and-resets-hosts).) **In the wild:** Path traversal (CWE-22 and its variants) is the most common weakness in PyPI advisories, with 507 since 2023. It shows up in file servers, archive extraction, model and plugin loaders, and anything that turns a request parameter into a path. @@ -148,7 +148,7 @@ zlib.decompress(bomb) # 50,000,000 bytes in memory **In the wild:** Deserialization of untrusted data (CWE-502) accounts for 210 PyPI advisories since 2023, concentrated in machine-learning tools that load models from the internet. [PyTorch 2.6 changed `torch.load`](https://dev-discuss.pytorch.org/t/bc-breaking-change-torch-load-is-being-flipped-to-use-weights-only-true-by-default-in-the-nightlies-after-137602/2573) to `weights_only=True` by default, and CVE-2025-32434 then showed that even that restricted mode could be bypassed in earlier versions. `picklescan`, a tool built to detect malicious pickles, has had 58 advisories of its own, most of them ways to get a malicious file past it undetected. -*No runnable example: demonstrating this means writing a working payload.* +[Example](../examples/58_pickle_deserialization_exec.py) ### 59. One Key Parameter, Two Meanings @@ -168,4 +168,4 @@ jwt.decode(token, public_key_pem, algorithms=["RS256"]) **In the wild:** PyJWT has had 23 advisories since 2023, and 15 of them involve keys or algorithms: public keys in DER form, JWK containers or whitespace-mutated PEM accepted as HMAC secrets, and empty HMAC keys accepted. That's 41 signature-verification advisories (CWE-347) across PyPI. -*No runnable example: demonstrating this means forging a token.* +[Example](../examples/59_key_algorithm_confusion.py) diff --git a/examples/14_bool_is_int.py b/examples/14_bool_is_int.py index 69c24e5..42d1929 100644 --- a/examples/14_bool_is_int.py +++ b/examples/14_bool_is_int.py @@ -10,8 +10,14 @@ assert Counter([1, True, 1.0]) == Counter({1: 3}) assert {0, False} == {0} assert True + True == 2 -assert isinstance(True, int) -# The fix: if the type matters, make it part of the key. +# Type checks trap too: JSON `true` passes `isinstance(..., int)`. +payload = {"limit": True} +assert isinstance(payload["limit"], int) + +# The fix: exclude bool explicitly when validating ints, and include the type in mixed keys. +is_real_int = isinstance(payload["limit"], int) and not isinstance(payload["limit"], bool) +assert not is_real_int + d = {(type(k), k): v for k, v in [(1, "one"), (True, "yes"), (1.0, "float one")]} assert len(d) == 3 diff --git a/examples/44_nan_breaks_containers.py b/examples/44_nan_breaks_containers.py index 43dffe1..80f6505 100644 --- a/examples/44_nan_breaks_containers.py +++ b/examples/44_nan_breaks_containers.py @@ -19,7 +19,13 @@ print(result) # [3, nan, 1, 2] assert [x for x in result if not math.isnan(x)] != [1, 2, 3] -# The fix: remove or replace NaNs before aggregating or sorting. +# Sets and dicts accept every distinct NaN object as a separate, un-findable key. +nan_set = {float("nan") for _ in range(5)} +print(len(nan_set), float("nan") in nan_set) # 5 False +assert len(nan_set) == 5 +assert float("nan") not in nan_set + +# The fix: remove or replace NaNs before aggregating, sorting or keying containers. clean = [x for x in [3, nan, 1, 2] if not math.isnan(x)] assert sorted(clean) == [1, 2, 3] assert max(clean) == 3 diff --git a/examples/58_pickle_deserialization_exec.py b/examples/58_pickle_deserialization_exec.py new file mode 100644 index 0000000..c418c5f --- /dev/null +++ b/examples/58_pickle_deserialization_exec.py @@ -0,0 +1,60 @@ +"""pickle.loads executes arbitrary callables encoded in the payload during deserialization.""" + +import hashlib +import hmac +import json +import pickle + +side_effects = [] + + +def record_call(arg): + side_effects.append(arg) + return {"loaded": arg} + + +class BenignPayload: + def __reduce__(self): + # Instructs the pickle VM to call record_call("ran-during-load") while loading. + return (record_call, ("ran-during-load",)) + + +# The trap: loading bytes runs code before the caller ever inspects the result. +raw = pickle.dumps(BenignPayload()) +result = pickle.loads(raw) +print(side_effects) # ['ran-during-load'] +assert side_effects == ["ran-during-load"] + +# The fix: use a data-only format such as JSON for untrusted input. +safe_bytes = json.dumps({"loaded": "data-only"}).encode() +assert json.loads(safe_bytes) == {"loaded": "data-only"} + + +# If you must load pickles you produced yourself, verify an HMAC before touching pickle.loads. +def signed_dumps(obj, secret): + body = pickle.dumps(obj) + sig = hmac.new(secret, body, hashlib.sha256).digest() + return sig + body + + +def signed_loads(blob, secret): + sig, body = blob[:32], blob[32:] + expected = hmac.new(secret, body, hashlib.sha256).digest() + if not hmac.compare_digest(sig, expected): + raise ValueError("untrusted pickle rejected before deserialization") + return pickle.loads(body) + + +secret = b"server-only-secret-key" +valid = signed_dumps({"ok": True}, secret) +assert signed_loads(valid, secret) == {"ok": True} + +tampered = b"\x00" * 32 + raw +side_effects.clear() +try: + signed_loads(tampered, secret) +except ValueError: + pass +else: + raise AssertionError("expected ValueError on untrusted payload") +assert side_effects == [] diff --git a/examples/59_key_algorithm_confusion.py b/examples/59_key_algorithm_confusion.py new file mode 100644 index 0000000..5dc38d1 --- /dev/null +++ b/examples/59_key_algorithm_confusion.py @@ -0,0 +1,62 @@ +"""Letting an untrusted token header choose the algorithm turns a public key into an HMAC secret.""" + +import base64 +import hashlib +import hmac +import json + +# Public verification material that anyone (including an attacker) can read. +SERVER_PUBLIC_KEY = b"-----BEGIN PUBLIC KEY-----\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8A\n-----END PUBLIC KEY-----" + + +def b64e(data): + return base64.urlsafe_b64encode(data).rstrip(b"=").decode() + + +def b64d(text): + return base64.urlsafe_b64decode(text + "=" * (-len(text) % 4)) + + +# The trap: one `key` parameter whose meaning is chosen by the token's own `alg` header. +def decode_insecure(token, key): + header_b64, payload_b64, sig_b64 = token.split(".") + header = json.loads(b64d(header_b64)) + signing_input = f"{header_b64}.{payload_b64}".encode() + + if header["alg"] == "HS256": + expected = hmac.new(key, signing_input, hashlib.sha256).digest() + if not hmac.compare_digest(b64d(sig_b64), expected): + raise ValueError("bad signature") + elif header["alg"] == "RS256": + raise NotImplementedError("asymmetric check omitted in demo") + else: + raise ValueError("unknown alg") + return json.loads(b64d(payload_b64)) + + +# An attacker labels the token HS256 and signs it using the server's public key bytes. +header = b64e(json.dumps({"alg": "HS256"}).encode()) +payload = b64e(json.dumps({"sub": "admin"}).encode()) +forged_sig = b64e(hmac.new(SERVER_PUBLIC_KEY, f"{header}.{payload}".encode(), hashlib.sha256).digest()) +forged_token = f"{header}.{payload}.{forged_sig}" + +claims = decode_insecure(forged_token, SERVER_PUBLIC_KEY) +print(claims) # {'sub': 'admin'} (accepted!) +assert claims == {"sub": "admin"} + + +# The fix: the server pins allowed algorithms explicitly and never trusts the header alone. +def decode_fixed(token, key, *, algorithms): + header_b64, _, _ = token.split(".") + header = json.loads(b64d(header_b64)) + if header.get("alg") not in algorithms: + raise ValueError(f"disallowed algorithm: {header.get('alg')!r}") + return decode_insecure(token, key) + + +try: + decode_fixed(forged_token, SERVER_PUBLIC_KEY, algorithms=["RS256"]) +except ValueError as e: + print(e) # disallowed algorithm: 'HS256' +else: + raise AssertionError("expected forged HS256 token to be rejected") diff --git a/examples/60_all_empty_vacuous_truth.py b/examples/60_all_empty_vacuous_truth.py new file mode 100644 index 0000000..6d69e86 --- /dev/null +++ b/examples/60_all_empty_vacuous_truth.py @@ -0,0 +1,23 @@ +"""all([]) is True (vacuous truth), so empty inputs and exhausted iterators pass every check.""" + +# The trap: "every user is an admin" is True when there are zero users. +users = [] +can_deploy = all(u["is_admin"] for u in users) +print(can_deploy) # True +assert can_deploy is True + +# Worse with iterators (#1): logging or counting first makes a failing check pass. +candidates = (u for u in [{"name": "guest", "is_admin": False}]) +_ = list(candidates) # Consumed earlier (e.g. for logging). +assert all(u["is_admin"] for u in candidates) is True + + +# The fix: materialize once and require at least one item when emptiness isn't valid. +def all_nonempty(items, predicate): + items = list(items) + return bool(items) and all(predicate(x) for x in items) + + +assert all_nonempty([], lambda u: u["is_admin"]) is False +assert all_nonempty([{"is_admin": False}], lambda u: u["is_admin"]) is False +assert all_nonempty([{"is_admin": True}], lambda u: u["is_admin"]) is True diff --git a/examples/61_groupby_unsorted_and_shared.py b/examples/61_groupby_unsorted_and_shared.py new file mode 100644 index 0000000..a6adb02 --- /dev/null +++ b/examples/61_groupby_unsorted_and_shared.py @@ -0,0 +1,28 @@ +"""itertools.groupby only groups consecutive runs, and advancing it drains prior groups.""" + +from collections import defaultdict +from itertools import groupby + +# Trap 1: unsorted data produces duplicate group keys, unlike SQL GROUP BY. +events = [("alice", 10), ("bob", 20), ("alice", 5)] +keys = [user for user, _ in groupby(events, key=lambda r: r[0])] +print(keys) # ['alice', 'bob', 'alice'] +assert keys == ["alice", "bob", "alice"] + +# Trap 2: storing (key, group_iter) pairs leaves earlier group iterators empty. +sorted_events = sorted(events, key=lambda r: r[0]) +saved = list(groupby(sorted_events, key=lambda r: r[0])) +first_key, first_group = saved[0] +print(first_key, list(first_group)) # alice [] (drained when groupby moved to 'bob') +assert first_key == "alice" +assert list(first_group) == [] + +# The fix: sort by the same key first and materialize each group immediately... +grouped = [(k, list(g)) for k, g in groupby(sorted(events, key=lambda r: r[0]), key=lambda r: r[0])] +assert grouped == [("alice", [("alice", 10), ("alice", 5)]), ("bob", [("bob", 20)])] + +# ...or group in O(n) without sorting using defaultdict(list). +by_user = defaultdict(list) +for user, amount in events: + by_user[user].append(amount) +assert dict(by_user) == {"alice": [10, 5], "bob": [20]} diff --git a/examples/62_chained_comparison_trap.py b/examples/62_chained_comparison_trap.py new file mode 100644 index 0000000..118a2d9 --- /dev/null +++ b/examples/62_chained_comparison_trap.py @@ -0,0 +1,20 @@ +"""Python chains every comparison operator, including `in` and `is`, not just `<` and `>`.""" + +# The trap: `"a" in d == True` expands to `("a" in d) and (d == True)`. +d = {"a": 1} +result = "a" in d == True # noqa: E712 +print(result) # False +assert result is False + +# Conversely, `False == False in [False]` expands to `(False == False) and (False in [False])`. +weird = False == False in [False] # noqa: E712 +print(weird) # True (even though `(False == False) in [False]` is False) +assert weird is True +assert ((False == False) in [False]) is False + +# Even `<` chains into `==`: `1 < 2 == True` is `(1 < 2) and (2 == True)`. +assert (1 < 2 == True) is False # noqa: E712 + +# The fix: don't compare booleans to True/False; or add explicit parentheses when needed. +assert ("a" in d) is True +assert "a" in d diff --git a/examples/63_match_case_bare_name.py b/examples/63_match_case_bare_name.py new file mode 100644 index 0000000..76089e3 --- /dev/null +++ b/examples/63_match_case_bare_name.py @@ -0,0 +1,39 @@ +"""Bare names inside `match`/`case` are capture variables, not constant lookups.""" + +from enum import IntEnum + +# The trap: HTTP_OK looks like a constant comparison, but a bare name matches +# anything and rebinds the local variable HTTP_OK to the subject's value. +HTTP_OK = 200 + + +def classify_broken(response): + HTTP_OK = 200 + match response: + case (HTTP_OK, body): # Captures status into HTTP_OK! + return f"ok ({HTTP_OK}): {body}" + case _: + return "error" + + +out = classify_broken((500, "database down")) +print(out) # ok (500): database down +assert out == "ok (500): database down" + + +# The fix: dotted names (such as Enum members) are value patterns, not captures. +class Status(IntEnum): + OK = 200 + SERVER_ERROR = 500 + + +def classify_fixed(response): + match response: + case (Status.OK, body): + return f"ok: {body}" + case _: + return "error" + + +assert classify_fixed((500, "database down")) == "error" +assert classify_fixed((200, "healthy")) == "ok: healthy" diff --git a/examples/64_async_generator_cleanup.py b/examples/64_async_generator_cleanup.py new file mode 100644 index 0000000..75061df --- /dev/null +++ b/examples/64_async_generator_cleanup.py @@ -0,0 +1,51 @@ +"""Breaking out of `async for` leaves an async generator's `finally` suspended.""" + +import asyncio +from contextlib import aclosing + + +def sync_records(state): + state["open"] = True + try: + yield 1 + yield 2 + finally: + state["open"] = False + + +async def async_records(state): + state["open"] = True + try: + yield 1 + yield 2 + finally: + await asyncio.sleep(0) + state["open"] = False + + +async def main(): + # In CPython, breaking out of a sync generator closes it immediately when refcount hits 0. + sync_state = {"open": False} + for _ in sync_records(sync_state): + break + assert sync_state["open"] is False + + # The trap: breaking out of `async for` cannot `await` cleanup synchronously, + # so resources stay open until GC or loop shutdown. + async_state = {"open": False} + agen = async_records(async_state) + async for _ in agen: + break + print(f"still open after break: {async_state['open']}") # True + assert async_state["open"] is True + await agen.aclose() # Clean up explicitly for this demo. + + # The fix: wrap the async generator in contextlib.aclosing(). + fixed_state = {"open": False} + async with aclosing(async_records(fixed_state)) as agen: + async for _ in agen: + break + assert fixed_state["open"] is False + + +asyncio.run(main()) diff --git a/examples/65_asyncio_gather_orphans.py b/examples/65_asyncio_gather_orphans.py new file mode 100644 index 0000000..58f0613 --- /dev/null +++ b/examples/65_asyncio_gather_orphans.py @@ -0,0 +1,55 @@ +"""asyncio.gather() propagates the first error without cancelling sibling tasks.""" + +import asyncio +import sys + + +async def validate(): + await asyncio.sleep(0.01) + raise ValueError("invalid order") + + +async def charge_card(ledger): + await asyncio.sleep(0.03) + ledger.append("charged") + + +async def main(): + # The trap: validate() fails, gather() raises, and charge_card() keeps running! + ledger = [] + t1 = asyncio.create_task(validate()) + t2 = asyncio.create_task(charge_card(ledger)) + try: + await asyncio.gather(t1, t2) + except ValueError: + pass + + assert ledger == [] # Nothing charged yet when the exception handler runs... + await asyncio.sleep(0.04) + print(ledger) # ['charged'] (...the sibling finished in the background!) + assert ledger == ["charged"] + await t2 + + # The fix (Python 3.11+): asyncio.TaskGroup cancels siblings before exiting `async with`. + ledger_fixed = [] + if sys.version_info >= (3, 11): + try: + async with asyncio.TaskGroup() as tg: + tg.create_task(validate()) + tg.create_task(charge_card(ledger_fixed)) + except Exception: # Catches ExceptionGroup(ValueError) while staying 3.10-parseable. + pass + else: + tasks = [asyncio.create_task(validate()), asyncio.create_task(charge_card(ledger_fixed))] + try: + await asyncio.gather(*tasks) + except ValueError: + for t in tasks: + t.cancel() + await asyncio.gather(*tasks, return_exceptions=True) + + await asyncio.sleep(0.04) + assert ledger_fixed == [] + + +asyncio.run(main()) diff --git a/examples/66_subprocess_pipe_deadlock.py b/examples/66_subprocess_pipe_deadlock.py new file mode 100644 index 0000000..65c26e5 --- /dev/null +++ b/examples/66_subprocess_pipe_deadlock.py @@ -0,0 +1,26 @@ +"""Popen.wait() with stdout=PIPE deadlocks once the child fills the OS pipe buffer.""" + +import subprocess +import sys + +# A child that writes 1 MB to stdout (~16x a typical 64 KB OS pipe buffer). +cmd = [sys.executable, "-c", "import sys; sys.stdout.write('x' * 1_000_000)"] + +# The trap: wait() waits for exit without reading stdout, while the child blocks on write(). +proc = subprocess.Popen(cmd, stdout=subprocess.PIPE) +try: + proc.wait(timeout=0.3) +except subprocess.TimeoutExpired: + deadlocked = True + proc.kill() + proc.communicate() # Drain and close pipe cleanly. +else: + deadlocked = False + proc.stdout.close() + +print(f"wait() deadlocked: {deadlocked}") # True +assert deadlocked is True + +# The fix: subprocess.run() (or proc.communicate()) drains pipes while waiting. +completed = subprocess.run(cmd, capture_output=True, text=True, check=True, timeout=5) +assert len(completed.stdout) == 1_000_000 diff --git a/examples/67_freethreaded_gil_assumptions.py b/examples/67_freethreaded_gil_assumptions.py new file mode 100644 index 0000000..217d52c --- /dev/null +++ b/examples/67_freethreaded_gil_assumptions.py @@ -0,0 +1,77 @@ +""""The GIL makes dict thread-safe" fails on compound operations and dict iteration.""" + +import threading +import time + +# The trap: individual C-level dict lookups are atomic under the GIL, so code often skips +# locks around shared dicts. Both check-then-set and concurrent iteration break anyway—and +# under free-threaded Python (3.13t/3.14t) there is no GIL to hide timing windows at all. +cache = {} +calls = 0 + + +def expensive(key): + global calls + time.sleep(0.01) # Simulates I/O or CPU work between check and store. + calls += 1 + return f"value-{key}" + + +def get_unlocked(key): + if key not in cache: + cache[key] = expensive(key) + return cache[key] + + +threads = [threading.Thread(target=get_unlocked, args=("user:1",)) for _ in range(5)] +for t in threads: + t.start() +for t in threads: + t.join() + +print(f"expensive() calls for one key: {calls}") # 5 (computed 5 times instead of 1) +assert calls > 1 + +# Concurrent mutation during iteration raises RuntimeError even with the GIL enabled. +shared = {i: i for i in range(200)} +iteration_error = None +ready = threading.Event() + + +def iterate_dict(): + global iteration_error + try: + for i, _ in enumerate(shared): + if i == 10: + ready.set() + time.sleep(0.0005) + except RuntimeError as e: + iteration_error = e + + +t = threading.Thread(target=iterate_dict) +t.start() +ready.wait() +shared["new_key"] = 999 +t.join() +assert isinstance(iteration_error, RuntimeError) + +# The fix: guard check-then-act and snapshots with a Lock. +cache.clear() +calls = 0 +lock = threading.Lock() + + +def get_locked(key): + with lock: + if key not in cache: + cache[key] = expensive(key) + return cache[key] + + +threads = [threading.Thread(target=get_locked, args=("user:1",)) for _ in range(5)] +for t in threads: + t.start() +for t in threads: + t.join() +assert calls == 1 diff --git a/examples/68_builtin_subclass_bypass.py b/examples/68_builtin_subclass_bypass.py new file mode 100644 index 0000000..d423d7a --- /dev/null +++ b/examples/68_builtin_subclass_bypass.py @@ -0,0 +1,51 @@ +"""Subclassing built-in dict/list skips overridden dunder methods in C fast-paths.""" + +from collections import UserDict + + +# The trap: CPython's dict.__init__, dict.update, dict.get, dict.setdefault and {**d} call +# internal C functions directly instead of dispatching through Python's dunder methods. +class LowerKeyDict(dict): + def __setitem__(self, key, value): + super().__setitem__(key.lower(), value) + + def __getitem__(self, key): + return super().__getitem__(key.lower()) + + def __contains__(self, key): + return super().__contains__(key.lower()) + + +d = LowerKeyDict(Accept="text/html") # __init__ skips __setitem__ +d["X-Token"] = "secret" # [] calls __setitem__ +d.update({"Cache-Control": "no-store"}) # .update() skips __setitem__ +d.setdefault("Host", "example.com") # .setdefault() skips __setitem__ + +print(list(d.keys())) # ['Accept', 'x-token', 'Cache-Control', 'Host'] +assert "Accept" not in d # __contains__ lowercases, so 'Accept' isn't even found in its own dict! +assert d["X-TOKEN"] == "secret" # [] calls __getitem__ +assert d.get("X-TOKEN") is None # .get() skips both __contains__ and __getitem__! + + +# The fix: subclass collections.UserDict (or collections.abc.MutableMapping). +class FixedLowerKeyDict(UserDict): + def __setitem__(self, key, value): + super().__setitem__(key.lower(), value) + + def __getitem__(self, key): + return super().__getitem__(key.lower()) + + def __delitem__(self, key): + super().__delitem__(key.lower()) + + def __contains__(self, key): + return super().__contains__(key.lower()) + + +fixed = FixedLowerKeyDict(Accept="text/html") +fixed["X-Token"] = "secret" +fixed.update({"Cache-Control": "no-store"}) +fixed.setdefault("Host", "example.com") +assert list(fixed.keys()) == ["accept", "x-token", "cache-control", "host"] +assert fixed.get("X-TOKEN") == "secret" +assert "ACCEPT" in fixed diff --git a/examples/69_slots_inheritance_dict.py b/examples/69_slots_inheritance_dict.py new file mode 100644 index 0000000..aa369ec --- /dev/null +++ b/examples/69_slots_inheritance_dict.py @@ -0,0 +1,44 @@ +"""Subclassing a __slots__ class without declaring __slots__ silently restores __dict__.""" + + +class Point: + __slots__ = ("x", "y") + + def __init__(self, x, y): + self.x, self.y = x, y + + +# The parent has no __dict__ and rejects unknown attributes. +p = Point(1, 2) +assert not hasattr(p, "__dict__") + + +# The trap: omitting __slots__ on a subclass recreates __dict__ and __weakref__ on instances. +class LabeledPoint(Point): + pass + + +lp = LabeledPoint(1, 2) +lp.typo_attr = 99 # Silently stored in lp.__dict__! +print(hasattr(lp, "__dict__"), lp.__dict__) # True {'typo_attr': 99} +assert hasattr(lp, "__dict__") +assert lp.__dict__ == {"typo_attr": 99} + + +# The fix: declare __slots__ on every subclass (use `()` if it adds no new attributes). +class SlottedLabeledPoint(Point): + __slots__ = ("label",) + + def __init__(self, x, y, label): + super().__init__(x, y) + self.label = label + + +sp = SlottedLabeledPoint(1, 2, "origin") +assert not hasattr(sp, "__dict__") +try: + sp.typo_attr = 99 +except AttributeError: + pass +else: + raise AssertionError("expected AttributeError on slotted subclass") diff --git a/examples/70_bool_falls_back_to_len.py b/examples/70_bool_falls_back_to_len.py new file mode 100644 index 0000000..a45f95a --- /dev/null +++ b/examples/70_bool_falls_back_to_len.py @@ -0,0 +1,40 @@ +"""Defining __len__ without __bool__ makes valid zero-length objects falsy.""" + +import warnings +import xml.etree.ElementTree as ET + + +# The trap: `if node:` calls __bool__, which falls back to `len(node) != 0`. +class Node: + def __init__(self, name, children=()): + self.name = name + self.children = list(children) + + def __len__(self): + return len(self.children) + + +leaf = Node("root") # A valid node that happens to have 0 children. +found = leaf if leaf else None +print(found) # None (treated as missing!) +assert bool(leaf) is False +assert found is None + +# Standard library example: an XML Element with no child elements is falsy too. +with warnings.catch_warnings(): + warnings.simplefilter("ignore", DeprecationWarning) + elem = ET.fromstring("ok") + assert bool(elem) is False + + +# The fix: test `is not None` for optional objects, and define `__bool__` when +# an empty container-like object is still a valid, present entity. +class SafeNode(Node): + def __bool__(self): + return True + + +safe_leaf = SafeNode("root") +assert bool(safe_leaf) is True +assert len(safe_leaf) == 0 +assert (leaf if leaf is not None else None) is leaf diff --git a/examples/71_stringified_annotations.py b/examples/71_stringified_annotations.py new file mode 100644 index 0000000..31da885 --- /dev/null +++ b/examples/71_stringified_annotations.py @@ -0,0 +1,49 @@ +"""`from __future__ import annotations` turns __annotations__ values into strings at runtime.""" + +from __future__ import annotations + +import typing + + +def parse_port(port: int) -> int: + return port + + +# The trap: inspecting __annotations__ directly sees 'int' (a str), not the int class. +ann = parse_port.__annotations__["port"] +print(type(ann), repr(ann)) # 'int' +assert isinstance(ann, str) + +try: + isinstance(8080, parse_port.__annotations__["port"]) +except TypeError as e: + print(e) # isinstance() arg 2 must be a type, a tuple of types, or a union +else: + raise AssertionError("expected TypeError when passing string annotation to isinstance") + +# Local classes have a second trap: get_type_hints() looks up names in module globals +# unless given localns, so annotations referencing function-local types fail outside the scope. +def make_handler(): + class LocalRequest: + pass + + def handle(req: LocalRequest) -> None: + pass + + return handle, LocalRequest + + +handler, local_cls = make_handler() +try: + typing.get_type_hints(handler) +except NameError: + pass +else: + raise AssertionError("expected NameError for unresolvable local class") + +# The fix: use typing.get_type_hints() instead of raw __annotations__, and keep +# runtime-inspected types at module scope (or pass localns explicitly). +hints = typing.get_type_hints(parse_port) +assert hints["port"] is int +assert isinstance(8080, hints["port"]) +assert typing.get_type_hints(handler, localns={"LocalRequest": local_cls})["req"] is local_cls diff --git a/examples/72_urljoin_drops_path.py b/examples/72_urljoin_drops_path.py new file mode 100644 index 0000000..9238e89 --- /dev/null +++ b/examples/72_urljoin_drops_path.py @@ -0,0 +1,40 @@ +"""urllib.parse.urljoin follows browser link rules, not path-append rules.""" + +from urllib.parse import urljoin, urlsplit + +# Trap 1: without a trailing slash, the last segment of base is treated as a file and dropped. +base = "https://api.example.com/v1" +print(urljoin(base, "users")) # https://api.example.com/users (/v1 vanished!) +assert urljoin(base, "users") == "https://api.example.com/users" + +# Trap 2: a leading slash in the target discards the entire base path, even with a trailing slash. +assert urljoin("https://api.example.com/v1/", "/users") == "https://api.example.com/users" + +# Trap 3: a scheme-relative target ("//...") replaces the host altogether. +escaped = urljoin("https://api.example.com/v1/", "//attacker.example/steal") +print(escaped) # https://attacker.example/steal +assert escaped == "https://attacker.example/steal" + + +# The fix: reject targets that carry a scheme or host, normalize one slash, and check containment. +def safe_api_url(base_url, endpoint): + target = urlsplit(endpoint) + if target.scheme or target.netloc: + raise ValueError(f"{endpoint!r} specifies its own origin") + base_norm = base_url.rstrip("/") + "/" + joined = urljoin(base_norm, endpoint.removeprefix("/")) + b, j = urlsplit(base_norm), urlsplit(joined) + if (b.scheme, b.netloc) != (j.scheme, j.netloc) or not j.path.startswith(b.path): + raise ValueError(f"{endpoint!r} escapes {base_norm}") + return joined + + +assert safe_api_url("https://api.example.com/v1", "users") == "https://api.example.com/v1/users" +assert safe_api_url("https://api.example.com/v1/", "/users") == "https://api.example.com/v1/users" +for bad in ["//attacker.example/steal", "https://attacker.example/x", "../admin"]: + try: + safe_api_url("https://api.example.com/v1", bad) + except ValueError: + pass + else: + raise AssertionError(f"expected ValueError for {bad!r}") diff --git a/examples/73_unicode_normalization_casing.py b/examples/73_unicode_normalization_casing.py new file mode 100644 index 0000000..fdd3b90 --- /dev/null +++ b/examples/73_unicode_normalization_casing.py @@ -0,0 +1,28 @@ +"""Visually identical Unicode strings can differ in bytes, length, equality and hash.""" + +import unicodedata + +# The trap: precomposed (NFC) vs decomposed (NFD, common in macOS filenames). +nfc = "caf\u00e9" # 'café' as 4 code points +nfd = "cafe\u0301" # 'café' as 5 code points ('e' + combining acute accent) + +print(nfc, nfd, nfc == nfd, len(nfc), len(nfd)) # café café False 4 5 +assert nfc != nfd +assert len(nfc) == 4 and len(nfd) == 5 +assert len({nfc, nfd}) == 2 # Two separate keys for the "same" string! + +# Case-insensitive Trap: .lower() doesn't handle multi-character mappings like German 'ß'. +assert "ß".lower() == "ß" +assert "ß".upper() == "SS" +assert "straße".lower() != "STRASSE".lower() + +# The fix: normalize external strings (typically NFC) and use .casefold() for caseless matching. +assert unicodedata.normalize("NFC", nfc) == unicodedata.normalize("NFC", nfd) + + +def canonical_key(text): + return unicodedata.normalize("NFC", text).casefold() + + +assert canonical_key(nfc) == canonical_key(nfd) +assert canonical_key("straße") == canonical_key("STRASSE") From 8a0f9aa9c92ea49039bc3b415b744db4acbdc41b Mon Sep 17 00:00:00 2001 From: Ehsan Rahman Date: Mon, 2 Nov 2026 16:21:04 +0000 Subject: [PATCH 2/2] Address review: resolve annotation strings and synchronize asyncio example --- docs/interpreter.md | 4 +- examples/65_asyncio_gather_orphans.py | 53 ++++++++++++++++++-------- examples/71_stringified_annotations.py | 12 ++++++ 3 files changed, 52 insertions(+), 17 deletions(-) diff --git a/docs/interpreter.md b/docs/interpreter.md index 0969d6e..3b6a0dd 100644 --- a/docs/interpreter.md +++ b/docs/interpreter.md @@ -132,6 +132,8 @@ isinstance(8080, parse_port.__annotations__["port"]) # TypeError! **Why:** Static type checkers wanted forward references without runtime import cost, while runtime frameworks (FastAPI, Pydantic, `dataclasses`, dependency injectors) treat annotations as executable metadata. Python 3.14 introduced lazy annotation functions via `annotationlib` ([PEP 649](https://peps.python.org/pep-0649/)) to replace stringification when `__future__` is *not* imported, meaning libraries now encounter eager types, PEP 563 strings, and PEP 649 evaluators side by side. -**The fix:** Never read `__annotations__` directly for types; call `typing.get_type_hints(fn)` (or `annotationlib.get_annotations()` on 3.14+), and keep runtime-inspected types at module scope where `get_type_hints()` can resolve them (Ruff's `TC001`–`TC003` flag imports moved behind `if TYPE_CHECKING:` that runtime consumers still need). +**The fix:** Never read `__annotations__` directly for types; call `typing.get_type_hints(fn)` (or `annotationlib.get_annotations(fn, eval_str=True)` on 3.14+), and keep runtime-inspected types at module scope where `get_type_hints()` can resolve them (Ruff's `TC001`–`TC003` flag imports moved behind `if TYPE_CHECKING:` that runtime consumers still need). + +With the future import above, `annotationlib.get_annotations(fn)` leaves strings unchanged by default (`eval_str=False`). Pass `eval_str=True` to resolve `"int"` to `int` ([API reference](https://docs.python.org/3.14/library/annotationlib.html#annotationlib.get_annotations)). [Example](../examples/71_stringified_annotations.py) diff --git a/examples/65_asyncio_gather_orphans.py b/examples/65_asyncio_gather_orphans.py index 58f0613..e6ab1c7 100644 --- a/examples/65_asyncio_gather_orphans.py +++ b/examples/65_asyncio_gather_orphans.py @@ -4,51 +4,72 @@ import sys -async def validate(): - await asyncio.sleep(0.01) +async def validate(charging_started): + await charging_started.wait() raise ValueError("invalid order") -async def charge_card(ledger): - await asyncio.sleep(0.03) +async def charge_card(ledger, charging_started, release_charge): + charging_started.set() + # Hold the side effect until the caller has observed the validation failure. + # Events make the ordering independent of event-loop speed or timer delays. + await release_charge.wait() ledger.append("charged") async def main(): # The trap: validate() fails, gather() raises, and charge_card() keeps running! ledger = [] - t1 = asyncio.create_task(validate()) - t2 = asyncio.create_task(charge_card(ledger)) + charging_started = asyncio.Event() + release_charge = asyncio.Event() + t1 = asyncio.create_task(validate(charging_started)) + t2 = asyncio.create_task(charge_card(ledger, charging_started, release_charge)) try: await asyncio.gather(t1, t2) except ValueError: pass + else: + raise AssertionError("expected validation failure") - assert ledger == [] # Nothing charged yet when the exception handler runs... - await asyncio.sleep(0.04) - print(ledger) # ['charged'] (...the sibling finished in the background!) - assert ledger == ["charged"] + assert ledger == [] + assert not t2.done() # gather() left its sibling alive after propagating the error. + release_charge.set() await t2 + print(ledger) # ['charged'] (the sibling finished after the error was handled!) + assert ledger == ["charged"] # The fix (Python 3.11+): asyncio.TaskGroup cancels siblings before exiting `async with`. ledger_fixed = [] + charging_started = asyncio.Event() + release_charge = asyncio.Event() if sys.version_info >= (3, 11): try: async with asyncio.TaskGroup() as tg: - tg.create_task(validate()) - tg.create_task(charge_card(ledger_fixed)) - except Exception: # Catches ExceptionGroup(ValueError) while staying 3.10-parseable. - pass + tg.create_task(validate(charging_started)) + charge = tg.create_task(charge_card(ledger_fixed, charging_started, release_charge)) + except Exception as exc: # ExceptionGroup, with syntax that also parses on 3.10. + assert len(exc.exceptions) == 1 + assert isinstance(exc.exceptions[0], ValueError) + else: + raise AssertionError("expected validation failure") else: - tasks = [asyncio.create_task(validate()), asyncio.create_task(charge_card(ledger_fixed))] + tasks = [ + asyncio.create_task(validate(charging_started)), + asyncio.create_task(charge_card(ledger_fixed, charging_started, release_charge)), + ] + charge = tasks[1] try: await asyncio.gather(*tasks) except ValueError: for t in tasks: t.cancel() await asyncio.gather(*tasks, return_exceptions=True) + else: + raise AssertionError("expected validation failure") - await asyncio.sleep(0.04) + assert charging_started.is_set() + assert charge.cancelled() # Cleanup finished before control returned to the caller. + release_charge.set() assert ledger_fixed == [] diff --git a/examples/71_stringified_annotations.py b/examples/71_stringified_annotations.py index 31da885..2f89dc1 100644 --- a/examples/71_stringified_annotations.py +++ b/examples/71_stringified_annotations.py @@ -47,3 +47,15 @@ def handle(req: LocalRequest) -> None: assert hints["port"] is int assert isinstance(8080, hints["port"]) assert typing.get_type_hints(handler, localns={"LocalRequest": local_cls})["req"] is local_cls + +# Python 3.14+ alternative: annotationlib leaves PEP 563 strings unchanged unless +# eval_str=True is passed. Check both the default and the resolved annotation. +try: + import annotationlib +except ImportError: # Python 3.10–3.13 + pass +else: + assert annotationlib.get_annotations(parse_port)["port"] == "int" + resolved = annotationlib.get_annotations(parse_port, eval_str=True) + assert resolved["port"] is int + assert isinstance(8080, resolved["port"])