622455 - The attachment mime type autodetection shouldn't rely on the browser when it gets text/x-*

Status: RESOLVED FIXED

(Bugzilla :: Attachments & Requests, enhancement, P2)

Description

[Michal 'hramrach' Suchanek]

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.13) Gecko/20101209 Iceweasel/3.6.13 (like Firefox/3.6.13)
Build Identifier: 3.6.3+

Attachment type is detected by filename only. Tested on 3.6.3+ landfill bugzilla install.

Since many browsers send the filename in the headers it is trivial to implement "autodetection" this way but it does not really detect file type, only file name.

At the very least the "autodetect" option should read "autodetect (by file name)" and the JS bug entry page which only allows autodetection should mention that the attachment type is determined by file name.

Reproducible: Always

Steps to Reproduce:
1. create files config.log config.txt config_log (or config,log) with configure messages in them
2. attach them to a bug
3.
Actual Results:  
they get content type of text/x-log (not displayable in Mozilla browsers) text/plain and application/octet-stream

Expected Results:  
All the attachments get type text/plain so that they can be easily read in the browser as they contain 7-bit text only.



If this is too hard to do at least the file upload dialog and new bug dialog should describe how the type is "detected".

Comment 2

[Max Kanat-Alexander]

Also please understand that your browser is doing the autodetection, not Bugzilla.

Comment 3

[timeless]

mkanat: that's a bogus interpretation of "auto detect", as a localizer i'd have to translate "auto detect" as "user browser's type" or something.

Comment 4

[Michal 'hramrach' Suchanek]

This is not a duplicate of bug 621478.

bug 621478 is about fixing this issue in b.m.o installation, this is about fixing it in bugzilla proper so that all bugzilla installations get it once they are upgraded.

bug 621478 includes good explanation of the issue and links to patches, however.

This failure seen on bugzilla also points at Firefox's failure to detect file types in any reasonable way.

Thanks

Comment 5

[Frédéric Buclin]

Note that with such a bug summary, your bug should be closed as invalid.

Comment 6

[Max Kanat-Alexander]

Yep, Bugzilla doesn't have anything to do with autodetection, it's all in your browser. So this is INVALID.

Comment 7

[Max Kanat-Alexander]

Oh, wait, I see LpSolit changed the summary.

Comment 8

[Frédéric Buclin]

If the user selects "auto-detect" and the File::MimeInfo::Magic module is present, we should use it, else we fall back to what the browser says as we do currently. See e.g. http://bzr.mozilla.org/bugzilla/extensions/typesniffer/trunk/annotate/head:/Extension.pm

Comment 9

[Frédéric Buclin]

Too late for 4.4. We already released rc1.

Comment 10

[Matt Selsky [:selsky]]

Bug 760562 moved type-sniffing to the core.  And it already uses type-sniffing for 'application/octet-stream'.  Is the request for this bug to add 'text/plain' to the list of mime-types that activate type-sniffing?

Comment 11

[Michal 'hramrach' Suchanek]

The problem is that browser sends some bogus mime types all the time.

Like when you attach a log file it would tag it application/x-log or text/x-log which both the browser and bugzilla would refuse to treat as text. 

Also the browser does not really look at file content and uses extension only.

So I would suggest to make a list of few 'known good' mime types. If a different mime type is sent and autodetection is requested attempt detection. If type detection does not return 'known good' type just stick to what browser sent.

'known good' types should be configurable so that bugzilla installations dealing with special file types can optimize this and skip detection for the expected data type.

Comment 12

[Michal 'hramrach' Suchanek]

$ file upstart.log
upstart.log: assembler source, ASCII text, with CRLF, CR line terminators, with overstriking
$ mimetype upstart.log
upstart.log: text/x-log

sad...

Comment 13

[Gervase Markham [:gerv]]

I think this bug is requesting that we do autodetection when we get "text/x-*". The other alternative is to always switch "text/x-*" to "text/plain", given that "autodetection" doesn't do the right thing. Although File::MimeInfo::Magic may not always work the same way as the "mimetype" command...

We already convert to text/plain when we get text/x-diff or text/x-patch the the user clicked the "patch" checkbox. I think the logic would be: "the browser will generally only display text/plain. This is a text type, so the browser can display it. So let's switch it to text/plain so it can."

Gerv

Comment 14

[Frédéric Buclin]

(In reply to Gervase Markham [:gerv] from comment #13)
> I think the logic would be: "the
> browser will generally only display text/plain. This is a text type, so the
> browser can display it. So let's switch it to text/plain so it can."

I agree with gerv. Automatically convert text/x-* to text/plain. Make sure x-diff and x-patch still correctly enable the 'patch' bit as they do currently.


Updating the bug summary as application/octet-stream has already been fixed in bug 760562.

Comment 15

[Gervase Markham [:gerv]]

So I think fixing this bug would involve updating the few lines of code which specially handle text/x-diff and text/x-patch's conversion to handle all of text/x-*, but only set the patch bit for those two given types.

Gerv

Comment 16

[Frédéric Buclin]

Attachment: patch, v1

Comment 17

[David Lawrence [:dkl]]

Comment on attachment 8598180 [details] [diff] [review]
patch, v1

Review of attachment 8598180 [details] [diff] [review]:
-----------------------------------------------------------------

r=dkl

Comment 18

[Frédéric Buclin]

Any chance to have it for 5.0?

Comment 19

[:glob ✱]

(In reply to Frédéric Buclin from comment #18)
> Any chance to have it for 5.0?

too late for 5.0.  we already released rc3.

Comment 20

[Frédéric Buclin]

To ssh://[e-mail]/bugzilla/bugzilla.git
   27398a7..737ffe4  master -> master