152844 - [INPUT TYPE=FILE ] file input with display: none does not hold value

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect, P3)

Description

[Mariusz Krzanowski]

I cannot upload file if <Input type=file > is invisible.
E.g.

I have following code
 
<td id="Td1" style="display:block; " >
<form id="Form1" enctype="multipart/form-data" action="some_dest_here"
method="post" >
  <input name="temp-file" type=file >
</form>
</td>

-----------------------------
Situation 1)

1) I'm pressing browse button and I'm selecting file to upload.
2) I'm pressing some other button, which by JavaScript executes FORM
and sends file to the server. Everything is fine.

---------

var curr_form = document.getElementById('Form1');
curr_form.submit();

--------------------------------------------------
Situation 2)

Before sending by Java Script FORM I set TD style to "dsplay: none"
and no file is sent by Mozilla.

var curr_td = document.getElementById('Td1');
curr_td.style.display = "none";
var curr_form = document.getElementById('Form1');
curr_form.submit();

----------------------
I checked also following combination

curr_td.style.display="none";
curr_td.style.display="block";

And I saw that file name went away.

------------------------

For my development it is a very serious problem. 
I tested behaviour of Internet Explorer and it allow send file 
if browse is invisible.

Comment 1

[Mariusz Krzanowski]

I forged add build number. It is 2002053012

Comment 2

[Christopher Hoess (gone)]

cc'ing mstoltz. isn't this one of our security mechanisms?

Comment 3

[Alfonso Martinez]

*** Bug 154423 has been marked as a duplicate of this bug. ***

Comment 4

[Mitchell Stoltz (not reading bugmail)]

This might be a security mechanism, I'm not sure. I'll look into it. The
behavior described here might be by design.

Comment 5

[David Mårtensson]

This is stated in RFC according hidden file fields
Look at last line.

Nebel & Masinter              Experimental                     [Page 10]

RFC 1867             Form-based File Upload in HTML        November 1995


8. Security Considerations

   It is important that a user agent not send any file that the user has
   not explicitly asked to be sent. Thus, HTML interpreting agents are
   expected to confirm any default file names that might be suggested
   with <INPUT TYPE=file VALUE="yyyy">.  Never have any hidden fields be
   able to specify any file.

Comment 6

[Daniel Veditz [:dveditz]]

Aren't file-upload controls in jkeiser's domain?  In anycase this sounds like
it's by design as a security protection.

Comment 7

[Roland Mainz]

dveditz wrote:
> In anycase this sounds like it's by design as a security protection.

IMHO there should be a warning dialog which tells the user that something is
being uploaded (for each file upload, regardless whether the form node is
visible or not) - including the filename being used. Just malfunctioning without
any hint what's going on is bad (what about morphing the bug ? :) ...

Comment 8

[John Keiser (jkeiser)]

Hmm, I don't think this was intentional, but perhaps it should be.  A worse
problem (potentially) is that if you have a wizard with a file upload in it
(wizards often set one page to display: none while displaying another), it will
lose the value the user typed (testcase in a moment).

Comment 9

[John Keiser (jkeiser)]

Attachment: Testcase

Instructions: (a) type something into the file input.  (b) check "turn blah
off".  (c) check "turn blah on".  Note that blah does not have a value anymore.

Comment 10

[John Keiser (jkeiser)]

Attachment: Patch

This fixes the problem by saving the value into the file input when the frame
goes away.

Comment 11

[John Keiser (jkeiser)]

This URL serves as a testcase:

http://www.johnkeiser.com/cgi-bin/mozform.pl?method=post&action=http%3A%2F%2Fwww.mozilla.gr.jp%3A4321&enctype=multipart/form-data

Steps to reproduce:
1. Enter a valid filename in the leftmost file control.
2. Click "Hide"
3. Click "Submit From Outside"
4. Verify that the file and its contents submitted.

Comment 12

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 108707 [details] [diff] [review]
Patch

is this the entire patch? I can't find any TakeTextFrameValue-function in
nsITextControlElement

Comment 13

[John Keiser (jkeiser)]

Yep, that's the entire patch ... I neglected to mention it depends on the patch
for bug 175670 being checked in because I thought I was going to get to check it
in tonight.  Sadly, we are in a freeze.

Think of TakeTextFrameValue() like SetValueGuaranteed().  See patch there,
actually.  All it does is set the input's mValue.

Comment 14

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Calling TakeTextFrameValue will set call SetValueChanged(PR_TRUE) which isn't
something we want to do just because .type or .style.display is changed. You
should probably only call TakeTextFrameValue if the user has changed the
textvalue since the frame was created (otherwise the old value will still be in
the content-node so we won't have to update it, right?)

Comment 15

[Mitchell Stoltz (not reading bugmail)]

It's good not to lose the value of the file control when it is hidden, but we
shouldn't allow submitting from hidden file controls.

Comment 16

[John Keiser (jkeiser)]

sicking: If anything needs to check whether value == defaultValue, it would be
TakeTextFrameValue()--the same problem exists for text inputs.  I'll add that in
once bug 175670 lands.  (Making a patch before then will be problematic :)

mstoltz: there are legitimate uses of it in wizards, though I see your point. 
However, consider the fact that a malicious web author can always make the input
type=file absolutely positioned and layer a div over it that is also absolutely
positioned, for the same effect.  Given that they can already do the same thing
in a different way, it seems like we would be punishing wizard developers for
very little security gain if we forced the file input not to submit when it is
display: none.

Comment 17

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

there is no way we can stop people from submitting hidden filecontrols. It will
always be possible to hide it using CSS by putting a non-transparent positioned
box over ot, or by putting it inside a 1x1px overflow:hidden parent.

So allowing display:none filecontrols to be submitted simply allows good
developers to do what they want to do in a good way, while not letting evil.com
do more then they can anyway.

IMHO of course :)

Comment 18

[Mitchell Stoltz (not reading bugmail)]

Fair enough. As long as scripts can't set the value of the control, we should be OK.

Comment 19

[Mariusz Krzanowski]

The browse button is read-only (I mean it's value). User has to select the file
conscious that it will be sent to the WWW server. Protecting by sending hidden
form is not necessary, because after file selecting the browser can send also
visible form using Java Script without user action. 
I think, until the user decides which file he/she want to make accessible for
the browser everything is OK.

Comment 20

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 108707 [details] [diff] [review]
Patch

eek, sorry i had forgot about this one. r=me since the changed flag doesn't
matter for file-fields anyway

Comment 21

[Johnny Stenback (:jst)]

Comment on attachment 108707 [details] [diff] [review]
Patch

sr=jst

Comment 22

[John Keiser (jkeiser)]

fix checked in.

Comment 23

[John Keiser (jkeiser)]

*** Bug 182388 has been marked as a duplicate of this bug. ***