https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32478
--- Comment #17 from David Gustafsson <[email protected]> --- (In reply to Marcel de Rooy from comment #15) > We remove these lines completely now and just trust ->yaml_preference: > - foreach my $line (@lines){ > - my ($field,$array) = split /:/, $line; > - next if !$array; > - $field =~ s/^\s*|\s*$//g; > - $array =~ s/[ [\]\r]//g; > - my @array = split /,/, $array; > - @array = map { $_ eq '""' || $_ eq "''" ? '' : $_ } @array; > - @array = map { $_ eq 'NULL' ? undef : $_ } @array; > - $pref_as_hash->{$field} = \@array; > - } > > But this is not the same. If I pasted correct YAML previously like below, it > would not work. > ccode : > - null > - NULL > - undefined > - 2 > - '3' > Now it does. > > But perhaps more serious is the opposite, if I am entering some valid YAML > array by 'forgetting' the first line: > - null > - NULL > - undefined > - 2 > - '3' > I will get back an array, while the former routine returned an empty hash ! > This will trigger an error like: Not a HASH reference at > /usr/share/koha/Koha/Item.pm line 2071. => foreach my $field (keys > %$denyingrules) { > > This is a bit long way of saying: I still think that we may need an > intermediate routine between pure YAML (yaml_preference) and what we expect > for specific preferences? I see your point, but was it not already possible to make that code crash (or even worse things) by for example using key values that does not exist on the item object, or for example using "delete" as key :/ sub is_denied_renewal { my ( $self ) = @_; my $denyingrules = Koha::Config::SysPrefs->find('ItemsDeniedRenewal')->get_yaml_pref_hash(); return 0 unless $denyingrules; foreach my $field (keys %$denyingrules) { my $val = $self->$field; Validation would be nice, but feels like out of the scope for this issue. I will update the patch to check that an actual hash before iterating over the keys though, as this particular case was not a problem before. If I'm not missing something $self->$field; where $field is user input is a security vulnerability and a pretty bad one at that. Should use get_column instead! I guess should create a new issue for this, or if we handle it in this one? -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
