From 26724018067fca77977e343263487bf2b6b25ba2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Buclin?= Date: Mon, 2 Dec 2013 17:04:12 +0100 Subject: Bug 938300: vers_cmp() incorrectly compares module versions r=sgreen a=justdave --- Bugzilla/DB.pm | 3 +- Bugzilla/Install/Requirements.pm | 31 +++++++------ Bugzilla/Install/Util.pm | 66 --------------------------- Bugzilla/Version.pm | 98 ++++++++++++++++++++++++++++++++++++++-- install-module.pl | 2 +- query.cgi | 2 +- 6 files changed, 113 insertions(+), 89 deletions(-) diff --git a/Bugzilla/DB.pm b/Bugzilla/DB.pm index f5320cb7f..063e2cf69 100644 --- a/Bugzilla/DB.pm +++ b/Bugzilla/DB.pm @@ -17,11 +17,12 @@ use parent -norequire, qw(DBI::db); use Bugzilla::Constants; use Bugzilla::Install::Requirements; -use Bugzilla::Install::Util qw(vers_cmp install_string); +use Bugzilla::Install::Util qw(install_string); use Bugzilla::Install::Localconfig; use Bugzilla::Util; use Bugzilla::Error; use Bugzilla::DB::Schema; +use Bugzilla::Version; use List::Util qw(max); use Storable qw(dclone); diff --git a/Bugzilla/Install/Requirements.pm b/Bugzilla/Install/Requirements.pm index 06f855fc5..09ea4abe3 100644 --- a/Bugzilla/Install/Requirements.pm +++ b/Bugzilla/Install/Requirements.pm @@ -17,7 +17,7 @@ use 5.10.1; use strict; use Bugzilla::Constants; -use Bugzilla::Install::Util qw(vers_cmp install_string bin_loc +use Bugzilla::Install::Util qw(install_string bin_loc extension_requirement_packages); use List::Util qw(max); use Term::ANSIColor; @@ -86,7 +86,6 @@ use constant APACHE_PATH => [qw( # are 'blacklisted'--that is, even if the version is high enough, Bugzilla # will refuse to say that it's OK to run with that version. sub REQUIRED_MODULES { - my $perl_ver = sprintf('%vd', $^V); my @modules = ( { package => 'CGI.pm', @@ -125,7 +124,7 @@ sub REQUIRED_MODULES { { package => 'DBI', module => 'DBI', - version => (vers_cmp($perl_ver, '5.13.3') > -1) ? '1.614' : '1.54' + version => ($^V >= v5.13.3) ? '1.614' : '1.54' }, # 2.24 contains several useful text virtual methods. { @@ -187,7 +186,6 @@ sub REQUIRED_MODULES { }; sub OPTIONAL_MODULES { - my $perl_ver = sprintf('%vd', $^V); my @modules = ( { package => 'GD', @@ -198,8 +196,9 @@ sub OPTIONAL_MODULES { { package => 'Chart', module => 'Chart::Lines', - # Versions below 2.1 cannot be detected accurately. - version => '2.1', + # Versions below 2.4.1 cannot be compared accurately, see + # https://rt.cpan.org/Public/Bug/Display.html?id=28218. + version => '2.4.1', feature => [qw(new_charts old_charts)], }, { @@ -314,7 +313,7 @@ sub OPTIONAL_MODULES { # We need the 'utf8_mode' method of HTML::Parser, for HTML::Scrubber. package => 'HTML-Parser', module => 'HTML::Parser', - version => (vers_cmp($perl_ver, '5.13.3') > -1) ? '3.67' : '3.40', + version => ($^V >= v5.13.3) ? '3.67' : '3.40', feature => ['html_desc'], }, { @@ -668,8 +667,8 @@ sub check_graphviz { return $return; } -# This was originally clipped from the libnet Makefile.PL, adapted here to -# use the below vers_cmp routine for accurate version checking. +# This was originally clipped from the libnet Makefile.PL, adapted here for +# accurate version checking. sub have_vers { my ($params, $output) = @_; my $module = $params->{module}; @@ -694,15 +693,17 @@ sub have_vers { if ($@) { no strict 'refs'; $vnum = ${"${module}::VERSION"}; - } - $vnum ||= -1; - # Fix CPAN versions like 1.9304. - if ($module eq 'CPAN' and $vnum =~ /^(\d\.\d{2})\d{2}$/) { - $vnum = $1; + # If we come here, then the version is not a valid one. + # We try to sanitize it. + if ($vnum =~ /^((\d+)(\.\d+)*)/) { + $vnum = $1; + } } + $vnum ||= -1; - my $vok = (vers_cmp($vnum,$wanted) > -1); + # Must do a string comparison as $vnum may be of the form 5.10.1. + my $vok = ($vnum ne '-1' && version->new($vnum) >= version->new($wanted)) ? 1 : 0; my $blacklisted; if ($vok && $params->{blacklist}) { $blacklisted = grep($vnum =~ /$_/, @{$params->{blacklist}}); diff --git a/Bugzilla/Install/Util.pm b/Bugzilla/Install/Util.pm index 53cc9d2ec..66ea7169e 100644 --- a/Bugzilla/Install/Util.pm +++ b/Bugzilla/Install/Util.pm @@ -38,7 +38,6 @@ our @EXPORT_OK = qw( include_languages success template_include_path - vers_cmp init_console ); @@ -476,49 +475,6 @@ sub template_include_path { return \@include_path; } -# This is taken straight from Sort::Versions 1.5, which is not included -# with perl by default. -sub vers_cmp { - my ($a, $b) = @_; - - # Remove leading zeroes - Bug 344661 - $a =~ s/^0*(\d.+)/$1/; - $b =~ s/^0*(\d.+)/$1/; - - my @A = ($a =~ /([-.]|\d+|[^-.\d]+)/g); - my @B = ($b =~ /([-.]|\d+|[^-.\d]+)/g); - - my ($A, $B); - while (@A and @B) { - $A = shift @A; - $B = shift @B; - if ($A eq '-' and $B eq '-') { - next; - } elsif ( $A eq '-' ) { - return -1; - } elsif ( $B eq '-') { - return 1; - } elsif ($A eq '.' and $B eq '.') { - next; - } elsif ( $A eq '.' ) { - return -1; - } elsif ( $B eq '.' ) { - return 1; - } elsif ($A =~ /^\d+$/ and $B =~ /^\d+$/) { - if ($A =~ /^0/ || $B =~ /^0/) { - return $A cmp $B if $A cmp $B; - } else { - return $A <=> $B if $A <=> $B; - } - } else { - $A = uc $A; - $B = uc $B; - return $A cmp $B if $A cmp $B; - } - } - @A <=> @B; -} - sub no_checksetup_from_cgi { print "Content-Type: text/html; charset=UTF-8\r\n\r\n"; print install_string('no_checksetup_from_cgi'); @@ -894,28 +850,6 @@ Used by L to determine the languages' list which are compiled with the browser's I and the languages of installed templates. -=item C - -=over - -=item B - -This is a comparison function, like you would use in C, except that -it compares two version numbers. So, for example, 2.10 would be greater -than 2.2. - -It's based on versioncmp from L, with some Bugzilla-specific -fixes. - -=item B: C<$a> and C<$b> - The versions you want to compare. - -=item B - -C<-1> if C<$a> is less than C<$b>, C<0> if they are equal, or C<1> if C<$a> -is greater than C<$b>. - -=back - =back =head1 B diff --git a/Bugzilla/Version.pm b/Bugzilla/Version.pm index 1dcd2b141..c6b178a8a 100644 --- a/Bugzilla/Version.pm +++ b/Bugzilla/Version.pm @@ -10,9 +10,10 @@ package Bugzilla::Version; use 5.10.1; use strict; -use parent qw(Bugzilla::Object); +use parent qw(Bugzilla::Object Exporter); + +@Bugzilla::Version::EXPORT = qw(vers_cmp); -use Bugzilla::Install::Util qw(vers_cmp); use Bugzilla::Util; use Bugzilla::Error; @@ -200,6 +201,53 @@ sub _check_product { return Bugzilla->user->check_can_admin_product($product->name); } +############################### +##### Functions #### +############################### + +# This is taken straight from Sort::Versions 1.5, which is not included +# with perl by default. +sub vers_cmp { + my ($a, $b) = @_; + + # Remove leading zeroes - Bug 344661 + $a =~ s/^0*(\d.+)/$1/; + $b =~ s/^0*(\d.+)/$1/; + + my @A = ($a =~ /([-.]|\d+|[^-.\d]+)/g); + my @B = ($b =~ /([-.]|\d+|[^-.\d]+)/g); + + my ($A, $B); + while (@A and @B) { + $A = shift @A; + $B = shift @B; + if ($A eq '-' and $B eq '-') { + next; + } elsif ( $A eq '-' ) { + return -1; + } elsif ( $B eq '-') { + return 1; + } elsif ($A eq '.' and $B eq '.') { + next; + } elsif ( $A eq '.' ) { + return -1; + } elsif ( $B eq '.' ) { + return 1; + } elsif ($A =~ /^\d+$/ and $B =~ /^\d+$/) { + if ($A =~ /^0/ || $B =~ /^0/) { + return $A cmp $B if $A cmp $B; + } else { + return $A <=> $B if $A <=> $B; + } + } else { + $A = uc $A; + $B = uc $B; + return $A cmp $B if $A cmp $B; + } + } + return @A <=> @B; +} + 1; __END__ @@ -243,11 +291,51 @@ below. =item C - Description: Returns the total of bugs that belong to the version. +=over + +=item B + +Returns the total of bugs that belong to the version. + +=item B + +none + +=item B + +Integer with the number of bugs. + +=back + +=back + +=head1 FUNCTIONS + +=over + +=item C + +=over + +=item B + +This is a comparison function, like you would use in C, except that +it compares two version numbers. So, for example, 2.10 would be greater +than 2.2. + +It's based on versioncmp from L, with some Bugzilla-specific +fixes. - Params: none. +=item B - Returns: Integer with the number of bugs. +C<$a> and C<$b> - The versions you want to compare. + +=item B + +C<-1> if C<$a> is less than C<$b>, C<0> if they are equal, or C<1> if C<$a> +is greater than C<$b>. + +=back =back diff --git a/install-module.pl b/install-module.pl index 37ea8cc41..a7359917d 100755 --- a/install-module.pl +++ b/install-module.pl @@ -23,7 +23,7 @@ use Bugzilla::Install::CPAN; use Bugzilla::Constants; use Bugzilla::Install::Requirements; -use Bugzilla::Install::Util qw(bin_loc init_console vers_cmp); +use Bugzilla::Install::Util qw(bin_loc init_console); use Data::Dumper; use Getopt::Long; diff --git a/query.cgi b/query.cgi index 0e921ac0c..65a7d3c3d 100755 --- a/query.cgi +++ b/query.cgi @@ -18,9 +18,9 @@ use Bugzilla::User; use Bugzilla::Util; use Bugzilla::Error; use Bugzilla::Product; +use Bugzilla::Version; use Bugzilla::Keyword; use Bugzilla::Field; -use Bugzilla::Install::Util qw(vers_cmp); use Bugzilla::Token; ############### -- cgit v1.2.1