[Bug 22429] New: Infinite loop in patron card printing
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22429 Bug ID: 22429 Summary: Infinite loop in patron card printing Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Label/patron card printing Assignee: cnighswonger@foundations.edu Reporter: dcook@prosentient.com.au QA Contact: testopia@bugs.koha-community.org Target Milestone: --- I ran into an infinite loop in patron card printing in the following scenario: if (($string_width + $text_attribs->{'llx'}) > $self->{'width'}) Then there's an odd "trim" or "wrap" which isn't working, which means that the following condition is not met and leads to an infinite loop: if (($string_width + $text_attribs->{'llx'}) < $self->{'width'}) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22429 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #1 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- We have seen conditions where exporting a batch of patron cards brings down the instance. Do you know how the patron cards need to be set up to create this one? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22429 --- Comment #2 from David Cook <dcook@prosentient.com.au> --- I'm going to follow this up more tomorrow I hope. I need to gather more data regarding these lines: $line =~ m/^.*(\s.*\s*|\s&|\<.*\>)$/; warn sprintf('Line wrap failed. DEBUG INFO: Data: \'%s\'\n Method: C4::Patroncards->draw_text Additional Information: Line wrap regexp failed. (Please file in this information in a bug report at http://bugs.koha-community.org', $line) and last WRAP_LINES if !$1; $trim = $1 . $trim; $line =~ s/$1//; $string_width = C4::Creators::PDF->StrWidth($line, $text_attribs->{'font'}, $text_attribs->{'font_size'}); -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22429 --- Comment #3 from David Cook <dcook@prosentient.com.au> --- (In reply to Katrin Fischer from comment #1)
We have seen conditions where exporting a batch of patron cards brings down the instance. Do you know how the patron cards need to be set up to create this one?
I have a bit of data but I need to gather more. Unfortunately it's the end of my work day. However, it's a "<firstname> <surname>" scenario and the name is quite long it seems. And when the Lower Left X for the layout is quite large, the two combine to require line wrapping. But at a glance I don't see how the line wrapping code could possibly work. In a CGI environment, it was causing huge problems as infinitely looping processes were running in the background and chewing up more and more resources as new processes were launched (and timed out at the web server end). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22429 --- Comment #4 from David Cook <dcook@prosentient.com.au> --- (In reply to Katrin Fischer from comment #1)
We have seen conditions where exporting a batch of patron cards brings down the instance. Do you know how the patron cards need to be set up to create this one?
I don't have master conditions yet but in a 17.05.x I've reproduced with the following: -- Layout: Units: PostScript Points Field 1 Text: <firstname> <surname> Font: Times-Roman Font size: 20pt Text alignment: Center Lower left X coordinate: 40pt Lower left Y coordinate: 20pt Barcode Print card number as barcode: checked Lower left X coordinate: 0 Lower left Y coordinate: 50 Print card number as text under barcode: checked -- -- Template: Units: SI Millimeters Page height: 32mm Page width: 95mm Card width: 95mm Card height: 30mm Top page margin: 5mm Left page margin: 2mm Number of columns: 1 Number of rows: 1 -- -- User: Cardnumber: 1 Surname: Test Firnatme: 12345678 123456 (Test) -- This can generate an infinitely looping process of /home/dcook/git/patroncards/create-pdf.pl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22429 --- Comment #5 from David Cook <dcook@prosentient.com.au> --- Forgot to mention the problem is in C4/Patroncards/Patroncard.pm -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22429 --- Comment #6 from David Cook <dcook@prosentient.com.au> --- Aha... it's the parentheses which are the problem. They're being interpreted as regex metacharacters and not literal values! First line: '12345678 123456 (Test) Test' First string width: 299.775390625 First $1: ' Test' First $trim: '' Next line: '12345678 123456 (Test)' Next string width: 249.94140625 Next $1: ' (Test)' Next $trim: ' (Test) Test' Next line: '12345678 123456 (Test)' Next string width: 249.94140625 Next $1: ' (Test)' Next $trim: ' (Test) (Test) Test' And the $trim just keeps prepending (Test) over and over again. -- Fortunately, this is a very easy solve. I change the following $line =~ s/$1//; to $line =~ s/\Q$1\E//; And voila. It works. I bet this sort of "regular expression injection" could bite us in other parts of Koha... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22429 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22429 --- Comment #7 from David Cook <dcook@prosentient.com.au> --- Created attachment 85972 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85972&action=edit Bug 22429: Infinite loop in patron card printing Text fields in Patron Card Text Layouts can contain regular expression metacharacters, which - instead of being treated as literal values - are interpreted and prevent line wrapping. This causes the process to get stuck in an infinite loop, which keeps running even after the web server has timed out (at least when using CGI). This patch escapes the relevant input from the text field so the regular expression substitution treats characters as literals instead of as metacharacters. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22429 --- Comment #8 from David Cook <dcook@prosentient.com.au> --- Test plan: 1. Create data in your test Koha as follows: Layout: Units: PostScript Points Field 1 Text: <firstname> <surname> Font: Times-Roman Font size: 20pt Text alignment: Center Lower left X coordinate: 40pt Lower left Y coordinate: 20pt Barcode Print card number as barcode: checked Lower left X coordinate: 0 Lower left Y coordinate: 50 Print card number as text under barcode: checked -- -- Template: Units: SI Millimeters Page height: 32mm Page width: 95mm Card width: 95mm Card height: 30mm Top page margin: 5mm Left page margin: 2mm Number of columns: 1 Number of rows: 1 -- -- User: Cardnumber: 1 Surname: Test Firstname: 12345678 123456 (Test) 2. Before applying the patch, try to export a batch that contains the user with cardnumber 1 3. Note that Apache will timeout and that create-pdf.pl infinitely loops in the background (I haven't tested with Plack, but Katrin has mentioned in https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22429#c1 that it will bring down the instance.) 3a. ps -efww | grep "create-pdf" 4. Apply the patch 5. Note that the patron card is generated and downloaded via your browser -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22429 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|cnighswonger@foundations.ed |dcook@prosentient.com.au |u | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22429 Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |christian.stelzenmueller@bs | |z-bw.de -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22429 --- Comment #9 from Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de> ---
"Aha... it's the parentheses which are the problem. They're being interpreted as regex metacharacters and not literal values!"
We noticed that there seems to be an infinite loop if the template has some or all entries with "0" in it (like in default when creating a new template). Could this be the same cause? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22429 --- Comment #10 from David Cook <dcook@prosentient.com.au> --- (In reply to Christian Stelzenmüller from comment #9)
"Aha... it's the parentheses which are the problem. They're being interpreted as regex metacharacters and not literal values!"
We noticed that there seems to be an infinite loop if the template has some or all entries with "0" in it (like in default when creating a new template). Could this be the same cause?
I don't think it would be the exact same cause, but probably related. My guess would be that the other regular expression used for "trimming" the long line might have problems with the 0s. I could probably look at that later today. Infinite loops are very easy to encounter with this code because it only breaks the loops for some very specific positive conditions. There probably should be some kind of sanity check to prevent infinite loops, or a more mathematically precise loop... I have an urgent matter at hand but I'll keep this in mind to look at later today... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22429 --- Comment #11 from David Cook <dcook@prosentient.com.au> --- (In reply to Christian Stelzenmüller from comment #9)
"Aha... it's the parentheses which are the problem. They're being interpreted as regex metacharacters and not literal values!"
We noticed that there seems to be an infinite loop if the template has some or all entries with "0" in it (like in default when creating a new template). Could this be the same cause?
Sorry, I just realized that you were referring to the template rather than the layout. Yes, it would be a related problem, but a separate issue. If 'label_width' is 0, then $self->{'width'} will be 0. That means that this condition will be triggered: if (($string_width + $text_attribs->{'llx'}) > $self->{'width'}) And because $self->{'width'} is 0, then the following condition would never feasibly be triggered: if (($string_width + $text_attribs->{'llx'}) < $self->{'width'}) And this condition is the only thing that can stop the infinite loop. Really the line wrapping code looks like it would benefit from a re-write. I'd suggest opening a new bug report. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22429 Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |22462 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22462 [Bug 22462] Crash in patron card printing -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22429 Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=22462 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22429 Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=21052 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22429 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org, | |victor.grousset@biblibre.co | |m -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22429 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85972|0 |1 is obsolete| | --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 86520 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=86520&action=edit Bug 22429: Infinite loop in patron card printing Text fields in Patron Card Text Layouts can contain regular expression metacharacters, which - instead of being treated as literal values - are interpreted and prevent line wrapping. This causes the process to get stuck in an infinite loop, which keeps running even after the web server has timed out (at least when using CGI). This patch escapes the relevant input from the text field so the regular expression substitution treats characters as literals instead of as metacharacters. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22429 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |martin.renvoize@ptfs-europe | |.com --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I agree that the other issues should be opened as a distinct bug and as such having testing this code I'm signing off on it. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22429 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |22497 See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=22497 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22497 [Bug 22497] '0' width can lead to an infinite loop. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22429 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA Blocks|22497 | --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Trivial fix.. I've decide to go straight for QA Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22497 [Bug 22497] '0' width can lead to an infinite loop. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22429 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |22497 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22497 [Bug 22497] '0' width can lead to an infinite loop. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22429 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com Status|Passed QA |Pushed to Master --- Comment #15 from Nick Clemens <nick@bywatersolutions.com> --- Awesome work all! Pushed to master for 19.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22429 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable --- Comment #16 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Pushed to 18.11.x for 18.11.04 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22429 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com --- Comment #17 from Lucas Gass <lucas@bywatersolutions.com> --- backported to 18.05.x for 18.05.11 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22429 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #18 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Pushed to 17.11.x for 17.11.17 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org