Re: Fix proposal for comparaison bugs in PostgreSQL::Version
От | Andrew Dunstan |
---|---|
Тема | Re: Fix proposal for comparaison bugs in PostgreSQL::Version |
Дата | |
Msg-id | ea18cfc3-7f8e-8975-b095-226b1865e2e7@dunslane.net обсуждение исходный текст |
Ответ на | Fix proposal for comparaison bugs in PostgreSQL::Version (Jehan-Guillaume de Rorthais <jgdr@dalibo.com>) |
Ответы |
Re: Fix proposal for comparaison bugs in PostgreSQL::Version
(Michael Paquier <michael@paquier.xyz>)
Re: Fix proposal for comparaison bugs in PostgreSQL::Version (Jehan-Guillaume de Rorthais <jgdr@dalibo.com>) Re: Fix proposal for comparaison bugs in PostgreSQL::Version (Justin Pryzby <pryzby@telsasoft.com>) |
Список | pgsql-hackers |
On 2022-06-28 Tu 16:53, Jehan-Guillaume de Rorthais wrote: > Hi, > > I found a comparaison bug when using the PostgreSQL::Version module. See: > > $ perl -I. -MPostgreSQL::Version -le ' > my $v = PostgreSQL::Version->new("9.6"); > > print "not 9.6 > 9.0" unless $v > 9.0; > print "not 9.6 < 9.0" unless $v < 9.0; > print "9.6 <= 9.0" if $v <= 9.0; > print "9.6 >= 9.0" if $v >= 9.0;' > not 9.6 > 9.0 > not 9.6 < 9.0 > 9.6 <= 9.0 > 9.6 >= 9.0 > > When using < or >, 9.6 is neither greater or lesser than 9.0. > When using <= or >=, 9.6 is equally greater and lesser than 9.0. > The bug does not show up if you compare with "9.0" instead of 9.0. > This bug is triggered with devel versions, eg. 14beta1 <=> 14. > > The bug appears when both objects have a different number of digit in the > internal array representation: > > $ perl -I. -MPostgreSQL::Version -MData::Dumper -le ' > print Dumper(PostgreSQL::Version->new("9.0")->{num}); > print Dumper(PostgreSQL::Version->new(9.0)->{num}); > print Dumper(PostgreSQL::Version->new(14)->{num}); > print Dumper(PostgreSQL::Version->new("14beta1")->{num});' > $VAR1 = [ '9', '0' ]; > $VAR1 = [ '9' ]; > $VAR1 = [ '14' ]; > $VAR1 = [ '14', -1 ]; > > Because of this, The following loop in "_version_cmp" is wrong because we are > comparing two versions with different size of 'num' array: > > for (my $idx = 0;; $idx++) > { > return 0 unless (defined $an->[$idx] && defined $bn->[$idx]); > return $an->[$idx] <=> $bn->[$idx] > if ($an->[$idx] <=> $bn->[$idx]); > } > > > If we want to keep this internal array representation, the only fix I can think > of would be to always use a 4 element array defaulted to 0. Previous examples > would be: > > $VAR1 = [ 9, 0, 0, 0 ]; > $VAR1 = [ 9, 0, 0, 0 ]; > $VAR1 = [ 14, 0, 0, 0 ]; > $VAR1 = [ 14, 0, 0, -1 ]; > > A better fix would be to store the version internally as version_num that are > trivial to compute and compare. Please, find in attachment an implementation of > this. > > The patch is a bit bigger because it improved the devel version to support > rc/beta/alpha comparison like 14rc2 > 14rc1. > > Moreover, it adds a bunch of TAP tests to check various use cases. Nice catch, but this looks like massive overkill. I think we can very simply fix the test in just a few lines of code, instead of a 190 line fix and a 130 line TAP test. It was never intended to be able to compare markers like rc1 vs rc2, and I don't see any need for it. If you can show me a sane use case I'll have another look, but right now it seems quite unnecessary. Here's my proposed fix. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Вложения
В списке pgsql-hackers по дате отправления:
Предыдущее
От: Hannu KrosingДата:
Сообщение: Re: Hardening PostgreSQL via (optional) ban on local file system access