357526 - buglist.cgi doesn't specify encoding as UTF-8 when the rest of Bugzilla does

Status: RESOLVED FIXED

(Bugzilla :: Query/Bug List, defect)

Description

[John Beranek]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.0.7) Gecko/20060909 Firefox/1.5.0.7
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.0.7) Gecko/20060909 Firefox/1.5.0.7

It appears that the Landfill bugzilla-tip installation of Bugzilla is configured for UTF-8 support, but found that buglist.cgi was sent marked as Latin-1.

I discovered this because I have a saved search with some characters that don't fit in Latin-1 or have different byte representations in UTF-8. The saved search link looked fine on all the other pages I've been through, but is wrong on buglist.cgi.

I hope this isn't a duplicate, I did a bit of searching and didn't find a report of this.


Reproducible: Always

Steps to Reproduce:
1. Save a saved search with some odd characters in, I had UK pound, Euro and some Japanese
2. Go to a buglist

Actual Results:  
You see messed up characters wherever the name of the saved search is shown.

Expected Results:  
The correct characters for the name of the saved search should be shown.

Comment 1

[John Beranek]

It seems like neither Bugzilla's or the stock CGI module (3.15 here, don't know about Landfill's)'s multipart_start() function actually handles the -type parameter buglist.cgi tries to use.

So, a possible patch follows, though perhaps a mod to Bugzilla/CGI.pm might be tidier.

Oh, also, the else clause of "if ($serverpush)" doesn't do anything with the utf8 Param either - unless that's handled some other way...

Comment 2

[John Beranek]

Attachment: Fixes up the call to multipart_start()

Fixes up the call to multipart_start() in buglist.cgi, to add UTF-8 charset specifier, if the utf8 Param is set.

Might be better off being done in Bugzilla/CGI.pm - I'm not sure...

Comment 3

[John Beranek]

(In reply to comment #1)
> It seems like neither Bugzilla's or the stock CGI module (3.15 here, don't know
> about Landfill's)'s multipart_start() function actually handles the -type
> parameter buglist.cgi tries to use.

I mean -charset, obviously. :)

I've now actually installed a tip installation of Bugzilla locally (my what a lot of Perl deps) and my patch does indeed work here...

Comment 4

[John Beranek]

Attachment: Fixup the call to multipart_start()

Nitpick on previous patch, adds missing semicolon.

Comment 5

[John Beranek]

OK, last comment from me on this for now. I've realised the call to $self->charset() in Bugzilla/CGI.pm's new() means that in the else clause (when server push isn't used), buglist.cgi _does_ get it right.

Verified with IE 7.

Comment 6

[Marc Schumann [:Wurblzap]]

Confirming.

Comment 7

[John Beranek]

Attachment: An alternate patch, to Bugzilla::CGI::multipart_start

This alternate patch leaves buglist.cgi alone, and fixes Bugzilla::CGI::multipart_start() to accept a -charset parameter...

Comment 8

[John Beranek]

Attachment: An alternate patch, to Bugzilla::CGI::multipart_start (version 2)

nitpick on first version, fix a typo in a comment.

Comment 9

[Max Kanat-Alexander]

Yeah, if this is true, it blocks 3.0. We want UTF-8 support to be very polished.

Comment 10

[Max Kanat-Alexander]

Comment on attachment 243050 [details] [diff] [review]
Fixup the call to multipart_start()

Yeah, I prefer fixing multipart_start, like you did.

By the way, this also counts as an upstream bug in CGI.pm, so you should file it in the perl RT.

Comment 11

[Max Kanat-Alexander]

Comment on attachment 243079 [details] [diff] [review]
An alternate patch, to Bugzilla::CGI::multipart_start (version 2)

Yes, after looking over CGI.pm, I see that their multipart_start is basically just stupid. It desperately needs to be fixed to work properly like header() does.

By inspection, this is the correct solution.

>+        # Remove any existing charset specifier
>+        $args{-type} =~ s/;.*$//;

  That's probably not necessary, though. CGI.pm doesn't do it (in header()).

Comment 12

[Max Kanat-Alexander]

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

Comment 13

[John Beranek]

(In reply to comment #12)
> Checking in Bugzilla/CGI.pm;
> /cvsroot/mozilla/webtools/bugzilla/Bugzilla/CGI.pm,v  <--  CGI.pm
> new revision: 1.30; previous revision: 1.29
> done
> 

Cheers.

I've verified the fix on http://landfill.bugzilla.org/bugzilla-tip/ and submitted a job on http://rt.cpan.org/ - ticket 22737.

Comment 14

[Frédéric Buclin]

I wonder if this patch/bug triggers the error messages which are filling my apache error log:

buglist.cgi: Use of uninitialized value in string ne at (eval 43) line 31.

The corresponding line is in CGI::header() (I'm using CGI 3.16):

 $type .= "; charset=$charset" if $type ne '' and $type !~ /\bcharset\b/ and $charset ne '';

Here, $charset is undefined, generating the error message above.


I don't understand the goal of this patch. Why not passing the charset to CGI::multipart_init() directly and let it call CGI::header()?

If Bugzilla::CGI is a hack to work around some bugs in older versions of CGI, then it's high time to require a newer minimal version of CGI for Bugzilla 3.0.

Comment 15

[John Beranek]

(In reply to comment #14)
> I wonder if this patch/bug triggers the error messages which are filling my
> apache error log:
> 
> buglist.cgi: Use of uninitialized value in string ne at (eval 43) line 31.
> 
> The corresponding line is in CGI::header() (I'm using CGI 3.16):
> 
>  $type .= "; charset=$charset" if $type ne '' and $type !~ /\bcharset\b/ and
> $charset ne '';

Nothing to do with my patch at all - in the multipart_start() case, header()'s not called at all.

> I don't understand the goal of this patch. Why not passing the charset to
> CGI::multipart_init() directly and let it call CGI::header()?

Erm, because CGI::multipart_init() doesn't accept the charset parameter, or indeed call CGI::header().

> If Bugzilla::CGI is a hack to work around some bugs in older versions of CGI,
> then it's high time to require a newer minimal version of CGI for Bugzilla 3.0.

It's not just a hack as far as I can see, it adds extra functionality to the stock CGI module, just what inheritance is designed for. The current version of CGI also doesn't accept charset in its multipart_start() function...

Comment 16

[Max Kanat-Alexander]

John is correct--the current version of CGI.pm is just as broken as it was two years ago. They haven't updated the multipart code at all.