Already implemented by #1497 — this issue exists to be its canonical issue, not to request separate work.
#1497 covers everything below and one vector I missed: performance_ms=float("nan") emits a bare NaN, which Python's lenient json.loads accepts but strict parsers reject — the same record loss, moved to the consumer. Its fix is also better factored (_is_json_safe_scalar, _describe_exception, allow_nan=False).
I filed this before seeing #1497. Rather than close it as a duplicate, I'm leaving it open because #1497's body says "Refs #1452 — does not close a new issue," and the Canonical issue and evidence gate requires exactly one Closes #<issue>. If that gate goes red, Closes #1498 is the one-line fix. If #1497 lands another way, close this as completed alongside it.
No separate PR should be opened for this.
Summary
#1491 merged and closed #1452. It fixes two of the three vectors. The third is still live on main (8517bf8).
The finding was identified and fixed on #1471 before that PR was closed as a competing implementation. Recording it so it isn't re-derived a seventh time — six PRs (#1471, #1472, #1477, #1488, #1494, #1497) independently attacked #1452, which is the exact waste #1452 itself was filed to stop.
Measured against merged main (8517bf8)
Three logger.info calls through a real StreamHandler, the middle one poisoned via extra={"request_id": ...}:
circular container : 3 of 3 records
raising __str__ : 3 of 3 records
forged __class__ + raise: 2 of 3 records <-- RECORD LOST
Mechanism
logging_config.py:169-176:
except Exception as exc: # noqa: BLE001 - never lose a record
safe: dict[str, Any] = {
...
if isinstance(value, (str, int, float, bool, type(None)))
}
safe["serialization_error"] = f"{type(exc).__name__}: {exc}"
return json.dumps(safe, ensure_ascii=True, default=str)isinstance consults value.__class__; json.dumps uses the real runtime type. An object can forge the first and keep the second:
class Forged:
@property
def __class__(self): return str
def __str__(self): raise RuntimeError("boom")
- First
json.dumps(..., default=str) calls str(o) → RuntimeError → caught.
isinstance(o, str) is True, so the filter retains it.- The fallback
json.dumps(..., default=str) calls str(o) again → RuntimeError propagates out of the except block.
logging swallows it via Handler.handleError. Record dropped.
The default=str on the fallback is what makes step 3 fatal: it re-enters the same raising __str__ the first dump already failed on. Without it the call would raise TypeError instead — still lost, just differently.
Secondary, same block: safe["serialization_error"] = f"{type(exc).__name__}: {exc}" is unguarded, so an exception whose own __str__ raises loses the record on that line instead. #1497 closes this too.
Severity
Very low. No live call site can trigger it — correlation_id comes from record.request_id and header values, all strings, and a forged __class__ would have to be introduced deliberately. This is a correctness-of-invariant issue, not an exploitable one: the comment above the block claims the fallback retains only native scalars, and isinstance does not enforce that.
It is the same defect class #1452 was about — a guard whose stated guarantee the code doesn't deliver — one level down.
Acceptance criteria
- A record poisoned with a forged-
__class__, raising-__str__ value is still emitted, with level authoritative.
- An exception whose own
__str__ raises does not cost the record.
- A regression test in
tests/unit/test_logging_config_crlf.py covers both and fails against the pre-fix implementation.
All three are satisfied by #1497 (29 passed; 5 of 6 new tests fail pre-fix).
Summary
#1491 merged and closed #1452. It fixes two of the three vectors. The third is still live on
main(8517bf8).The finding was identified and fixed on #1471 before that PR was closed as a competing implementation. Recording it so it isn't re-derived a seventh time — six PRs (#1471, #1472, #1477, #1488, #1494, #1497) independently attacked #1452, which is the exact waste #1452 itself was filed to stop.
Measured against merged
main(8517bf8)Three
logger.infocalls through a realStreamHandler, the middle one poisoned viaextra={"request_id": ...}:Mechanism
logging_config.py:169-176:isinstanceconsultsvalue.__class__;json.dumpsuses the real runtime type. An object can forge the first and keep the second:json.dumps(..., default=str)callsstr(o)→RuntimeError→ caught.isinstance(o, str)is True, so the filter retains it.json.dumps(..., default=str)callsstr(o)again →RuntimeErrorpropagates out of theexceptblock.loggingswallows it viaHandler.handleError. Record dropped.The
default=stron the fallback is what makes step 3 fatal: it re-enters the same raising__str__the first dump already failed on. Without it the call would raiseTypeErrorinstead — still lost, just differently.Secondary, same block:
safe["serialization_error"] = f"{type(exc).__name__}: {exc}"is unguarded, so an exception whose own__str__raises loses the record on that line instead. #1497 closes this too.Severity
Very low. No live call site can trigger it —
correlation_idcomes fromrecord.request_idand header values, all strings, and a forged__class__would have to be introduced deliberately. This is a correctness-of-invariant issue, not an exploitable one: the comment above the block claims the fallback retains only native scalars, andisinstancedoes not enforce that.It is the same defect class #1452 was about — a guard whose stated guarantee the code doesn't deliver — one level down.
Acceptance criteria
__class__, raising-__str__value is still emitted, withlevelauthoritative.__str__raises does not cost the record.tests/unit/test_logging_config_crlf.pycovers both and fails against the pre-fix implementation.All three are satisfied by #1497 (29 passed; 5 of 6 new tests fail pre-fix).