Summary: | Patron card creator infinite loop during line wrapping in template/layout incompatibility | ||
---|---|---|---|
Product: | Koha | Reporter: | David Cook <dcook> |
Component: | Tools | Assignee: | David Cook <dcook> |
Status: | Pushed to stable --- | QA Contact: | Marcel de Rooy <m.de.rooy> |
Severity: | critical | ||
Priority: | P5 - low | CC: | fridolin.somers, m.de.rooy |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | Trivial patch | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: |
25.05.00,24.11.06
|
Circulation function: | |
Attachments: |
Bug 39295: Prevent infinite loop on line wrapping
Bug 39295: Prevent infinite loop on line wrapping Bug 39295: Prevent infinite loop on line wrapping |
Description
David Cook
2025-03-11 01:49:37 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. 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 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> while (1) { Nothing beats this kind of constructs ;) 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> Should it be critical? Perhaps major would be enough ;) This code is horrible. This is kind of a trust signoff :) (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... Pushed for 25.05! Well done everyone, thank you! Pushed to 24.11.x for 24.11.06 |