From d923c725d90eb519feda5fe48cb15f4077a8bc24 Mon Sep 17 00:00:00 2001 From: "bzrmirror%bugzilla.org" Date: Thu, 11 Sep 2014 00:02:29 +0000 Subject: [PATCH] Bug 1052724: Use JSON::XS instead of Data::Dumper to store parameters into data/params r=dkl r=wurblzap a=sgreen git-svn-id: svn://10.0.0.236/trunk@265568 18797224-902f-48f8-a5cc-f745e15eee43 --- mozilla/webtools/bugzilla/.bzrrev | 2 +- mozilla/webtools/bugzilla/.gitrev | 2 +- mozilla/webtools/bugzilla/Bugzilla.pm | 10 +- mozilla/webtools/bugzilla/Bugzilla/Config.pm | 98 +++++++++++-------- .../webtools/bugzilla/Bugzilla/Install/DB.pm | 2 +- .../bugzilla/Bugzilla/Install/Filesystem.pm | 2 +- .../bugzilla/Bugzilla/Install/Requirements.pm | 13 ++- mozilla/webtools/bugzilla/checksetup.pl | 4 +- .../bugzilla/docs/en/rst/administration.rst | 4 +- .../template/en/default/setup/strings.txt.pl | 1 - 10 files changed, 77 insertions(+), 61 deletions(-) diff --git a/mozilla/webtools/bugzilla/.bzrrev b/mozilla/webtools/bugzilla/.bzrrev index 7d4d6781414..e6398b5094a 100644 --- a/mozilla/webtools/bugzilla/.bzrrev +++ b/mozilla/webtools/bugzilla/.bzrrev @@ -1 +1 @@ -9132 \ No newline at end of file +9133 \ No newline at end of file diff --git a/mozilla/webtools/bugzilla/.gitrev b/mozilla/webtools/bugzilla/.gitrev index b35ba0b15e6..46fb08c06ab 100644 --- a/mozilla/webtools/bugzilla/.gitrev +++ b/mozilla/webtools/bugzilla/.gitrev @@ -1 +1 @@ -5802599c4c23b92aee027d763d73002d1880e31e \ No newline at end of file +e1603d01bbc3523b622db2f295400aa5a5f14509 \ No newline at end of file diff --git a/mozilla/webtools/bugzilla/Bugzilla.pm b/mozilla/webtools/bugzilla/Bugzilla.pm index e1f2fde3b51..7d935db4808 100644 --- a/mozilla/webtools/bugzilla/Bugzilla.pm +++ b/mozilla/webtools/bugzilla/Bugzilla.pm @@ -122,8 +122,8 @@ sub init_page { # # This code must go here. It cannot go anywhere in Bugzilla::CGI, because # it uses Template, and that causes various dependency loops. - if (Bugzilla->params->{"shutdownhtml"} - && !grep { $_ eq $script } SHUTDOWNHTML_EXEMPT) + if (!grep { $_ eq $script } SHUTDOWNHTML_EXEMPT + and Bugzilla->params->{'shutdownhtml'}) { # Allow non-cgi scripts to exit silently (without displaying any # message), if desired. At this point, no DBI call has been made @@ -939,9 +939,9 @@ Change the database object to refer to the main database. =item C -The current Parameters of Bugzilla, as a hashref. If C -does not exist, then we return an empty hashref. If C -is unreadable or is not valid perl, we C. +The current Parameters of Bugzilla, as a hashref. If C +does not exist, then we return an empty hashref. If C +is unreadable or is not valid, we C. =item C diff --git a/mozilla/webtools/bugzilla/Bugzilla/Config.pm b/mozilla/webtools/bugzilla/Bugzilla/Config.pm index 04c2c0c6f19..a41b30abe3a 100644 --- a/mozilla/webtools/bugzilla/Bugzilla/Config.pm +++ b/mozilla/webtools/bugzilla/Bugzilla/Config.pm @@ -12,11 +12,16 @@ use strict; use warnings; use parent qw(Exporter); +use autodie qw(:default); use Bugzilla::Constants; use Bugzilla::Hook; -use Data::Dumper; +use Bugzilla::Util qw(trick_taint); + +use JSON::XS; +use File::Slurp; use File::Temp; +use File::Basename; # Don't export localvars by default - people should have to explicitly # ask for it, as a (probably futile) attempt to stop code using it @@ -93,8 +98,30 @@ sub SetParam { sub update_params { my ($params) = @_; my $answer = Bugzilla->installation_answers; + my $datadir = bz_locations()->{'datadir'}; + my $param; + + # If the old data/params file using Data::Dumper output still exists, + # read it. It will be deleted once the parameters are stored in the new + # data/params.js file. + my $old_file = "$datadir/params"; + + if (-e $old_file) { + require Safe; + my $s = new Safe; + + $s->rdo($old_file); + die "Error reading $old_file: $!" if $!; + die "Error evaluating $old_file: $@" if $@; + + # Now read the param back out from the sandbox. + $param = \%{ $s->varglob('param') }; + } + else { + # Read the new data/params.js file. + $param = read_param_file(); + } - my $param = read_param_file(); my %new_params; # If we didn't return any param values, then this is a new installation. @@ -212,7 +239,6 @@ sub update_params { } # Write any old parameters to old-params.txt - my $datadir = bz_locations()->{'datadir'}; my $old_param_file = "$datadir/old-params.txt"; if (scalar(keys %oldparams)) { my $op_file = new IO::File($old_param_file, '>>', 0600) @@ -222,12 +248,9 @@ sub update_params { " and so have been\nmoved from your parameters file into", " $old_param_file:\n"; - local $Data::Dumper::Terse = 1; - local $Data::Dumper::Indent = 0; - my $comma = ""; foreach my $item (keys %oldparams) { - print $op_file "\n\n$item:\n" . Data::Dumper->Dump([$oldparams{$item}]) . "\n"; + print $op_file "\n\n$item:\n" . $oldparams{$item} . "\n"; print "${comma}$item"; $comma = ", "; } @@ -258,6 +281,11 @@ sub update_params { write_params($param); + if (-e $old_file) { + unlink $old_file; + say "$old_file has been converted into $old_file.js, using the JSON format."; + } + # Return deleted params and values so that checksetup.pl has a chance # to convert old params to new data. return %oldparams; @@ -266,22 +294,10 @@ sub update_params { sub write_params { my ($param_data) = @_; $param_data ||= Bugzilla->params; + my $param_file = bz_locations()->{'datadir'} . '/params.js'; - my $datadir = bz_locations()->{'datadir'}; - my $param_file = "$datadir/params"; - - local $Data::Dumper::Sortkeys = 1; - - my ($fh, $tmpname) = File::Temp::tempfile('params.XXXXX', - DIR => $datadir ); - - print $fh (Data::Dumper->Dump([$param_data], ['*param'])) - || die "Can't write param file: $!"; - - close $fh; - - rename $tmpname, $param_file - or die "Can't rename $tmpname to $param_file: $!"; + my $json_data = JSON::XS->new->canonical->pretty->encode($param_data); + write_file($param_file, { binmode => ':utf8', atomic => 1 }, \$json_data); # It's not common to edit parameters and loading # Bugzilla::Install::Filesystem is slow. @@ -295,21 +311,23 @@ sub write_params { sub read_param_file { my %params; - my $datadir = bz_locations()->{'datadir'}; - if (-e "$datadir/params") { - # Note that checksetup.pl sets file permissions on '$datadir/params' + my $file = bz_locations()->{'datadir'} . '/params.js'; - # Using Safe mode is _not_ a guarantee of safety if someone does - # manage to write to the file. However, it won't hurt... - # See bug 165144 for not needing to eval this at all - my $s = new Safe; + if (-e $file) { + my $data; + read_file($file, binmode => ':utf8', buf_ref => \$data); - $s->rdo("$datadir/params"); - die "Error reading $datadir/params: $!" if $!; - die "Error evaluating $datadir/params: $@" if $@; - - # Now read the param back out from the sandbox - %params = %{$s->varglob('param')}; + # If params.js has been manually edited and e.g. some quotes are + # missing, we don't want JSON::XS to leak the content of the file + # to all users in its error message, so we have to eval'uate it. + %params = eval { %{JSON::XS->new->decode($data)} }; + if ($@) { + my $error_msg = (basename($0) eq 'checksetup.pl') ? + $@ : 'run checksetup.pl to see the details.'; + die "Error parsing $file: $error_msg"; + } + # JSON::XS doesn't detaint data for us. + trick_taint($params{$_}) foreach keys %params; } elsif ($ENV{'SERVER_SOFTWARE'}) { # We're in a CGI, but the params file doesn't exist. We can't @@ -319,7 +337,7 @@ sub read_param_file { # so that the user sees the error. require CGI::Carp; CGI::Carp->import('fatalsToBrowser'); - die "The $datadir/params file does not exist." + die "The $file file does not exist." . ' You probably need to run checksetup.pl.', } return \%params; @@ -375,7 +393,7 @@ specified. Description: Writes the parameters to disk. Params: C<$params> (optional) - A hashref to write to the disk - instead of Cparams>. Used only by + instead of Cparams>. Used only by C. Returns: nothing @@ -383,12 +401,12 @@ Returns: nothing =item C Description: Most callers should never need this. This is used - by Cparams> to directly read C<$datadir/params> - and load it into memory. Use Cparams> instead. + by Cparams> to directly read C<$datadir/params.js> + and load it into memory. Use Cparams> instead. Params: none -Returns: A hashref containing the current params in C<$datadir/params>. +Returns: A hashref containing the current params in C<$datadir/params.js>. =item C diff --git a/mozilla/webtools/bugzilla/Bugzilla/Install/DB.pm b/mozilla/webtools/bugzilla/Bugzilla/Install/DB.pm index 0b0603970eb..2bf3c0c1a90 100644 --- a/mozilla/webtools/bugzilla/Bugzilla/Install/DB.pm +++ b/mozilla/webtools/bugzilla/Bugzilla/Install/DB.pm @@ -2561,7 +2561,7 @@ sub _fix_whine_queries_title_and_op_sys_value { undef, "Other", "other"); if (Bugzilla->params->{'defaultopsys'} eq 'other') { # We can't actually fix the param here, because WriteParams() will - # make $datadir/params unwriteable to the webservergroup. + # make $datadir/params.js unwriteable to the webservergroup. # It's too much of an ugly hack to copy the permission-fixing code # down to here. (It would create more potential future bugs than # it would solve problems.) diff --git a/mozilla/webtools/bugzilla/Bugzilla/Install/Filesystem.pm b/mozilla/webtools/bugzilla/Bugzilla/Install/Filesystem.pm index 157a8429eca..73536b4b5ac 100644 --- a/mozilla/webtools/bugzilla/Bugzilla/Install/Filesystem.pm +++ b/mozilla/webtools/bugzilla/Bugzilla/Install/Filesystem.pm @@ -171,7 +171,7 @@ sub FILESYSTEM { 'docs/style.css' => { perms => WS_SERVE }, 'docs/*/rel_notes.txt' => { perms => WS_SERVE }, 'docs/*/README.docs' => { perms => OWNER_WRITE }, - "$datadir/params" => { perms => CGI_WRITE }, + "$datadir/params.js" => { perms => CGI_WRITE }, "$datadir/old-params.txt" => { perms => OWNER_WRITE }, "$extensionsdir/create.pl" => { perms => OWNER_EXECUTE }, "$extensionsdir/*/*.pl" => { perms => WS_EXECUTE }, diff --git a/mozilla/webtools/bugzilla/Bugzilla/Install/Requirements.pm b/mozilla/webtools/bugzilla/Bugzilla/Install/Requirements.pm index 2ceb01cfd64..bec753830d8 100644 --- a/mozilla/webtools/bugzilla/Bugzilla/Install/Requirements.pm +++ b/mozilla/webtools/bugzilla/Bugzilla/Install/Requirements.pm @@ -166,6 +166,12 @@ sub REQUIRED_MODULES { module => 'File::Slurp', version => '9999.13', }, + { + package => 'JSON-XS', + module => 'JSON::XS', + # 2.0 is the first version that will work with JSON::RPC. + version => '2.01', + }, ); if (ON_WINDOWS) { @@ -298,13 +304,6 @@ sub OPTIONAL_MODULES { version => 0, feature => ['jsonrpc', 'rest'], }, - { - package => 'JSON-XS', - module => 'JSON::XS', - # 2.0 is the first version that will work with JSON::RPC. - version => '2.0', - feature => ['jsonrpc_faster'], - }, { package => 'Test-Taint', module => 'Test::Taint', diff --git a/mozilla/webtools/bugzilla/checksetup.pl b/mozilla/webtools/bugzilla/checksetup.pl index c7e8994e58d..1785faf5447 100755 --- a/mozilla/webtools/bugzilla/checksetup.pl +++ b/mozilla/webtools/bugzilla/checksetup.pl @@ -109,7 +109,7 @@ my $lc_hash = Bugzilla->localconfig; # At this point, localconfig is defined and is readable. So we know # everything we need to create the DB. We have to create it early, -# because some data required to populate data/params is stored in the DB. +# because some data required to populate data/params.js is stored in the DB. Bugzilla::DB::bz_check_requirements(!$silent); Bugzilla::DB::bz_create_database() if $lc_hash->{'db_check'}; @@ -364,7 +364,7 @@ L. =item 9 -Updates the system parameters (stored in F), using +Updates the system parameters (stored in F), using L. =item 10 diff --git a/mozilla/webtools/bugzilla/docs/en/rst/administration.rst b/mozilla/webtools/bugzilla/docs/en/rst/administration.rst index 81dc35107c5..d26e354b9dd 100644 --- a/mozilla/webtools/bugzilla/docs/en/rst/administration.rst +++ b/mozilla/webtools/bugzilla/docs/en/rst/administration.rst @@ -346,7 +346,7 @@ user_verify_class well, you may otherwise not be able to log back in to Bugzilla once you log out. If this happens to you, you will need to manually edit - :file:`data/params` and set user_verify_class to + :file:`data/params.js` and set user_verify_class to ``DB``. LDAPserver @@ -414,7 +414,7 @@ user_verify_class well, you may otherwise not be able to log back in to Bugzilla once you log out. If this happens to you, you will need to manually edit - :file:`data/params` and set user_verify_class to + :file:`data/params.js` and set user_verify_class to ``DB``. RADIUS_server diff --git a/mozilla/webtools/bugzilla/template/en/default/setup/strings.txt.pl b/mozilla/webtools/bugzilla/template/en/default/setup/strings.txt.pl index dc6a52fe6c2..6fb34371363 100644 --- a/mozilla/webtools/bugzilla/template/en/default/setup/strings.txt.pl +++ b/mozilla/webtools/bugzilla/template/en/default/setup/strings.txt.pl @@ -88,7 +88,6 @@ END feature_inbound_email => 'Inbound Email', feature_jobqueue => 'Mail Queueing', feature_jsonrpc => 'JSON-RPC Interface', - feature_jsonrpc_faster => 'Make JSON-RPC Faster', feature_new_charts => 'New Charts', feature_old_charts => 'Old Charts', feature_memcached => 'Memcached Support',