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

RESOLVED FIXED in mozilla1.3beta

Status

()

P3
major
RESOLVED FIXED
16 years ago
16 years ago

People

(Reporter: Mariusz.Krzanowski, Assigned: john)

Tracking

Trunk
mozilla1.3beta
x86
Windows 2000
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [FIX], URL)

Attachments

(2 attachments)

(Reporter)

Description

16 years ago
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.
(Reporter)

Comment 1

16 years ago
I forged add build number. It is 2002053012
cc'ing mstoltz. isn't this one of our security mechanisms?

Comment 3

16 years ago
*** Bug 154423 has been marked as a duplicate of this bug. ***
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

16 years ago
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.
Aren't file-upload controls in jkeiser's domain?  In anycase this sounds like
it's by design as a security protection.

Comment 7

16 years ago
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 ? :) ...
(Assignee)

Comment 8

16 years ago
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).
Assignee: rods → jkeiser
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: [INPUT TYPE=FILE ] Uploading file with invisible browse does not work → [INPUT TYPE=FILE ] file input with display: none does not hold value
(Assignee)

Comment 9

16 years ago
Created attachment 107782 [details]
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.
(Assignee)

Comment 10

16 years ago
Created attachment 108707 [details] [diff] [review]
Patch

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

Comment 11

16 years ago
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.
Status: NEW → ASSIGNED
Whiteboard: [FIX]
(Assignee)

Updated

16 years ago
Attachment #108707 - Flags: review?(bugmail)
Comment on attachment 108707 [details] [diff] [review]
Patch

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

Comment 13

16 years ago
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.
Depends on: 175670
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?)
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.
(Assignee)

Comment 16

16 years ago
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.
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 :)
Fair enough. As long as scripts can't set the value of the control, we should be OK.
(Reporter)

Comment 19

16 years ago
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.
(Assignee)

Updated

16 years ago
Blocks: 179440
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
Attachment #108707 - Flags: review?(bugmail) → review+
(Assignee)

Updated

16 years ago
Attachment #108707 - Flags: superreview?(jst)
(Assignee)

Updated

16 years ago
Priority: -- → P3
Target Milestone: --- → mozilla1.3beta
Comment on attachment 108707 [details] [diff] [review]
Patch

sr=jst
Attachment #108707 - Flags: superreview?(jst) → superreview+
(Assignee)

Comment 22

16 years ago
fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
(Assignee)

Comment 23

16 years ago
*** Bug 182388 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.