Summary: | Infinite loop in patron card printing | ||
---|---|---|---|
Product: | Koha | Reporter: | David Cook <dcook> |
Component: | Label/patron card printing | Assignee: | David Cook <dcook> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | major | ||
Priority: | P5 - low | CC: | christian.stelzenmueller, fridolin.somers, jonathan.druart, lucas, martin.renvoize, nick, victor |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: |
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22462 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21052 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22497 |
||
Change sponsored?: | --- | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Bug Depends on: | |||
Bug Blocks: | 22497, 22462 | ||
Attachments: |
Bug 22429: Infinite loop in patron card printing
Bug 22429: Infinite loop in patron card printing |
Description
David Cook
2019-02-28 07:01:27 UTC
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'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'}); (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). (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 Forgot to mention the problem is in C4/Patroncards/Patroncard.pm 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... Created attachment 85972 [details] [review] 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. 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 > "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?
(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... (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. Created attachment 86520 [details] [review] 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> 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. Trivial fix.. I've decide to go straight for QA Awesome work all! Pushed to master for 19.05 Pushed to 18.11.x for 18.11.04 backported to 18.05.x for 18.05.11 Pushed to 17.11.x for 17.11.17 |