Bug 146495 - clean up format ambiguities. We now have separate "format" and "ctype" parameters. Also fixes bug 140513, bug 143604, and bug 148133. Patch by gerv; r=myk.

6 files changed, 33 insertions, 125 deletions

diff --git a/buglist.cgi b/buglist.cgi
index 74e436aee..eeede9944 100755
--- a/buglist.cgi
+++ b/buglist.cgi
@@ -89,7 +89,7 @@ else {
 # Determine the format in which the user would like to receive the output.
 # Uses the default format if the user did not specify an output format;
 # otherwise validates the user's choice against the list of available formats.
-my $format = ValidateOutputFormat($::FORM{'format'}, "list");
+my $format = GetFormat("list/list", $::FORM{'format'}, $::FORM{'ctype'});
 
 # Use server push to display a "Please wait..." message for the user while
 # executing their query if their browser supports it and they are viewing
@@ -764,7 +764,7 @@ if ($format->{'extension'} eq "html") {
     }
 }
 else {
-    print "Content-Type: $format->{'contenttype'}\n";
+    print "Content-Type: $format->{'ctype'}\n";
 }
 
 print "\n"; # end HTTP headers
@@ -775,7 +775,7 @@ print "\n"; # end HTTP headers
 ################################################################################
 
 # Generate and return the UI (HTML page) from the appropriate template.
-$template->process("list/$format->{'template'}", $vars)
+$template->process($format->{'template'}, $vars)
   || ThrowTemplateError($template->error());
 
 
diff --git a/duplicates.cgi b/duplicates.cgi
index 5196027ac..3eeab3fb5 100755
--- a/duplicates.cgi
+++ b/duplicates.cgi
@@ -202,12 +202,13 @@ $vars->{'product'} = $product;
 $vars->{'products'} = \@::legal_product;
 
 
-my $format = ValidateOutputFormat($::FORM{'format'}, "duplicates", "reports");
+my $format = 
+  GetFormat("reports/duplicates", $::FORM{'format'}, $::FORM{'ctype'});
  
-print "Content-Type: $format->{'contenttype'}\n\n";
+print "Content-Type: $format->{'ctype'}\n\n";
 
 # Generate and return the UI (HTML page) from the appropriate template.
-$template->process("reports/$format->{'template'}", $vars)
+$template->process($format->{'template'}, $vars)
   || ThrowTemplateError($template->error());
 
 
