195686 - a valid ctype that doesn't have a template defined dumps default template with wrong content type

Status: RESOLVED FIXED

(Bugzilla :: User Interface, defect)

Description

[Dave Miller [:justdave]]

See the URL.

If you send a ctype param indicating a content-type that we consider legal, and
there's no template defined for that content type, we dump the default template.
 This may or may not be the right thing to do, but what's worse, is we send the
content-type header for the type the user requested, even though the template
that's getting used is a different content-type.  That could be dangerous...

Comment 1

[Gervase Markham [:gerv]]

This is somewhat difficult to fix, because we only know if a template exists
_after_ we have printed the content type, because that's when we invoke the
Template Toolkit to go and look for it along its search path.

I don't see this as dangerous; the problem is limited to printing HTML as a
non-HTML content type. The possible content-types it could arrive as are (from
localconfig):

$contenttypes = {
  "html" => "text/html" , 
   "rdf" => "application/xml" , 
   "xml" => "text/xml" , 
   "js" => "application/x-javascript" , 
   "png" => "image/png" , 
};
and also text/plain, which is used for CSV (not sure why it's not in my list)
and is the default. I don't see HTML being mistakenly served as one of the above
being a risk.

What we could do is remember what content-type we printed, and then print an
appropriate error message in ThrowTemplateError(). bbaetz: didn't you have some
plan to encapsulate the printing of the content-type in Bugzilla::Template,
thereby eliminating header_done, allowing the sending of a charset, and other
fun stuff?

Gerv

Comment 2

[Dave Miller [:justdave]]

Isn't the ctype determination being done by Bugzilla before it gets sent to
Template Toolkit?  Otherwise how would it know to fall back on the default one?

Comment 3

[Gervase Markham [:gerv]]

Bugzilla translates ".html." to text/html, yes. But it has no idea whether the
relevant template exists (although see bbaetz's comments about
template->provider in the bug 195695.) 

Gerv

Comment 4

[Bradley Baetz (:bbaetz)]

There used to be code to walk the directory structure. That got taken out a
while back; hence this bug and bug 195695

Comment 5

[Dave Miller [:justdave]]

ok, but the point is, if you asked it for csv and there isn't a
template.csv.tmpl, how does it know to use template.html.tmpl instead?

Comment 6

[Bradley Baetz (:bbaetz)]

It doesn't. I don't see that on the url - I just get the generic 
Template::Exception error (Can you please update landfill's TT install to the 
devel version, btw?)

Comment 7

[Bradley Baetz (:bbaetz)]

Not that I could repro all of that, but fixed by bug 195695, I belive