cap oversized field width/precision in str.format inference

[nabhan06]

Type of Changes

	Type
✓	🐛 Bug fix

Description

_infer_str_format_call evaluates the template eagerly with format_template.format(*args), so a tiny analyzed line like "{:>2000000000}".format("x") makes astroid build the padded multi-gigabyte string during inference (I saw RSS jump to ~3 GB before it settles). A width or precision handed in through a nested {} field, e.g. "{:>{}}".format("x", 2000000000), does the same. This is the same eager-materialization problem that **, <<, and list/str multiply already guard against, just reached through the format mini-language.

The fix routes the call through a small string.Formatter subclass and checks the width/precision in format_field, which runs after nested fields are resolved, so it covers both literal and argument-supplied sizes. Oversized cases now infer as Uninferable and normal formatting is unchanged.

Refs #3107

[DanielNoord]

@Pierre-Sassoulas I guess an AI provider has figured out that cap oversized ... is a nice way to create PRs.

Do we really carre about this? There don't seem to be actual user reports and with this regex matching we're making this all very complex.

[nabhan06]

Fair on the regex, that was the ugly part. I dropped it and just scan the format spec for digit runs over the cap, since width and precision are the only multi-digit numbers a spec can contain. Same behaviour, a lot less to read now.

On whether it's worth it: this is the exact same eager-materialization issue as #3107 (str/bytes multiply and shift), just reached through the format mini-language. "{:>2000000000}".format("x") pushes RSS to a couple of GB during inference of otherwise untrusted source, same as "x" * 10 ** 10 did before that fix. No user report, agreed, I hit it while poking at the format path after #3107. If you'd rather not carry the extra surface for a case nobody's reported, I'm fine closing it, your call.

[Pierre-Sassoulas]

I guess users expect astroid/pylint to be slow and do not troubleshoot exactly why all the time. I know those are AI contributions, but it's reasonable to add safeguard like this imo.

[codspeed-hq]

Merging this PR will not alter performance

✅ 3 untouched benchmarks
⏩ 1 skipped benchmark¹

---

_{Comparing nabhan06:format-field-size-cap (bd6ada9) with main (fe29fb1)}

Footnotes

1. 1 benchmark was skipped, so the baseline result was used instead. If it was deleted from the codebase, click here and archive it to remove it from the performance reports. ↩

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.61%. Comparing base (fe29fb1) to head (bd6ada9).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3131   +/-   ##
=======================================
  Coverage   93.61%   93.61%           
=======================================
  Files          92       92           
  Lines       11381    11390    +9     
=======================================
+ Hits        10654    10663    +9     
  Misses        727      727           

