Handling whitespace in `wordwrap` after CVE fix

Django Internals
1

Is there anyone with strong opinions about wordwrap? I’d love some feedback! :mega:

The wordwrap template filter relies on the private, undocumented wrap() function. After a fix for CVE-2025-26699, a user reported changes when wrapping text that includes paragraphs with only line breaks (see release blocker ticket). While reviewing the PR with the fix, I also noticed regressions with paragraphs made only of spaces.

The fix replaced a custom wrap() implementation with textwrap.TextWrapper, which I like because it removes non-performant, home-grown logic in favor of stdlib code. But it does change how lines with only whitespace are handled, especially when they exceed the given width. For an example, see my comment in the PR.

I’m mainly trying to avoid adding new custom logic around textwrap.TextWrapper. The closer we stick to it, the fewer bugs and surprises we’ll have. Even if that slightly changes how whitespace-only lines are handled, I think it may be worth it, especially since the common real-life use case is probably chaining wordwrap with linebreakbr, where spaces don’t matter but newlines do.

Given the minimal wordwrap docs and confusing wrap() docstring, I think leaning on textwrap.TextWrapper’s behavior gives us a simpler, more predictable path forward. Concretely, for lines containing only spaces, I think we should either:

  • drop all spaces, but keep as many newlines as originally given (original proposal from the PR), or
  • keep lines intact, even if that means that some lines will be longer than width (current status of PR at the time of this writing).
2

I’m struggling to see that the exact wrappping behaviour was (at all) guaranteed by the stability policy (even if this change weren’t a security fix). :thinking:

It strikes me as similar to when the generated SQL coming out of the ORM has small changes. (I.e. not something that’s guaranteed)

1 Like
3

As a user, I’m not sure I like the idea of eating lines during wordwrap - it seems a surprising outcome. But I’d accept it fi there was no option.

But to me, there is an easy fix that shouldn’t generate additional memory problems, although it does add a small amount of extra logic around text wrap.TextWrapper—but only in the sense that it oddly returns an empty list or an empty line, which is not what we want.

I’d suggest changing the for-loop to be something like:

    empty = ''
    result = []
    for line in text.splitlines(True):
        newlines = wrapper.wrap(line)
        if len(newlines) == 0:
           result.append(empty)
       else:
           result.extend(newlines)

I’ve created the empty variable so the empty string is always a single instance, reducing memory use in a long run of empty lines.

I suppose you could also use

     result.extend(newlines or [empty])

if you wanted to remove the if statement, but that strikes me as less explicit and more fraught in a security context.

This will have higher memory than the existing solution use if long runs of empty lines as each empty line extends the result list by a single element.

In terms of evaluating memory use against the original code, does a member of the security team know whether it was ‘c’ stack exhaustion caused by the generator pattern in the old code? I’m struggling to remember how generators are implemented in the interpreter - or see a smoking gun in the difference between the old and new code for consuming a lot of memory.

4

Your suggestion is essentially what I originally proposed in the PR I submitted. It’s a clean solution. The only drawback is that it doesn’t preserve whitespace—it keeps the line itself, but drops the whitespace within it.