165943 - Patch: allow URL with "file://" scheme in HTML file upload form (input type="file")

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, enhancement)

Description

[Tilmann Bubeck]

Normally in form input fields of type "file" it is possible to use the file
browser to select a local file and upload this file. By using drag-and-drop from
other mozilla windows you normally get a full URL containing the scheme
"file://" which is written into the input field.

Without the patch, mozilla will try to open the file including the scheme and
will never find the file. Then it will not upload the file.

This patch changes mozilla so that it recognizes "file://" URLs and is able to
upload them. This makes it possible to use drag-and-drop to drop files onto file
input fields.

The patch is based upon mozilla-1.1. Could you please apply?

Thanks!
  Till

--- content/html/content/src/nsHTMLInputElement.cpp-moz11	Sat Aug 31 21:34:22 2002
+++ content/html/content/src/nsHTMLInputElement.cpp	Sat Aug 31 22:21:06 2002
@@ -103,7 +103,7 @@
 #include "nsIFile.h"
 #include "nsILocalFile.h"
 #include "nsIFileStreams.h"
-
+#include "nsNetUtil.h"
 
 // XXX align=left, hspace, vspace, border? other nav4 attrs
 
@@ -2339,14 +2339,20 @@
   //
   if (type == NS_FORM_INPUT_FILE) {
 
-    //
-    // Open the file
-    //
     nsCOMPtr<nsILocalFile> file(do_CreateInstance(NS_LOCAL_FILE_CONTRACTID,
                                                   &rv));
     NS_ENSURE_SUCCESS(rv, rv);
 
-    rv = file->InitWithPath(value);
+    if (Substring(value, 0, 6) == NS_LITERAL_STRING("file:/")) {
+      rv = NS_InitFileFromURLSpec(file, NS_ConvertUCS2toUTF8(value)); 
+    } else {
+      rv = file->InitWithPath(value);
+    }
+
+    //
+    // Open the file
+    //
+
     if (NS_SUCCEEDED(rv)) {
 
       //

Comment 1

[John Keiser (jkeiser)]

Good patch, thanks :)  Three things:

(1) you only have to look for "file:" to determine if it's a file URL, according
to the way URLs work.
(2) I don't see a reason to move that comment--it works better up before the
init and file decl anyway.
(3) could you please upload the patch as an attachment?  Standard procedure :)

I'll r= with those changes.  It would be nice if we solve the general case (bug
121059), but I can live with just doing file: for now :)

CC'ing darin, who ought to sr= this.

Comment 2

[John Keiser (jkeiser)]

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

Comment 3

[Darin Fisher]

this patch will not compile against the trunk.  NS_InitFileFromURLSpec has been
replaced by NS_GetFileFromURLSpec, which returns a nsIFile object.

Comment 4

[Tilmann Bubeck]

Attachment: patch using NS_GetFileFromURLSpec

This patch changes mozilla so that it recognizes "file://" URLs and is able to
upload them. This makes it possible to use drag-and-drop to drop files onto
file input fields.

The patch is based upon the current mozilla CVS trunk and uses
NS_GetFileFromURLSpec as [e-mail] correctly requested.
Could you please apply?

I also thought about a more improved patch which uses nsStandardURL to allow an
arbitrary URL inside an input field. It will then take this URL, do an "open"
and feed the InputStream into the aFormSubmission->AddNameFilePair. However I'm
not sure what this means for a delay, if the URL takes a long time to download
or an error happens... Are you interested in this?

In any case it would be great if you apply the current patch, because I will
need some time to write the above mentioned (better) patch.

Comment 5

[Darin Fisher]

Comment on attachment 97472 [details] [diff] [review]
patch using NS_GetFileFromURLSpec