Flag	Coverage Δ
linux	93.48% <100.00%> (+<0.01%)	⬆️
pypy	93.61% <100.00%> (+<0.01%)	⬆️
windows	93.59% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
astroid/brain/brain_builtin_inference.py	93.42% <100.00%> (+0.12%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[DanielNoord]

I guess users expect astroid/pylint to be slow and do not troubleshoot exactly why all the time. I know those are AI contributions, but it's reasonable to add safeguard like this imo.

But we're making astroid slower for very specific edge cases that users haven't even reported yet. Who writes code like this: "{:>2000000000}".format("x")?

[Pierre-Sassoulas]

Maybe we anticipate too much, maybe it's similar to #2228 (Who does f(*([0]*(2**32+1))) ?) that we did before llms, I don't know. I think we need some sane limits to not explode in memory when we parse random code. There might be a way to do it more elegantly and cheaply though.

[DanielNoord]

pylint-dev/pylint#8748 feels like somebody fuzzing the code.

I understand we don't want to crash, but if CPython would crach on the code itself it probably makes more sense to just kill the run (and indicate where we crashed) rather than just going with Uninferable. After all, we are a linter: if the code is so bad we can't even parse it we can just crash loudly imo

[nabhan06]

On the CPython angle: "{:>2000000000}".format("x") doesn't actually crash CPython, it just builds the ~2GB string and returns it fine. So there's nothing to crash loudly on here, the code is valid, it's only astroid that blows up because it materializes that string eagerly while analyzing a line that at runtime might sit behind a branch that never runs (or come from generated/untrusted input).

That's the same shape as "x" * 10 ** 10 in #3107 and the [0]*(2**32+1) splat in #2228, and in both of those astroid already went with Uninferable instead of crashing, so this just keeps that behavior consistent rather than inventing a new rule for the format path.

On cost: codspeed shows no change, and the check only looks at the digit runs inside a format spec, so normal formatting isn't touched. If you'd rather a simpler shape than the Formatter subclass I'm happy to try, though the subclass is what lets it catch the width passed as an argument ("{:>{}}".format("x", 2000000000)), which a plain template scan would miss. And as I said earlier, if you'd rather not carry it at all it's an easy close, no hard feelings.

[Pierre-Sassoulas]

I don't have a strong opinion because I don't know where to draw the line honestly. User complaints would have been a nice signal, but we do have fuzzing report based on generated never seen in the Real World ( ™️ ) code that is hardly distinguishable from actual user signal. And I'm not sure users are always watching htop while astroid runs. We probably need to evaluate impacts on standard case before optimizing for corner cases. Right now the benchmark is going to catch major regression (>5% slowdown). I guess complexity analysis at review time is the only thing protecting us from small regression included by adding handling code we don't need for corner case that never happen. And determining the difference between corner case and actual use case is about taste right now. Maybe we should be looking into open source code (number of occurence in the primer?). What do you think @jacobtylerwalls @cdce8p ?

[correctmost]

User complaints would have been a nice signal, but we do have fuzzing report based on generated never seen in the Real World ( ™️ ) code that is hardly distinguishable from actual user signal.

I'm not sure how many people actually do this, but it is possible to run Pylint on every save (or change!) with editors like VS Code. That means you can end up checking code that is in a weird intermediate state or has a (temporary) copy/paste error that needs to be undone. That kind of code is unlikely to show up in a primer run, and users may not be inclined to report an issue that is not easily reproducible from the command line.

I don't know if this particular case needs to be fixed, but I just wanted to highlight one scenario where the added robustness can help.

[nabhan06]

On where to draw the line: the cap sits at 1e8, which is orders of magnitude past any width or precision real code actually uses (a few hundred at most), so nothing in the primer would trip it. That means no false positives on legitimate formatting; the only behavior change is on specs that would balloon into hundreds of MB or more. So the cost of keeping it is basically nil even if the trigger is rare. That said, it's your call on whether it's worth the extra surface, and I'm good either way.

[Pierre-Sassoulas]

@nabhan06 I meant when to decide if a code construct that can explode the cost of inference should be considered too niche to warrant a special case that make the normal inference slower. Evaluating if it's going to explode always have a cost even if small. (i.e. x and y in 'if the inference for "{:>2000000000}".format("x") explode and is found in x percent of the code, we can accept astroid to be y % slower all the time')

[nabhan06]

Fair, let me put actual numbers on the x and y.

y first: the guard doesn't touch general inference at all. It lives entirely inside _infer_str_format_call, which only runs once astroid already has a constant template and constant args and was going to call .format() anyway. Every other inference path pays nothing. So the only real cost is on that format path, and it comes down to one thing: I now route those calls through string.Formatter().format() (pure Python) instead of the C str.format(). That is the slowdown actually worth measuring, and it is what the format benchmark exercises (codspeed flags no change).

x: effectively zero in real code. The cap is 1e8 and nothing legitimate comes near a width or precision that large.

If the pure-Python swap is the part you would rather not pay for the common case, I can keep the C str.format() on the normal path and just pre-scan the spec (resolving nested {} sizes against the args) to reject oversized fields before formatting. A bit more code, but the fast path stays byte-for-byte what it is today. Happy to push that version if you prefer it, or to close this out if it is not worth the surface.