diff --git a/enter_bug.cgi b/enter_bug.cgi
index 7aa6dfc66..a779aa85d 100755
--- a/enter_bug.cgi
+++ b/enter_bug.cgi
@@ -382,8 +382,9 @@ if ($::usergroupset ne '0') {
 
 $vars->{'default'} = \%default;
 
-my $format = ValidateOutputFormat($::FORM{'format'}, "create", "bug/create");
+my $format = 
+  GetFormat("bug/create/create", $::FORM{'format'}, $::FORM{'ctype'});
 
-print "Content-type: $format->{'contenttype'}\n\n";
-$template->process("bug/create/$format->{'template'}", $vars)
+print "Content-type: $format->{'ctype'}\n\n";
+$template->process($format->{'template'}, $vars)
   || ThrowTemplateError($template->error());          
diff --git a/globals.pl b/globals.pl
index 5931d186d..29582e153 100644
--- a/globals.pl
+++ b/globals.pl
@@ -1515,119 +1515,27 @@ $Template::Stash::SCALAR_OPS->{ truncate } =
     
 ###############################################################################
 
-sub GetOutputFormats {
-    # Builds a set of possible output formats for a script by looking for
-    # format files in the appropriate template directories as specified by 
-    # the template include path, the sub-directory parameter, and the
-    # template name parameter.
+# Constructs a format object from URL parameters. You most commonly call it 
+# like this:
+# my $format = GetFormat("foo/bar", $::FORM{'format'}, $::FORM{'ctype'});
+sub GetFormat {
+    my ($template, $format, $ctype) = @_;
     
-    # This function is relevant for scripts with one basic function whose
-    # results can be represented in multiple formats, f.e. buglist.cgi, 
-    # which has one function (query and display of a list of bugs) that can 
-    # be represented in multiple formats (i.e. html, rdf, xml, etc.).
+    $ctype ||= "html";
     
-    # It is *not* relevant for scripts with several functions but only one
-    # basic output format, f.e. editattachstatuses.cgi, which not only lists 
-    # statuses but also provides adding, editing, and deleting functions.
-    # (although it may be possible to make this function applicable under 
-    # these circumstances with minimal modification).
+    # Security - allow letters and a hyphen only
+    $ctype =~ s/[^a-zA-Z\-]//g;
+    $format =~ s/[^a-zA-Z\-]//g;
     
-    # Format files have names that look like SCRIPT-FORMAT.EXT.tmpl, where
-    # SCRIPT is the name of the CGI script being invoked, SUBDIR is the name 
-    # of the template sub-directory, FORMAT is the name of the format, and EXT 
-    # is the filename extension identifying the content type of the output.
-     
-    # When a format file is found, a record for that format is added to
-    # the hash of format records, indexed by format name, with each record
-    # containing the name of the format file, its filename extension,
-    # and its content type (obtained by reference to the $::contenttypes
-    # hash defined in localconfig).
-    
-    my ($subdir, $script) = @_;
-
-    # A set of output format records, indexed by format name, each record 
-    # containing template, extension, and contenttype fields.
-    my $formats = {};
-    
-    # Get the template include path from the template object.
-    my $includepath = $::template->context->{ LOAD_TEMPLATES }->[0]->include_path();
-    
-    # Loop over each include directory in reverse so that format files
-    # earlier in the path override files with the same name later in
-    # the path (i.e. "custom" formats override "default" ones).
-    foreach my $path (reverse @$includepath) {
-        # Get the list of files in the given sub-directory if it exists.
-        my $dirname = File::Spec->catdir($path, $subdir);
-        opendir(SUBDIR, $dirname) || next;
-        my @files = readdir SUBDIR;
-        closedir SUBDIR;
+    $template .= ($format ? "-$format" : "");
+    $template .= ".$ctype.tmpl";
         
-        # Loop over each file in the sub-directory looking for format files
-        # (files whose name looks like SCRIPT-FORMAT.EXT.tmpl).
-        foreach my $file (@files) {
-            if ($file =~ /^\Q$script\E-(.+)\.(.+)\.tmpl$/) {
-                # This must be a valid file
-                # If an attacker could add a previously unused format
-                # type to trick us into running it, then they could just
-                # change an existing one...
-                # (This implies that running without a webservergroup is
-                # insecure, but that is the case anyway)
-                trick_taint($file);
-
-                $formats->{$1} = { 
-                  'template'    => $file , 
-                  'extension'   => $2 , 
-                  'contenttype' => $::contenttypes->{$2} || "text/plain" , 
-                };
-            }
-        }
-    }
-    return $formats;
-}
-
-sub ValidateOutputFormat {
-    my ($format, $script, $subdir) = @_;
-    
-    # If the script name is undefined, assume the script currently being
-    # executed, deriving its name from Perl's built-in $0 (program name) var.
-    if (!defined($script)) {
-        my ($volume, $dirs, $filename) = File::Spec->splitpath($0);
-        $filename =~ /^(.+)\.cgi$/;
-        $script = $1
-          || DisplayError("Could not determine the name of the script.")
-          && exit;
-    }
-    
-    # If the format name is undefined or the default format is specified,
-    # do not do any validation but instead return the default format.
-    if (!defined($format) || $format eq "default") {
-        return 
-          { 
-            'template'    => "$script.html.tmpl" , 
-            'extension'   => "html" , 
-            'contenttype' => "text/html" , 
-          };
-    }
-    
-    # If the subdirectory name is undefined, assume the script name.
-    $subdir = $script if !defined($subdir);
-    
-    # Get the list of output formats supported by this script.
-    my $formats = GetOutputFormats($subdir, $script);
-    
-    # Validate the output format requested by the user.
-    if (!$formats->{$format}) {
-        my $escapedname = html_quote($format);
-        DisplayError("The <em>$escapedname</em> output format is not 
-          supported by this script.  Supported formats (besides the 
-          default HTML format) are <em>" . 
-          join("</em>, <em>", map(html_quote($_), keys(%$formats))) . 
-          "</em>.");
-        exit;
-    }
-    
-    # Return the validated output format.
-    return $formats->{$format};
+    return 
+    { 
+        'template'    => $template , 
+        'extension'   => $ctype , 
+        'ctype' => $::contenttypes->{$ctype} || "text/plain" , 
+    };
 }
 
 ###############################################################################
diff --git a/post_bug.cgi b/post_bug.cgi
index 8d21f1505..430ae37a8 100755
--- a/post_bug.cgi
+++ b/post_bug.cgi
@@ -59,11 +59,9 @@ my $comment;
 
 $vars->{'form'} = \%::FORM;
 
-# We can't use ValidateOutputFormat here because it defaults to HTML.
-my $template_name = "bug/create/comment";
-$template_name .= ($::FORM{'format'} ? "-$::FORM{'format'}" : "");
+my $format = GetFormat("bug/create/comment", $::FORM{'format'}, "txt");
 
-$template->process("$template_name.txt.tmpl", $vars, \$comment)
+$template->process($format->{'template'}, $vars, \$comment)
   || ThrowTemplateError($template->error());
 
 ValidateComment($comment);
diff --git a/query.cgi b/query.cgi
index 2ce900767..89c63073b 100755
--- a/query.cgi
+++ b/query.cgi
@@ -363,7 +363,7 @@ $default{'querytype'} = $deforder || 'Importance';
 $vars->{'default'} = \%default;
 
 # Generate and return the UI (HTML page) from the appropriate template.
-my $format = ValidateOutputFormat($::FORM{'format'}, "search");
-print "Content-type: text/html\n\n";
-$template->process("search/$format->{'template'}", $vars)
+my $format = GetFormat("search/search", $::FORM{'format'}, $::FORM{'ctype'});
+print "Content-Type: $format->{'ctype'}\n\n";
+$template->process($format->{'template'}, $vars)
   || ThrowTemplateError($template->error());