265731 - multipart_start in Bugzilla::CGI does not honor $cgi->charset in the Content-Type header it produces

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, defect)

Description

[Dave Miller [:justdave]]

multipart_start in Bugzilla::CGI does not honor $cgi->charset in the
Content-Type header it produces.  This results in the section always being
interpreted as ISO-8859-1, no matter what charset it really is.

This breaks buglist display on sites that override the charset defined in the
$self->charset() call at the top of Bugzilla/CGI.pm.

Steps to reproduce:

1) Set the charset to UTF-8 (change the existing $self->charset call to
$self->charset("UTF-8") near the top of Bugzilla/CGI.pm)
2) Submit a new bug that contains non-latin1 characters in the bug summary
3) Run a query that returns that bug in the results.

Observe that the non-latin1 characters are now garbled in the query output.

Comment 1

[Dave Miller [:justdave]]

Attachment: Patch v1

This adds the charset information to Content-Type header if available.	Code in
question swiped from the header() function in Perl's CGI.pm.  Tested and works
on https://bugzilla.ubuntu.com/

Comment 2

[Dave Miller [:justdave]]

Let's go ahead and get this in 2.18, since it's something folks are likely to
customize.

Comment 3

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 163118 [details] [diff] [review]
Patch v1

>Index: Bugzilla/CGI.pm

>+    my $charset = $self->charset;
>     $type = $type || 'text/html';
>+    $type .= "; charset=$charset" if $type ne '' and $type =~ m!^text/! and $type !~ /\bcharset\b/ and $charset ne '';

Nits:

this would read easier with some line wrapping (perhaps putting each
conditional on its own line);

$type will never eq '' since the preceding line will populate an empty string,
so "$type ne ''" doesn't need to be checked;

/\bcharset\b/ would be more robust if we checked for the equals, i.e.
/\bcharset=\b/, just as we check for the slash after text in m!^text/!.

But these are minor issues; overall this is a good and important fix for us to
have. r=myk

Comment 4

[Dave Miller [:justdave]]

(In reply to comment #3)

> $type will never eq '' since the preceding line will populate an empty string,
> so "$type ne ''" doesn't need to be checked;
> 
> /\bcharset\b/ would be more robust if we checked for the equals, i.e.
> /\bcharset=\b/, just as we check for the slash after text in m!^text/!.

Hah! :)  Yeah, that code was copied verbatim from Perl's CGI.pm. :)  Go figure.

committed on trunk:

Checking in Bugzilla/CGI.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/CGI.pm,v  <--  CGI.pm
new revision: 1.12; previous revision: 1.11
done

and 2.18 branch:

Checking in Bugzilla/CGI.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/CGI.pm,v  <--  CGI.pm
new revision: 1.10.2.2; previous revision: 1.10.2.1
done