>+    rv = NS_GetFileFromURLSpec(NS_ConvertUCS2toUTF8(value), 

if performance matters, you might want to ensure that the first 5 chars
equals "file:" before calling into GetFileFromURLSpec.


>+    if (NS_FAILED(rv)) {
>+	// this is no "file://", try as local file
>+	nsCOMPtr<nsILocalFile> localFile;
>+	rv = NS_NewNativeLocalFile(NS_ConvertUCS2toUTF8(value), PR_FALSE,
>+				   getter_AddRefs(localFile));

this looks very suspicious... NS_NewNativeLocalFile expects text in
the native charset of the underlying file system.  (e.g.., ISO-8859-1
if locale is en-US.)  so, you need to call NS_NewLocalFile(value)
instead.

also iirc, file:// URLs are formatted using the native charset with
non-ASCII bytes URL escaped.  in other words, if value is an URL, 
then value had better be ASCII... but, if value contains any non-ASCII
unicode chars, then treating it as an URL is way wrong!  you shouldn't
worry too much about this... NS_GetFileFromURLSpec should error out
(even if it doesn't do so today, which'd be a bug).

Comment 6

[John Keiser (jkeiser)]

I don't think this is extremely performance-critical--file inputs are rare and
for goodness sakes, we're opening and reading a file here :)  But if you want to
check for file: it's OK by me.

The Native thing is a big deal.  Some filesystems support storing filenames as
Unicode or UTF8.  Though I wonder how we deal with that in file URLs?  Probably
escape the non-ASCII characters with %..?  So that we read in the URL as UTF8
after decoding?  In that case we'd want to URL-encode the filename before
passing it in.

Passing the InputStream *directly* from a URL wouldn't work because we have to
know the stream's size immediately before we ever send it--we have to report it
as part of Content-Length header.  But downloading into a string or file and
making a stream off of that would work (if you did a file you could use the
DELETE_ON_CLOSE feature, though there are nuances involved with re-POSTing that
would make this feature "fun" to implement).

Comment 7

[Darin Fisher]

i'm 90% sure that if value ever contained a file:// URL, that it would be
properly formatted already.  anyways, it is not the responsibility of this patch
to ensure that.  NS_ConvertUCS2toUTF8 is OK when calling NS_GetFileFromURLSpec
because the API currently says that that the input URL spec should be UTF8.  so,
only change that is necessary is the NS_NewLocalFile(value) one.

Comment 8

[timeless]

BeOS uses UTF8 :-)

Comment 9

[Tilmann Bubeck]

Attachment: Patch with NS_GetFileFromURLSpec and NS_NewLocalFile

This is a slightly modified version of my prevois patch. It now uses
NS_NewLocalFile instead of NS_NewNativeLocalFile.

Comment 10

[Darin Fisher]

Comment on attachment 97617 [details] [diff] [review]
Patch with NS_GetFileFromURLSpec and NS_NewLocalFile

sr=darin

Comment 11

[John Keiser (jkeiser)]

Comment on attachment 97617 [details] [diff] [review]
Patch with NS_GetFileFromURLSpec and NS_NewLocalFile

r=jkeiser

Do you have CVS access?  Do you need someone to check this in for you?

Comment 12

[Tilmann Bubeck]

Do you mean me (with CVS acess)? No, I don't have CVS access (also I am capable
of using CVS). Could someone else please check this in?

Comment 13

[Tilmann Bubeck]

Is there anything wrong with the patch? I didn't find it in mozilla 1.2 alpha...
Should I change anything to your needs?

Thanks!

Comment 14

[John Keiser (jkeiser)]

Attachment: Testing Testing 123

Re-uploading the patch to test this bug

Comment 15

[John Keiser (jkeiser)]

Attachment: Testing Testing 123

Re-uploading with normal URL to test again (ignore)

Comment 16

[John Keiser (jkeiser)]

Attachment: Patch v1.1

This is a slightly modified version of the patch incorporating darin's earlier
suggestion ... only because we are asserting now whenever we do a normal file
upload.

Comment 17

[Darin Fisher]

Comment on attachment 99141 [details] [diff] [review]
Patch v1.1

you probably want to use a case insensitive string compare since necko would
accept "FiLe:"

Comment 18

[John Keiser (jkeiser)]

Attachment: Patch v1.1.1

Good point.

Comment 19

[John Keiser (jkeiser)]

Comment on attachment 99146 [details] [diff] [review]
Patch v1.1.1

r=jkeiser

Comment 20

[Darin Fisher]

Comment on attachment 99146 [details] [diff] [review]
Patch v1.1.1

sr=darin

Comment 21

[John Keiser (jkeiser)]

Fix checked in.

Comment 22

[Cristian Tuns]

Attachment: Bug 165943 - disable browser_nested_iframe.js on mac x64 r=#intermittent-reviewers

Comment 23

[Phabricator Automation]

Comment on attachment 9260858 [details]
Bug 165943 - disable browser_nested_iframe.js on mac x64 r=#intermittent-reviewers

Revision D137036 was moved to bug 1659435. Setting attachment 9260858 [details] to obsolete.