Closed Bug 919041 Opened 6 years ago Closed 6 years ago

Remove BMO feature which alters content-type on attachments from users without editbugs

Categories

(bugzilla.mozilla.org :: General, defect)

Production
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: fb+mozdev, Assigned: glob)

Details

Attachments

(2 obsolete files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:24.0) Gecko/20100101 Firefox/24.0 (Beta/Release)
Build ID: 20130909203154

Steps to reproduce:

Click "add an attachment" & add file, select "text/html" from the content type dropdown, submit.


Actual results:

Radio button switches to "select from list". Content type for new attachment says "text/plain". 


Expected results:

Content type for new attachment should say "text/html".


(I somehow suspect this is a privilege issue, i. e. for normal users saving the dropdown value fails.)
Attached file example attachment (obsolete) —
Comment on attachment 808011 [details]
example attachment

I can however correct the content type afterwards. 

(BTW: Attachment details editing says "MIME Type" instead of "Content Type".
Attachment #808011 - Attachment mime type: text/plain → text/html
Attached file example attachment #2 (obsolete) —
Attachment #808011 - Attachment is obsolete: true
Comment on attachment 808013 [details]
example attachment #2 [details] [diff] [review]

manually entering (i. e. typing the string "text/html") the content type does not work either when adding an attachment
Attachment #808013 - Attachment is obsolete: true
Attachment #808013 - Attachment mime type: text/plain → text/html
(In reply to Florian Bender from comment #4)
> Comment on attachment 808013 [details]
> example attachment #2 [details] [diff] [review] [diff] [review]
> 
> manually entering (i. e. typing the string "text/html") the content type
> does not work either when adding an attachment

Are you de-selecting the "Patch" checkbox? If not then, when checked, patches automatically become text/plain.

dkl
Flags: needinfo?(fb+mozdev)
(In reply to David Lawrence [:dkl] from comment #5)
> Are you de-selecting the "Patch" checkbox? If not then, when checked,
> patches automatically become text/plain.

Aren't the mime-type settings grayed out when the patch box is checked?
Complete STR in comment 0. I neither selected nor deselected patch.
Flags: needinfo?(fb+mozdev)
bmo has custom code to restrict content types attachable by non-privileged people (object_end_of_create_validators in the BMO extension).  non-privileged people in this context is users without editbugs.

the code honours image/* and application/pdf content-types, changes text/* content-types to text/plain, and changes any others to application/octet-stream.


this code came from bug 625004, and is completely unrelated to profanivore.

gerv - can you explain the rational behind this code?
i don't think we should impose these restrictions as they pose no real protection, just annoyance.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(gerv)
If I had to guess, I'd say it's probably intended to stop spammers from hosting ads or malicious javascript here.
Well, I could change the Content Type in a later step, so either this does not work as expected, or it was not supposed to work like that. 

When auto-switching, you may try to 
1) scan for <script> blocks and switch the Content Type accordingly,
2) display a message showing the intentional switch of Content Types! 

The latter one is important to not confuse people.
glob: that patch is from bug 554121. The aim was to not allow untrusted people (those without editbugs) to upload malicious attachments - i.e. those with active content. However, that bug was eventually RESOLVED WONTFIX. Its inclusion in that checkin was a screw-up on my part.

So perhaps we should remove it - although the fact that it's survived so long without seeming to cause anyone much inconvenience (until now) suggests that maybe it's actually a reasonable security measure? Both reed and Max said in the original bug that it would be very inconvenient to have - but it seems like we've had it by accident for two and a half years without anyone being inconvenienced until now.

Gerv
Flags: needinfo?(gerv)
(In reply to Gervase Markham [:gerv] from comment #11)
> So perhaps we should remove it - although the fact that it's survived so
> long without seeming to cause anyone much inconvenience (until now) suggests
> that maybe it's actually a reasonable security measure?

this isn't the first time this issue has been raised, just the first time a bug's been filed.
i've had several discussions on irc about this, all from people using an api to upload a file and having the content-type "mysteriously" changed.

let's remove the code :)
Assignee: nobody → glob
Summary: BMO ignores my attachment content type selection → Remove BMO feature which alters content-type on attachments from users without editbugs
Committing to: bzr+ssh://bjones%40mozilla.com@bzr.mozilla.org/bmo/4.2/
modified extensions/BMO/Extension.pm
Committed revision 9033.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Awesome, thanks a lot!
You need to log in before you can comment on or make changes to this bug.