diff options
| -rw-r--r-- | Bugzilla/Constants.pm | 1 | ||||
| -rw-r--r-- | Bugzilla/Install/Filesystem.pm | 19 | ||||
| -rwxr-xr-x | collectstats.pl | 7 | ||||
| -rwxr-xr-x | reports.cgi | 122 | ||||
| -rw-r--r-- | template/en/default/global/user-error.html.tmpl | 2 | ||||
| -rw-r--r-- | template/en/default/reports/old-charts.html.tmpl | 2 |
6 files changed, 75 insertions, 78 deletions
diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm index 4ca04c478..9a0f6cd2e 100644 --- a/Bugzilla/Constants.pm +++ b/Bugzilla/Constants.pm @@ -623,6 +623,7 @@ sub bz_locations { 'datadir' => $datadir, 'attachdir' => "$datadir/attachments", 'skinsdir' => "$libpath/skins", + 'graphsdir' => "$libpath/graphs", # $webdotdir must be in the web server's tree somewhere. Even if you use a # local dot, we output images to there. Also, if $webdotdir is # not relative to the bugzilla root directory, you'll need to diff --git a/Bugzilla/Install/Filesystem.pm b/Bugzilla/Install/Filesystem.pm index 42f292b6d..0bad63708 100644 --- a/Bugzilla/Install/Filesystem.pm +++ b/Bugzilla/Install/Filesystem.pm @@ -122,6 +122,7 @@ sub FILESYSTEM { my $skinsdir = bz_locations()->{'skinsdir'}; my $localconfig = bz_locations()->{'localconfig'}; my $template_cache = bz_locations()->{'template_cache'}; + my $graphsdir = bz_locations()->{'graphsdir'}; # We want to set the permissions the same for all localconfig files # across all PROJECTs, so we do something special with $localconfig, @@ -197,7 +198,7 @@ sub FILESYSTEM { dirs => DIR_CGI_WRITE }, $webdotdir => { files => WS_SERVE, dirs => DIR_CGI_WRITE | DIR_ALSO_WS_SERVE }, - graphs => { files => WS_SERVE, + $graphsdir => { files => WS_SERVE, dirs => DIR_CGI_WRITE | DIR_ALSO_WS_SERVE }, "$datadir/db" => { files => CGI_WRITE, dirs => DIR_CGI_WRITE }, @@ -269,7 +270,7 @@ sub FILESYSTEM { # Directories that cgi scripts can write to. "$datadir/db" => DIR_CGI_WRITE, $attachdir => DIR_CGI_WRITE, - graphs => DIR_CGI_WRITE | DIR_ALSO_WS_SERVE, + $graphsdir => DIR_CGI_WRITE | DIR_ALSO_WS_SERVE, $webdotdir => DIR_CGI_WRITE | DIR_ALSO_WS_SERVE, # Directories that contain content served directly by the web server. "$skinsdir/custom" => DIR_WS_SERVE, @@ -331,6 +332,17 @@ EOT "$datadir/.htaccess" => { perms => WS_SERVE, contents => HT_DEFAULT_DENY }, + "$graphsdir/.htaccess" => { perms => WS_SERVE, contents => <<EOT +# Allow access to .png and .gif files. +<FilesMatch (\\.gif|\\.png)\$> + Allow from all +</FilesMatch> + +# And no directory listings, either. +Deny from all +EOT + }, + "$webdotdir/.htaccess" => { perms => WS_SERVE, contents => <<EOT # Restrict access to .dot files to the public webdot server at research.att.com # if research.att.com ever changes their IP, or if you use a different @@ -373,10 +385,11 @@ sub update_filesystem { my %files = %{$fs->{create_files}}; my $datadir = bz_locations->{'datadir'}; + my $graphsdir = bz_locations->{'graphsdir'}; # If the graphs/ directory doesn't exist, we're upgrading from # a version old enough that we need to update the $datadir/mining # format. - if (-d "$datadir/mining" && !-d 'graphs') { + if (-d "$datadir/mining" && !-d $graphsdir) { _update_old_charts($datadir); } diff --git a/collectstats.pl b/collectstats.pl index f5ba2ddab..f8ee89215 100755 --- a/collectstats.pl +++ b/collectstats.pl @@ -49,9 +49,12 @@ use Bugzilla::Field; # in the regenerate mode). $| = 1; +my $datadir = bz_locations()->{'datadir'}; +my $graphsdir = bz_locations()->{'graphsdir'}; + # Tidy up after graphing module my $cwd = Cwd::getcwd(); -if (chdir("graphs")) { +if (chdir($graphsdir)) { unlink <./*.gif>; unlink <./*.png>; # chdir("..") doesn't work if graphs is a symlink, see bug 429378 @@ -68,8 +71,6 @@ if ($#ARGV >= 0 && $ARGV[0] eq "--regenerate") { $regenerate = 1; } -my $datadir = bz_locations()->{'datadir'}; - # As we can now customize statuses and resolutions, looking at the current list # of legal values only is not enough as some now removed statuses and resolutions # may have existed in the past, or have been renamed. We want them all. diff --git a/reports.cgi b/reports.cgi index fd84f5330..01bbb50c7 100755 --- a/reports.cgi +++ b/reports.cgi @@ -38,31 +38,28 @@ use Bugzilla::Util; use Bugzilla::Error; use Bugzilla::Status; +use File::Basename; +use Digest::MD5 qw(md5_hex); + # If we're using bug groups for products, we should apply those restrictions # to viewing reports, as well. Time to check the login in that case. my $user = Bugzilla->login(); +my $cgi = Bugzilla->cgi; +my $template = Bugzilla->template; +my $vars = {}; if (!Bugzilla->feature('old_charts')) { ThrowCodeError('feature_disabled', { feature => 'old_charts' }); } my $dir = bz_locations()->{'datadir'} . "/mining"; -my $graph_url = 'graphs'; -my $graph_dir = bz_locations()->{'libpath'} . '/' .$graph_url; +my $graph_dir = bz_locations()->{'graphsdir'}; +my $graph_url = basename($graph_dir); +my $product_name = $cgi->param('product') || ''; Bugzilla->switch_to_shadow_db(); -my $cgi = Bugzilla->cgi; -my $template = Bugzilla->template; -my $vars = {}; - -# We only want those products that the user has permissions for. -my @myproducts; -push( @myproducts, "-All-"); -# Extract product names from objects and add them to the list. -push( @myproducts, map { $_->name } @{$user->get_selectable_products} ); - -if (! defined $cgi->param('product')) { +if (!$product_name) { # Can we do bug charts? (-d $dir && -d $graph_dir) || ThrowCodeError('chart_dir_nonexistent', @@ -80,49 +77,61 @@ if (! defined $cgi->param('product')) { push(@datasets, $datasets); } + # We only want those products that the user has permissions for. + my @myproducts = ('-All-'); + # Extract product names from objects and add them to the list. + push( @myproducts, map { $_->name } @{$user->get_selectable_products} ); + $vars->{'datasets'} = \@datasets; $vars->{'products'} = \@myproducts; print $cgi->header(); - - $template->process('reports/old-charts.html.tmpl', $vars) - || ThrowTemplateError($template->error()); - exit; } else { - my $product = $cgi->param('product'); - # For security and correctness, validate the value of the "product" form variable. # Valid values are those products for which the user has permissions which appear # in the "product" drop-down menu on the report generation form. - grep($_ eq $product, @myproducts) - || ThrowUserError("invalid_product_name", {product => $product}); + my ($product) = grep { $_->name eq $product_name } @{$user->get_selectable_products}; + ($product || $product_name eq '-All-') + || ThrowUserError('invalid_product_name', {product => $product_name}); - # We've checked that the product exists, and that the user can see it - # This means that is OK to detaint - trick_taint($product); + # Product names can change over time. Their ID cannot; so use the ID + # to generate the filename. + my $prod_id = $product ? $product->id : 0; - defined($cgi->param('datasets')) || ThrowUserError('missing_datasets'); + # Make sure there is something to plot. + my @datasets = $cgi->param('datasets'); + scalar(@datasets) || ThrowUserError('missing_datasets'); - my $datasets = join('', $cgi->param('datasets')); + if (grep { $_ !~ /^[A-Za-z0-9:_-]+$/ } @datasets) { + ThrowUserError('invalid_datasets', {'datasets' => \@datasets}); + } - my $data_file = daily_stats_filename($product); - my $image_file = chart_image_name($data_file, $datasets); - my $url_image = correct_urlbase() . "$graph_url/$image_file"; + # Filenames must not be guessable as they can point to products + # you are not allowed to see. Also, different projects can have + # the same product names. + my $key = Bugzilla->localconfig->{'site_wide_secret'}; + my $project = bz_locations()->{'project'} || ''; + my $image_file = join(':', ($key, $project, $prod_id, @datasets)); + # Wide characters cause md5_hex() to die. + if (Bugzilla->params->{'utf8'}) { + utf8::encode($image_file) if utf8::is_utf8($image_file); + } + $image_file = md5_hex($image_file) . '.png'; + trick_taint($image_file); if (! -e "$graph_dir/$image_file") { - generate_chart("$dir/$data_file", "$graph_dir/$image_file", $product, $datasets); + generate_chart($dir, "$graph_dir/$image_file", $product, \@datasets); } - $vars->{'url_image'} = $url_image; + $vars->{'url_image'} = "$graph_url/$image_file"; print $cgi->header(-Content_Disposition=>'inline; filename=bugzilla_report.html'); - - $template->process('reports/old-charts.html.tmpl', $vars) - || ThrowTemplateError($template->error()); - exit; } +$template->process('reports/old-charts.html.tmpl', $vars) + || ThrowTemplateError($template->error()); + ##################### # Subroutines # ##################### @@ -131,9 +140,8 @@ sub get_data { my $dir = shift; my @datasets; - my $datafile = daily_stats_filename('-All-'); - open(DATA, '<', "$dir/$datafile") - || ThrowCodeError('chart_file_open_fail', {filename => "$dir/$datafile"}); + open(DATA, '<', "$dir/-All-") + || ThrowCodeError('chart_file_open_fail', {filename => "$dir/-All-"}); while (<DATA>) { if (/^# fields?: (.+)\s*$/) { @@ -145,38 +153,12 @@ sub get_data { return @datasets; } -sub daily_stats_filename { - my ($prodname) = @_; - $prodname =~ s/\//-/gs; - return $prodname; -} - -sub chart_image_name { - my ($data_file, $datasets) = @_; - - # This routine generates a filename from the requested fields. The problem - # is that we have to check the safety of doing this. We can't just require - # that the fields exist, because what stats were collected could change - # over time (eg by changing the resolutions available) - # Instead, just require that each field name consists only of letters, - # numbers, underscores and hyphens. - - if ($datasets !~ m/^[A-Za-z0-9:_-]+$/) { - ThrowUserError('invalid_datasets', {'datasets' => $datasets}); - } - - # Since we pass the tests, consider it OK - trick_taint($datasets); - - # Cache charts by generating a unique filename based on what they - # show. Charts should be deleted by collectstats.pl nightly. - my $id = join ("_", split (":", $datasets)); - - return "${data_file}_${id}.png"; -} - sub generate_chart { - my ($data_file, $image_file, $product, $datasets) = @_; + my ($dir, $image_file, $product, $datasets) = @_; + $product = $product ? $product->name : '-All-'; + my $data_file = $product; + $data_file =~ s/\//-/gs; + $data_file = $dir . '/' . $data_file; if (! open FILE, $data_file) { if ($product eq '-All-') { @@ -187,7 +169,7 @@ sub generate_chart { my @fields; my @labels = qw(DATE); - my %datasets = map { $_ => 1 } split /:/, $datasets; + my %datasets = map { $_ => 1 } @$datasets; my %data = (); while (<FILE>) { diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl index a734435a1..5aa8955fd 100644 --- a/template/en/default/global/user-error.html.tmpl +++ b/template/en/default/global/user-error.html.tmpl @@ -907,7 +907,7 @@ [% ELSIF error == "invalid_datasets" %] [% title = "Invalid Datasets" %] - Invalid datasets <em>[% datasets FILTER html %]</em>. Only digits, + Invalid datasets <em>[% datasets.join(":") FILTER html %]</em>. Only digits, letters and colons are allowed. [% ELSIF error == "invalid_format" %] diff --git a/template/en/default/reports/old-charts.html.tmpl b/template/en/default/reports/old-charts.html.tmpl index ca3ba6c7d..4bdc0cffa 100644 --- a/template/en/default/reports/old-charts.html.tmpl +++ b/template/en/default/reports/old-charts.html.tmpl @@ -51,7 +51,7 @@ [%# We cannot use translated statuses and resolutions from field-descs.none.html # because old charts do not distinguish statuses from resolutions. %] [% FOREACH dataset = datasets %] - <option value="[% dataset.value FILTER html %]:" + <option value="[% dataset.value FILTER html %]" [% " selected=\"selected\"" IF dataset.selected %]> [% dataset.value FILTER html %]</option> [% END %] |