Bug 39295 - Patron card creator infinite loop during line wrapping in template/layout incompatibility
Summary: Patron card creator infinite loop during line wrapping in template/layout inc...
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: Main
Hardware: All All
: P5 - low critical
Assignee: David Cook
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2025-03-11 01:49 UTC by David Cook
Modified: 2025-03-17 07:28 UTC (History)
1 user (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
25.05.00
Circulation function:


Attachments
Bug 39295: Prevent infinite loop on line wrapping (2.34 KB, patch)
2025-03-11 01:59 UTC, David Cook
Details | Diff | Splinter Review
Bug 39295: Prevent infinite loop on line wrapping (2.39 KB, patch)
2025-03-11 08:31 UTC, David Nind
Details | Diff | Splinter Review
Bug 39295: Prevent infinite loop on line wrapping (2.49 KB, patch)
2025-03-14 11:00 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2025-03-11 01:49:37 UTC
To reproduce:

0. Change Koha test user's firstnameto "Firstname" and surname to "Lastname"

1. Create a template:
Units: PostScript Points
Page height: 60pt
Page width: 60pt
Card height: 60pt
Card with: 60pt

Top page margin: 10pt
Left page margin: 10pt
Number of columns: 1
Number of rows: 1
Gap between columns: 10pt
Gap between rows: 10pt

2. Create a layout:
Units: US inches

Field 1:
Text: Library Name
Font: Times-Roman
Font size: 12 pt
Text alignment: Left
Lower left X coordinate: 100in
Lower left Y coordinate: 100in

Field 2:
Text: <firstname> <surname>
Font: Times-Roman
Font size: 10pt
Text alignment: Left
Lower left X coordinate: 5in
Lower left Y coordinate: 5in

3. Create a batch for borrowernumber 51 (if using "koha" test user in ktd)

4. Export using the above template and layout
5. Note that an infinite loop is triggered

If you use strace, the syscalls produced during the loop will look like this:
[pid 935276] newfstatat(AT_FDCWD, "/usr/share/fonts/truetype/dejavu/DejaVuSans-Bold.ttf", {st_mode=S_IFREG|0644, st_size=705684, ...}, 0) = 0
[pid 935276] newfstatat(AT_FDCWD, "/usr/share/fonts/truetype/dejavu/DejaVuSans-Bold.ttf", {st_mode=S_IFREG|0644, st_size=705684, ...}, 0) = 0
[pid 935276] newfstatat(AT_FDCWD, "/usr/share/fonts/truetype/dejavu/DejaVuSans-Bold.ttf", {st_mode=S_IFREG|0644, st_size=705684, ...}, 0) = 0
Comment 1 David Cook 2025-03-11 01:52:27 UTC
The patron creator code is problematic in a lot of ways, and I haven't worked out the exact interplay between template and layout that causes this problem.

All I know is that the infinite loop happens because in the WRAP_LINES loop the $line can be set to "" which causes $string_width to be set to 0, which creates an infinite loop as it tries to wrap a string which doesn't exist to create a label which is impossible.
Comment 2 David Cook 2025-03-11 01:59:35 UTC
Created attachment 179133 [details] [review]
Bug 39295: Prevent infinite loop on line wrapping

This change checks that the string width is larger than 0 before
attempting to perform line wrapping on the line.

Test plan:
0. Apply the patch and koha-plack --restart kohadev
1. Follow instructions at end of this message[1]
2. Note that the infinite loop does not happen and that a blank
card is created
3. Export the batch using the test layout and template that come
with koha-testing-docker
4. Note that the patron card is correctly created

[1]
To reproduce:

0. Change Koha test user's firstnameto "Firstname" and surname to "Lastname"

1. Create a template:
Units: PostScript Points
Page height: 60pt
Page width: 60pt
Card height: 60pt
Card with: 60pt

Top page margin: 10pt
Left page margin: 10pt
Number of columns: 1
Number of rows: 1
Gap between columns: 10pt
Gap between rows: 10pt

2. Create a layout:
Units: US inches

Field 1:
Text: Library Name
Font: Times-Roman
Font size: 12 pt
Text alignment: Left
Lower left X coordinate: 100in
Lower left Y coordinate: 100in

Field 2:
Text: <firstname> <surname>
Font: Times-Roman
Font size: 10pt
Text alignment: Left
Lower left X coordinate: 5in
Lower left Y coordinate: 5in

3. Create a batch for borrowernumber 51 (if using "koha" test user in ktd)

4. Export using the above template and layout
Comment 3 David Nind 2025-03-11 08:31:26 UTC
Created attachment 179141 [details] [review]
Bug 39295: Prevent infinite loop on line wrapping

This change checks that the string width is larger than 0 before
attempting to perform line wrapping on the line.

Test plan:
0. Apply the patch and koha-plack --restart kohadev
1. Follow instructions at end of this message[1]
2. Note that the infinite loop does not happen and that a blank
card is created
3. Export the batch using the test layout and template that come
with koha-testing-docker
4. Note that the patron card is correctly created

[1]
To reproduce:

0. Change Koha test user's firstnameto "Firstname" and surname to "Lastname"

1. Create a template:
Units: PostScript Points
Page height: 60pt
Page width: 60pt
Card height: 60pt
Card with: 60pt

Top page margin: 10pt
Left page margin: 10pt
Number of columns: 1
Number of rows: 1
Gap between columns: 10pt
Gap between rows: 10pt

2. Create a layout:
Units: US inches

Field 1:
Text: Library Name
Font: Times-Roman
Font size: 12 pt
Text alignment: Left
Lower left X coordinate: 100in
Lower left Y coordinate: 100in

Field 2:
Text: <firstname> <surname>
Font: Times-Roman
Font size: 10pt
Text alignment: Left
Lower left X coordinate: 5in
Lower left Y coordinate: 5in

3. Create a batch for borrowernumber 51 (if using "koha" test user in ktd)

4. Export using the above template and layout

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 Marcel de Rooy 2025-03-14 10:58:12 UTC
            while (1) {

Nothing beats this kind of constructs ;)
Comment 5 Marcel de Rooy 2025-03-14 11:00:27 UTC
Created attachment 179322 [details] [review]
Bug 39295: Prevent infinite loop on line wrapping

This change checks that the string width is larger than 0 before
attempting to perform line wrapping on the line.

Test plan:
0. Apply the patch and koha-plack --restart kohadev
1. Follow instructions at end of this message[1]
2. Note that the infinite loop does not happen and that a blank
card is created
3. Export the batch using the test layout and template that come
with koha-testing-docker
4. Note that the patron card is correctly created

[1]
To reproduce:

0. Change Koha test user's firstnameto "Firstname" and surname to "Lastname"

1. Create a template:
Units: PostScript Points
Page height: 60pt
Page width: 60pt
Card height: 60pt
Card with: 60pt

Top page margin: 10pt
Left page margin: 10pt
Number of columns: 1
Number of rows: 1
Gap between columns: 10pt
Gap between rows: 10pt

2. Create a layout:
Units: US inches

Field 1:
Text: Library Name
Font: Times-Roman
Font size: 12 pt
Text alignment: Left
Lower left X coordinate: 100in
Lower left Y coordinate: 100in

Field 2:
Text: <firstname> <surname>
Font: Times-Roman
Font size: 10pt
Text alignment: Left
Lower left X coordinate: 5in
Lower left Y coordinate: 5in

3. Create a batch for borrowernumber 51 (if using "koha" test user in ktd)

4. Export using the above template and layout

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 6 Marcel de Rooy 2025-03-14 11:01:03 UTC
Should it be critical? Perhaps major would be enough ;)
Comment 7 Marcel de Rooy 2025-03-14 11:01:33 UTC
This code is horrible. This is kind of a trust signoff :)
Comment 8 David Cook 2025-03-16 22:45:19 UTC
(In reply to Marcel de Rooy from comment #4)
>             while (1) {
> 
> Nothing beats this kind of constructs ;)

I'm glad that someone else feels my pain haha. 

(In reply to Marcel de Rooy from comment #6)
> Should it be critical? Perhaps major would be enough ;)

I wondered about this as well. I marked it as critical because it has the ability to make a Koha instance unusable with a couple of clicks.

I think a person could argue "major" since hopefully most people won't create the necessary conditions to trigger it, but it's really bad when it is triggered. 100% CPU for the Starman worker processes and no responsiveness for the user. 

(In reply to Marcel de Rooy from comment #7)
> This code is horrible. This is kind of a trust signoff :)

Merci :). This code is horrible. Sometimes, I wonder what it'll take for us to replace it. It limps along as "good enough" I think...
Comment 9 Katrin Fischer 2025-03-17 07:28:50 UTC
Pushed for 25.05!

Well done everyone, thank you!