408446 - Non-text attachments are mangled by "binmode STDOUT, ':utf8'"

Status: RESOLVED FIXED

(Bugzilla :: Attachments & Requests, defect, P1)

Description

[雷]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.2; zh-CN; rv:1.8.1.11) Gecko/20071127 Firefox/2.0.0.11
Build Identifier: 1.137

attachment.cgi can not display attachment right; pic can not display or html file garbled .

Reproducible: Always

Steps to Reproduce:
1.upload a pic for a bug
2.view attachment
3 [review].firefox display the pic have problem and can not be display

or 
1.upload a html(charset:gb2312) for a bug
2 view html
3the html file garbled  beacase $cgi->charset is utf-8 

Actual Results:  
firefox display the pic have problem and can not be display

Expected Results:  
display the pic 

i think we should use binmode when output a attachment and clear default charset for text/html mime

in attachment.cgi:

$cgi->charset('');
print $cgi->header(-type=>"$contenttype;name=\"$filename\"",
                       -content_disposition=> "inline; filename=\"$filename\"",
                       -content_length => $attachment->datasize);
	binmode STDOUT;
print $attachment->data;

Comment 1

[Max Kanat-Alexander]

Yeah, we know about this, but we hadn't filed a bug yet. :-)

In case anybody isn't sure what this bug is about: Since we switched to utf8 strings in Bugzilla, and set "binmode STDOUT, ':utf8'" for all output, any non-text attachment is garbled when it comes out of the DB. It seems to be fine going into the DB, but not coming out.

I'm not sure if we need to utf8::downgrade the string coming out, or just turn off the ':utf8' IO layer on STDOUT.

Comment 2

[雷]

yeah, i find (binmode STDOUT, ':utf8') if Bugzilla->params->{'utf8'};
so it should not have this bug.

i fell the problem happend come out out the DB originally.
but when i add “binmode STDOUT”;
before print $attachment->data; the problem fixed;
then ,i think the data coming out DB may not have problem.
may be some place reset the binmode;

Comment 3

[Max Kanat-Alexander]

(In reply to comment #2)
> yeah, i find (binmode STDOUT, ':utf8') if Bugzilla->params->{'utf8'};
> so it should not have this bug.

  No, no. "binmode STDOUT" is not he same as "binmode STDOUT, ':utf8'". The second prints everything out as through it was a utf8 character, instead of printing out binary. We need to make attachments print out as binary. It should be pretty simple, as you say--it should just be adding "binmode STDOUT" before we print attachments.

Comment 4

[A. Shimono [:himorin]]

it seems that mysql dbd (dbi driver?) doesn't set utf8 flag for longblob type.

$attachment->data : not utf8 flagged
$attachment->description : utf8 flagged
----
$VAR1 = bless( {
                 'ispatch' => '0',
                 'data' => <garbaged>,
                 'attached' => '2007.12.20 17:29',
                 'bug_id' => '5982',
                 'description' => "\x{30c6}\x{30b9}\x{30c8}",
                 'isurl' => '0',
                 'isprivate' => '0',
                 'filename' => 'test.txt',
                 'isobsolete' => '0',
                 'id' => '3668',
                 'contenttype' => 'text/plain',
                 'attacher_id' => '1856',
                 'modification_time' => '2007-12-20 17:29:02'
               }, 'Bugzilla::Attachment' );

Comment 5

[Frédéric Buclin]

(In reply to comment #4)
> it seems that mysql dbd (dbi driver?) doesn't set utf8 flag for longblob type.
> 
> $attachment->data : not utf8 flagged
> $attachment->description : utf8 flagged

Does it explain why my first name (Frédéric) is mangled in patches I attach in Bugzilla 3.1.2+? Both the "view" and "diff" modes show my first name as Frédéric and downloading the patch doesn't help; it's mangled too. But my first name is displayed correctly when reading my patch from the SQL command-line directly so it seems the patch is correctly stored in the DB, fortunately.

Comment 6

[Max Kanat-Alexander]

(In reply to comment #5)
> Does it explain why my first name (Frédéric) is mangled in patches I attach
> in Bugzilla 3.1.2+?

  Yes, that's probably related to this bug.

Comment 7

[Max Kanat-Alexander]

Attachment: v1

Okay, this seems to fix it for me. This is what I thought I'd have to do, too.

Comment 8

[Frédéric Buclin]

Comment on attachment 295309 [details] [diff] [review]
v1

>+        binmode STDOUT, ':raw'; # Turn off UTF8 encoding.

This fixes the problem when viewing, viewing all, downloading or viewing details of attachments, including images. But this still doesn't fix the problem when viewing a patch in diff or interdiff mode. It seems that adding the same code in Bugzilla::Attachment::PatchReader::process_(inter)diff fixes the problem there, but I wonder if we should then add |binmode STDOUT, ':utf8';| again after the patch is displayed so that Bugzilla can still display the footer of the page correctly. Max, what do you think?

Comment 9

[Frédéric Buclin]

It would be nice to fix diff and interdiff too before committing this patch.

Comment 10

[Max Kanat-Alexander]

We can fix those in a separate patch. Could you file a bug for that?

Comment 11

[A. Shimono [:himorin]]

(In reply to comment #6)
> > Does it explain why my first name (Frédéric) is mangled in patches I attach
> > in Bugzilla 3.1.2+?
> 
>   Yes, that's probably related to this bug.

Sorry for late.
That's related to this bug. And,, i think I should change the bug summary like
"Bytes over 0x80 in attachment datas are mangled by "binmode STDOUT, ':utf8'".

Comment 12

[Frédéric Buclin]

(In reply to comment #10)
> We can fix those in a separate patch. Could you file a bug for that?
> 

Done! I filed bug 410902 about diff and interdiff.

Comment 13

[Max Kanat-Alexander]

Checking in attachment.cgi;
/cvsroot/mozilla/webtools/bugzilla/attachment.cgi,v  <--  attachment.cgi
new revision: 1.138; previous revision: 1.137
done

Comment 14

[雷]

well anthoer problem is: When  a add a html file as acttachement and the file has is own charset like gb2312 or iso8859-1 , then I view the file , the file can not display right first .I should change charset via browser.
becase bugzilla output chatset:utf-8

Comment 15

[Max Kanat-Alexander]

(In reply to comment #14)
> well anthoer problem is: When  a add a html file as acttachement and the file
> has is own charset like gb2312 or iso8859-1 , then I view the file , the file
> can not display right first .I should change charset via browser.
> becase bugzilla output chatset:utf-8

  That's a different bug, already filed.