aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorFrédéric Buclin <LpSolit@gmail.com>2014-09-11 01:53:07 +0200
committerFrédéric Buclin <LpSolit@gmail.com>2014-09-11 01:53:07 +0200
commite1603d01bbc3523b622db2f295400aa5a5f14509 (patch)
tree53cfc18ee24e626912401bea9ab4be346e1e05ce
parent5802599c4c23b92aee027d763d73002d1880e31e (diff)
downloadbugs-e1603d01bbc3523b622db2f295400aa5a5f14509.tar
bugs-e1603d01bbc3523b622db2f295400aa5a5f14509.tar.gz
bugs-e1603d01bbc3523b622db2f295400aa5a5f14509.tar.bz2
bugs-e1603d01bbc3523b622db2f295400aa5a5f14509.tar.xz
bugs-e1603d01bbc3523b622db2f295400aa5a5f14509.zip
Bug 1052724: Use JSON::XS instead of Data::Dumper to store parameters into data/params
r=dkl r=wurblzap a=sgreen
-rw-r--r--Bugzilla.pm10
-rw-r--r--Bugzilla/Config.pm102
-rw-r--r--Bugzilla/Install/DB.pm2
-rw-r--r--Bugzilla/Install/Filesystem.pm2
-rw-r--r--Bugzilla/Install/Requirements.pm13
-rwxr-xr-xchecksetup.pl4
-rw-r--r--docs/en/rst/administration.rst4
-rw-r--r--template/en/default/setup/strings.txt.pl1
8 files changed, 77 insertions, 61 deletions
diff --git a/Bugzilla.pm b/Bugzilla.pm
index e1f2fde3b..7d935db48 100644
--- a/Bugzilla.pm
+++ b/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<params>
-The current Parameters of Bugzilla, as a hashref. If C<data/params>
-does not exist, then we return an empty hashref. If C<data/params>
-is unreadable or is not valid perl, we C<die>.
+The current Parameters of Bugzilla, as a hashref. If C<data/params.js>
+does not exist, then we return an empty hashref. If C<data/params.js>
+is unreadable or is not valid, we C<die>.
=item C<local_timezone>
diff --git a/Bugzilla/Config.pm b/Bugzilla/Config.pm
index 04c2c0c6f..a41b30abe 100644
--- a/Bugzilla/Config.pm
+++ b/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'
-
- # 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;
-
- $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')};
+ my $file = bz_locations()->{'datadir'} . '/params.js';
+
+ if (-e $file) {
+ my $data;
+ read_file($file, binmode => ':utf8', buf_ref => \$data);
+
+ # 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 C<Bugzilla->params>. Used only by
+ instead of C<Bugzilla-E<gt>params>. Used only by
C<update_params>.
Returns: nothing
@@ -383,12 +401,12 @@ Returns: nothing
=item C<read_param_file()>
Description: Most callers should never need this. This is used
- by C<Bugzilla->params> to directly read C<$datadir/params>
- and load it into memory. Use C<Bugzilla->params> instead.
+ by C<Bugzilla-E<gt>params> to directly read C<$datadir/params.js>
+ and load it into memory. Use C<Bugzilla-E<gt>params> 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<param_panels()>
diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm
index 0b0603970..2bf3c0c1a 100644
--- a/Bugzilla/Install/DB.pm
+++ b/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/Bugzilla/Install/Filesystem.pm b/Bugzilla/Install/Filesystem.pm
index 157a8429e..73536b4b5 100644
--- a/Bugzilla/Install/Filesystem.pm
+++ b/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/Bugzilla/Install/Requirements.pm b/Bugzilla/Install/Requirements.pm
index 2ceb01cfd..bec753830 100644
--- a/Bugzilla/Install/Requirements.pm
+++ b/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) {
@@ -299,13 +305,6 @@ sub OPTIONAL_MODULES {
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',
# 1.06 no longer throws warnings with Perl 5.10+.
diff --git a/checksetup.pl b/checksetup.pl
index c7e8994e5..1785faf54 100755
--- a/checksetup.pl
+++ b/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<Bugzilla::Install::Filesystem/create_htaccess>.
=item 9
-Updates the system parameters (stored in F<data/params>), using
+Updates the system parameters (stored in F<data/params.js>), using
L<Bugzilla::Config/update_params>.
=item 10
diff --git a/docs/en/rst/administration.rst b/docs/en/rst/administration.rst
index 81dc35107..d26e354b9 100644
--- a/docs/en/rst/administration.rst
+++ b/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/template/en/default/setup/strings.txt.pl b/template/en/default/setup/strings.txt.pl
index dc6a52fe6..6fb343713 100644
--- a/template/en/default/setup/strings.txt.pl
+++ b/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',