Florian Bender Reporter
	Description • 11 years ago

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.)

	Florian Bender Reporter
	Comment 2 • 11 years ago

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".

	Florian Bender Reporter
	Comment 4 • 11 years ago

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

	David Lawrence [:dkl]
	Comment 5 • 11 years ago

(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

	Dave Miller [:justdave]
	Comment 6 • 11 years ago

(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?

	Florian Bender Reporter
	Comment 7 • 11 years ago

Complete STR in comment 0. I neither selected nor deselected patch.

	:glob ✱ Assignee
	Comment 8 • 11 years ago

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.

	Dave Miller [:justdave]
	Comment 9 • 11 years ago

If I had to guess, I'd say it's probably intended to stop spammers from hosting ads or malicious javascript here.

	Florian Bender Reporter
	Comment 10 • 11 years ago

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.

	Gervase Markham [:gerv]
	Comment 11 • 11 years ago

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

	:glob ✱ Assignee
	Comment 12 • 11 years ago

(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 :)

	:glob ✱ Assignee
	Comment 13 • 11 years ago

Committing to: bzr+ssh://bjones%40mozilla.com@bzr.mozilla.org/bmo/4.2/
modified extensions/BMO/Extension.pm
Committed revision 9033.

	Florian Bender Reporter
	Comment 14 • 11 years ago

Awesome, thanks a lot!