Bug 1301887 - File::Slurp triggers warnings on perl 5.24 and it is recommended to not use it (#21)

r=mtyson

8 files changed, 39 insertions, 23 deletions

diff --git a/Bugzilla/Config.pm b/Bugzilla/Config.pm
index 49918d5c1..8bd774b70 100644
--- a/Bugzilla/Config.pm
+++ b/Bugzilla/Config.pm
@@ -17,10 +17,9 @@ use autodie qw(:default);
 use Bugzilla::Constants;
 use Bugzilla::Hook;
 use Bugzilla::Install::Util qw(i_am_persistent);
-use Bugzilla::Util qw(trick_taint);
+use Bugzilla::Util qw(trick_taint read_text write_text);
 
 use JSON::XS;
-use File::Slurp;
 use File::Temp;
 use File::Basename;
 
@@ -301,7 +300,7 @@ sub write_params {
     my $param_file = bz_locations()->{'datadir'} . '/params.json';
 
     my $json_data = JSON::XS->new->canonical->pretty->encode($param_data);
-    write_file($param_file, { binmode => ':utf8', atomic => 1 }, \$json_data);
+    write_text($param_file, $json_data);
 
     # It's not common to edit parameters and loading
     # Bugzilla::Install::Filesystem is slow.
@@ -318,8 +317,7 @@ sub read_param_file {
     my $file = bz_locations()->{'datadir'} . '/params.json';
 
     if (-e $file) {
-        my $data;
-        read_file($file, binmode => ':utf8', buf_ref => \$data);
+        my $data = read_text($file);
         trick_taint($data);
 
         # If params.json has been manually edited and e.g. some quotes are
diff --git a/Bugzilla/Install/Filesystem.pm b/Bugzilla/Install/Filesystem.pm
index feb150220..4c08cc538 100644
--- a/Bugzilla/Install/Filesystem.pm
+++ b/Bugzilla/Install/Filesystem.pm
@@ -31,7 +31,6 @@ use File::Path;
 use File::Basename;
 use File::Copy qw(move);
 use File::Spec;
-use File::Slurp;
 use IO::File;
 use POSIX ();
 
@@ -541,7 +540,7 @@ sub update_filesystem {
 
     # Remove old assets htaccess file to force recreation with correct values.
     if (-e "$assetsdir/.htaccess") {
-        if (read_file("$assetsdir/.htaccess") =~ /<FilesMatch \\\.css\$>/) {
+        if (read_text("$assetsdir/.htaccess") =~ /<FilesMatch \\\.css\$>/) {
             unlink("$assetsdir/.htaccess");
         }
     }
diff --git a/Bugzilla/JobQueue.pm b/Bugzilla/JobQueue.pm
index 0a5bcb116..66aba68d7 100644
--- a/Bugzilla/JobQueue.pm
+++ b/Bugzilla/JobQueue.pm
@@ -14,8 +14,8 @@ use warnings;
 use Bugzilla::Constants;
 use Bugzilla::Error;
 use Bugzilla::Install::Util qw(install_string);
+use Bugzilla::Util qw(read_text);
 use File::Basename;
-use File::Slurp;
 use base qw(TheSchwartz);
 use fields qw(_worker_pidfile);
 
@@ -124,7 +124,7 @@ sub subprocess_worker {
             # And poll the PID to detect when the working has finished.
             # We do this instead of system() to allow for the INT signal to
             # interrup us and trigger kill_worker().
-            my $pid = read_file($self->{_worker_pidfile}, err_mode => 'quiet');
+            my $pid = read_text($self->{_worker_pidfile}, err_mode => 'quiet');
             if ($pid) {
                 sleep(3) while(kill(0, $pid));
             }
@@ -139,7 +139,7 @@ sub subprocess_worker {
 sub kill_worker {
     my $self = Bugzilla->job_queue();
     if ($self->{_worker_pidfile} && -e $self->{_worker_pidfile}) {
-        my $worker_pid = read_file($self->{_worker_pidfile});
+        my $worker_pid = read_text($self->{_worker_pidfile});
         if ($worker_pid && kill(0, $worker_pid)) {
             $self->debug("Stopping worker process");
             system "$0 -f -p '" . $self->{_worker_pidfile} . "' stop";
diff --git a/Bugzilla/Template.pm b/Bugzilla/Template.pm
index 8871d810d..ce0276751 100644
--- a/Bugzilla/Template.pm
+++ b/Bugzilla/Template.pm
@@ -32,7 +32,6 @@ use Digest::MD5 qw(md5_hex);
 use File::Basename qw(basename dirname);
 use File::Find;
 use File::Path qw(rmtree mkpath);
-use File::Slurp;
 use File::Spec;
 use IO::Dir;
 use List::MoreUtils qw(firstidx);
@@ -505,7 +504,7 @@ sub _concatenate_css {
         next unless -e "$cgi_path/$files{$source}";
         my $file = $skins_path . '/' . md5_hex($source) . '.css';
         if (!-e $file) {
-            my $content = read_file("$cgi_path/$files{$source}");
+            my $content = read_text("$cgi_path/$files{$source}");
 
             # minify
             $content =~ s{/\*.*?\*/}{}sg;   # comments
@@ -515,7 +514,7 @@ sub _concatenate_css {
             # rewrite urls
             $content =~ s{url\(([^\)]+)\)}{_css_url_rewrite($source, $1)}eig;
 
-            write_file($file, "/* $files{$source} */\n" . $content . "\n");
+            write_text($file, "/* $files{$source} */\n" . $content . "\n");
         }
         push @minified, $file;
     }
@@ -525,9 +524,9 @@ sub _concatenate_css {
     if (!-e $file) {
         my $content = '';
         foreach my $source (@minified) {
-            $content .= read_file($source);
+            $content .= read_text($source);
         }
-        write_file($file, $content);
+        write_text($file, $content);
     }
 
     $file =~ s/^\Q$cgi_path\E\///o;
@@ -566,7 +565,7 @@ sub _concatenate_js {
         next unless -e "$cgi_path/$files{$source}";
         my $file = $skins_path . '/' . md5_hex($source) . '.js';
         if (!-e $file) {
-            my $content = read_file("$cgi_path/$files{$source}");
+            my $content = read_text("$cgi_path/$files{$source}");
 
             # minimal minification
             $content =~ s#/\*.*?\*/##sg;    # block comments
@@ -575,7 +574,7 @@ sub _concatenate_js {
             $content =~ s#\n{2,}#\n#g;      # blank lines
             $content =~ s#(^\s+|\s+$)##g;   # whitespace at the start/end of file
 
-            write_file($file, ";/* $files{$source} */\n" . $content . "\n");
+            write_text($file, ";/* $files{$source} */\n" . $content . "\n");
         }
         push @minified, $file;
     }
@@ -585,9 +584,9 @@ sub _concatenate_js {
     if (!-e $file) {
         my $content = '';
         foreach my $source (@minified) {
-            $content .= read_file($source);
+            $content .= read_text($source);
         }
-        write_file($file, $content);
+        write_text($file, $content);
     }
 
     $file =~ s/^\Q$cgi_path\E\///o;
diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm
index 78376c6fe..565cda19d 100644
--- a/Bugzilla/Util.pm
+++ b/Bugzilla/Util.pm
@@ -24,7 +24,7 @@ use parent qw(Exporter);
                              validate_email_syntax check_email_syntax clean_text
                              get_text template_var display_value disable_utf8
                              detect_encoding email_filter
-                             join_activity_entries);
+                             join_activity_entries read_text write_text);
 
 use Bugzilla::Constants;
 use Bugzilla::RNG qw(irand);
@@ -39,6 +39,8 @@ use Scalar::Util qw(tainted blessed);
 use Text::Wrap;
 use Encode qw(encode decode resolve_alias);
 use Encode::Guess;
+use File::Basename qw(dirname);
+use File::Temp qw(tempfile);
 
 sub trick_taint {
     require Carp;
@@ -101,6 +103,27 @@ sub html_quote {
     return $var;
 }
 
+sub read_text {
+    my ($filename) = @_;
+    open my $fh, '<:encoding(utf-8)', $filename;
+    local $/ = undef;
+    my $content = <$fh>;
+    close $fh;
+    return $content;
+}
+
+sub write_text {
+    my ($filename, $content) = @_;
+    my ($tmp_fh, $tmp_filename) = tempfile('.tmp.XXXXXXXXXX',
+        DIR    => dirname($filename),
+        UNLINK => 0,
+    );
+    binmode $tmp_fh, ':encoding(utf-8)';
+    print $tmp_fh $content;
+    close $tmp_fh;
+    rename $tmp_filename, $filename;
+}
+
 sub html_light_quote {
     my ($text) = @_;
     # admin/table.html.tmpl calls |FILTER html_light| many times.
diff --git a/META.json b/META.json
index b254c100d..29b0c35a5 100644
--- a/META.json
+++ b/META.json
@@ -370,7 +370,6 @@
             "Digest::SHA" : "0",
             "Email::MIME" : "1.904",
             "Email::Sender" : "1.300011",
-            "File::Slurp" : "9999.13",
             "HTTP::Request" : "0",
             "HTTP::Response" : "0",
             "JSON::XS" : "2.01",
diff --git a/META.yml b/META.yml
index eeb5ab800..a1647627b 100644
--- a/META.yml
+++ b/META.yml
@@ -179,7 +179,6 @@ requires:
   Digest::SHA: '0'
   Email::MIME: '1.904'
   Email::Sender: '1.300011'
-  File::Slurp: '9999.13'
   HTTP::Request: '0'
   HTTP::Response: '0'
   JSON::XS: '2.01'
diff --git a/Makefile.PL b/Makefile.PL
index ca3176202..325fed4d1 100644
--- a/Makefile.PL
+++ b/Makefile.PL
@@ -55,7 +55,6 @@ my %requires = (
     'Digest::SHA'         => 0,
     'Email::MIME'         => '1.904',
     'Email::Sender'       => '1.300011',
-    'File::Slurp'         => '9999.13',
     'HTTP::Request'       => 0,
     'HTTP::Response'      => 0,
     'JSON::XS'            => '2.01',