Skip to content

Infinite loop in fill_textbox #4400

New issue

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

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

Already on GitHub? Sign in to your account

Closed
jotaf98 opened this issue Mar 23, 2025 · 5 comments
Closed

Infinite loop in fill_textbox #4400

jotaf98 opened this issue Mar 23, 2025 · 5 comments

Comments

@jotaf98
Copy link

jotaf98 commented Mar 23, 2025

Description of the bug

Hi, first of all, thanks for a great library!

I found an infinite loop condition when calling Writer.fill_textbox due to a bug in utils.py, where the correct counter for loop exit is not used.

The fix is simple, lines 4619-4620 in this file should read

            if n == 0:
                break

instead of if len(words) == 0.

How to reproduce the bug

Minimal script to reproduce:

import fitz

page = fitz.Document().new_page()
writer = fitz.TextWriter(page.rect)

text = '111111111'

writer.fill_textbox(rect=fitz.Rect(0, 0, 100, 20), pos=(80, 0), text=text, fontsize=8)

# this line will never be reached, as the above one enters an infinite loop

PyMuPDF version

1.25.4

Operating system

Windows

Python version

3.13

@julian-smith-artifex-com
Copy link
Collaborator

Thanks for the report and reproducer.

I have reproduced the problem in my tree, will investigate some more now and report back here later.

@julian-smith-artifex-com
Copy link
Collaborator

I've just pushed a fix onto our main branch, so this will be fixed in our next release.

@jotaf98
Copy link
Author

jotaf98 commented Mar 25, 2025

Glad to have helped, and thanks for the fix!

I'm not 100% sure about the change, compared to just changing the break condition to use n instead of len(words).

Maybe I'm not seeing it, but how do you guarantee that the loop terminates?

Or put another way, if n goes to 0, would it be better to raise an error (your assert) or break?

@julian-smith-artifex-com
Copy link
Collaborator

I don't think changing the break condition to use n would have fixed the problem properly - i'm not 100% sure, but it's possible that it would have silently dropped some text when given text as in your reproducer.

The problem was this chain of events:

  1. When we are on the first line (which can be less wide than the others because it starts at pos.x, not rect.tl.x), we used the wrong width when calling norm_words().
  2. So the individual words in words[] could be longer than the actual available width.
  3. This broke the assumption of the following loop (the one that used to go infinite) that it would always succeed when n=1.

The new assert n line is a genuine assert - it's never expected to fail, but serves to verify the assumptions that the code is written with. So it's a little bit of documentation about the intent of the code, plus it'll help if a future change reintroduce the original problem.

[I should probably have put some or all of this explanation into the commit message.]

I hope that makes sense.

@julian-smith-artifex-com
Copy link
Collaborator

Fixed in PyMuPDF-1.25.5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants