Bug 22497 - '0' width can lead to an infinite loop.
Summary: '0' width can lead to an infinite loop.
Status: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: Label/patron card printing (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Chris Nighswonger
QA Contact: Testopia
URL:
Keywords:
Depends on: 22429
Blocks:
  Show dependency treegraph
 
Reported: 2019-03-12 12:30 UTC by Martin Renvoize
Modified: 2019-05-06 02:28 UTC (History)
3 users (show)

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


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2019-03-12 12:30:56 UTC
> > "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.
Comment 1 Martin Renvoize 2019-03-12 12:31:30 UTC
See bug 22429 for context
Comment 2 Jonathan Druart 2019-05-04 13:50:39 UTC
(In reply to Martin Renvoize from comment #0)
> Really the line wrapping code looks like it would benefit from a re-write.

Should not we go for a quick fix instead?
Like last if $width == 0?
Comment 3 David Cook 2019-05-06 02:27:20 UTC
(In reply to Jonathan Druart from comment #2)
> (In reply to Martin Renvoize from comment #0)
> > Really the line wrapping code looks like it would benefit from a re-write.
> 
> Should not we go for a quick fix instead?
> Like last if $width == 0?

I was thinking something like:

last if $text_attribs->{'llx'}) >= $self->{'width'}

Although that's just off the top of my head.
Comment 4 David Cook 2019-05-06 02:28:14 UTC
And I really mean off the top of my head. Like maybe that should be:

last if $text_attribs->{'llx'}) > $self->{'width'}

I can't recall the exact interplay between those two values