https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41932
--- Comment #40 from David Cook <[email protected]> --- Comment on attachment 198700 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198700 Bug 41932: Adjust generic class sort to allow zero-padding Review of attachment 198700: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=41932&attachment=198700) ----------------------------------------------------------------- Upon review... the code is not very easy to read at all and I think it might not validate/sanitize the inputs enough, although I haven't done a practical check yet. We want to be careful when it comes to regex inputs. I wish we hadn't added regex to other parts of Koha because it's a nightmare to secure. While the code is concise, it's very difficult to read. It's Perlish for sure but it makes code review and maintenance much harder. Might be worth breaking the zeropad function into a few different functions and unit testing them... ::: C4/ClassSortRoutine/Generic.pm @@ +90,5 @@ > + $format =~ /:/ > + ? split /:/, $format, 2 > + : ( q{}, $format ); > + if ($filter) { > + my $test = eval { $key =~ /$filter/ ? 1 : 0 }; Upon review, it seems that $filter isn't sufficiently validated. @@ +98,5 @@ > + > + my @pairs = map { my $p = $_; $p =~ s/\s*//g; $p } map { split /,/, > $_, 2 } split /;/, $pairs; > + if ( @pairs % 2 > 0 || grep { /\D|^0?$/ } @pairs ) { > + > + # We expect: even number, only digits, not zero or empty That said, it looks like we validate some things here but maybe that's the pairs? -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list -- [email protected] To unsubscribe send an email to [email protected] website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
