Bug 1127150 - Fix FormData File filename. r=baku
☠☠ backed out by 717e92977f9a ☠ ☠
authorNikhil Marathe <nsm.nikhil@gmail.com>
Sat, 21 Feb 2015 11:54:44 -0800
changeset 257302 cc77a5165615f32a6d85087cc80d0bf88f93db9f
parent 257301 ef51eb31fa09d3580a68d4c918646cde5a456662
child 257303 717e92977f9a8eaf77ddaacaed259efe383e5759
push id4610
push userjlund@mozilla.com
push dateMon, 30 Mar 2015 18:32:55 +0000
treeherdermozilla-beta@4df54044d9ef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku
bugs1127150
milestone38.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1127150 - Fix FormData File filename. r=baku
dom/base/nsFormData.cpp
dom/base/nsFormData.h
dom/html/test/test_formData.html
--- a/dom/base/nsFormData.cpp
+++ b/dom/base/nsFormData.cpp
@@ -3,16 +3,18 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsFormData.h"
 #include "nsIVariant.h"
 #include "nsIInputStream.h"
 #include "mozilla/dom/File.h"
 #include "mozilla/dom/HTMLFormElement.h"
 
+#include "MultipartFileImpl.h"
+
 using namespace mozilla;
 using namespace mozilla::dom;
 
 nsFormData::nsFormData(nsISupports* aOwner)
   : nsFormSubmission(NS_LITERAL_CSTRING("UTF-8"), nullptr)
   , mOwner(aOwner)
 {
 }
@@ -70,23 +72,43 @@ nsFormData::Append(const nsAString& aNam
 {
   AddNameValuePair(aName, aValue);
 }
 
 void
 nsFormData::Append(const nsAString& aName, File& aBlob,
                    const Optional<nsAString>& aFilename)
 {
-  nsString filename;
-  if (aFilename.WasPassed()) {
-    filename = aFilename.Value();
-  } else {
-    filename.SetIsVoid(true);
+  // Step 3 "If value is a Blob object and not a File object, set value to
+  // a new File object, representing the same bytes, whose name attribute value
+  // is "blob"."
+  // Step 4 "If value is a File object and filename is given, set value to
+  // a new File object, representing the same bytes, whose name attribute
+  // value is filename."
+  nsString filename = NS_LITERAL_STRING("blob");
+  if (aBlob.IsFile()) {
+    aBlob.GetName(filename);
+    if (aFilename.WasPassed()) {
+      filename = aFilename.Value();
+    }
   }
-  AddNameFilePair(aName, &aBlob, filename);
+
+  nsAutoTArray<nsRefPtr<FileImpl>, 1> blobImpls;
+  blobImpls.AppendElement(aBlob.Impl());
+
+  nsAutoString contentType;
+  aBlob.GetType(contentType);
+
+  nsRefPtr<MultipartFileImpl> impl =
+    new MultipartFileImpl(blobImpls, filename, contentType);
+
+  nsRefPtr<File> file = new File(aBlob.GetParentObject(), impl);
+  nsAutoString voidString;
+  voidString.SetIsVoid(true);
+  AddNameFilePair(aName, file, voidString);
 }
 
 void
 nsFormData::Delete(const nsAString& aName)
 {
   // We have to use this slightly awkward for loop since uint32_t >= 0 is an
   // error for being always true.
   for (uint32_t i = mFormData.Length(); i-- > 0; ) {
@@ -259,20 +281,21 @@ nsFormData::Constructor(const GlobalObje
 // nsIXHRSendable
 
 NS_IMETHODIMP
 nsFormData::GetSendInfo(nsIInputStream** aBody, uint64_t* aContentLength,
                         nsACString& aContentType, nsACString& aCharset)
 {
   nsFSMultipartFormData fs(NS_LITERAL_CSTRING("UTF-8"), nullptr);
 
+  nsAutoString voidString;
+  voidString.SetIsVoid(true);
   for (uint32_t i = 0; i < mFormData.Length(); ++i) {
     if (mFormData[i].valueIsFile) {
-      fs.AddNameFilePair(mFormData[i].name, mFormData[i].fileValue,
-                         mFormData[i].filename);
+      fs.AddNameFilePair(mFormData[i].name, mFormData[i].fileValue, voidString);
     }
     else {
       fs.AddNameValuePair(mFormData[i].name, mFormData[i].stringValue);
     }
   }
 
   fs.GetContentType(aContentType);
   aCharset.Truncate();
--- a/dom/base/nsFormData.h
+++ b/dom/base/nsFormData.h
@@ -34,17 +34,16 @@ private:
   ~nsFormData() {}
 
   typedef mozilla::dom::File File;
   struct FormDataTuple
   {
     nsString name;
     nsString stringValue;
     nsRefPtr<File> fileValue;
-    nsString filename;
     bool valueIsFile;
   };
 
   // Returns the FormDataTuple to modify. This may be null, in which case
   // no element with aName was found.
   FormDataTuple*
   RemoveAllOthersAndGetFirstFormDataTuple(const nsAString& aName);
 
@@ -61,17 +60,16 @@ private:
   void SetNameFilePair(FormDataTuple* aData,
                        const nsAString& aName,
                        File* aBlob,
                        const nsAString& aFilename)
   {
     MOZ_ASSERT(aData);
     aData->name = aName;
     aData->fileValue = aBlob;
-    aData->filename = aFilename;
     aData->valueIsFile = true;
   }
 
   void ExtractValue(const FormDataTuple& aTuple,
                     mozilla::dom::OwningFileOrUSVString* aOutValue);
 public:
   explicit nsFormData(nsISupports* aOwner = nullptr);
 
--- a/dom/html/test/test_formData.html
+++ b/dom/html/test/test_formData.html
@@ -92,35 +92,50 @@ function testSet() {
   is(f.getAll("other").length, 1, "set() should replace existing entries.");
   is(f.getAll("other")[0], "value4", "set() should replace existing entries.");
 }
 
 function testIterate() {
   todo(false, "Implement this in Bug 1085284.");
 }
 
+function testFilename() {
+  var f = new FormData();
+  // Spec says if a Blob (which is not a File) is added, the name parameter is effectively ignored, always set to "blob".
+  f.append("blob", new Blob(["hi"]), "blob1.txt");
+  is(f.get("blob").name, "blob", "Blob's filename should be blob irrespective of what was passed.");
+
+  // Spec says if a File is added, the name parameter is respected if passed.
+  f.append("file1", new File(["hi"], "file1.txt"));
+  is(f.get("file1").name, "file1.txt", "File's filename should be original's name if no filename is explicitly passed.");
+  f.append("file2", new File(["hi"], "file2.txt"), "fakename.txt");
+  is(f.get("file2").name, "fakename.txt", "File's filename should be explicitly passed name.");
+  f.append("file3", new File(["hi"], ""));
+  is(f.get("file3").name, "", "File's filename is returned even if empty.");
+}
+
 function testSend() {
     var xhr = new XMLHttpRequest();
     xhr.open("POST", "form_submit_server.sjs");
     xhr.onload = function () {
         var response = xhr.response;
 
         for (var entry of response) {
             is(entry.body, 'hey');
             is(entry.headers['Content-Type'], 'text/plain');
         }
 
         is(response[0].headers['Content-Disposition'],
             'form-data; name="empty"; filename="blob"');
 
         is(response[1].headers['Content-Disposition'],
-            'form-data; name="explicit"; filename="explicit-file-name"');
+            'form-data; name="explicit"; filename="blob"');
 
         is(response[2].headers['Content-Disposition'],
-            'form-data; name="explicit-empty"; filename=""');
+            'form-data; name="explicit-empty"; filename="blob"');
 
         is(response[3].headers['Content-Disposition'],
             'form-data; name="file-name"; filename="testname"');
 
         is(response[4].headers['Content-Disposition'],
             'form-data; name="empty-file-name"; filename="blob"');
 
         is(response[5].headers['Content-Disposition'],
@@ -128,35 +143,39 @@ function testSend() {
 
         SimpleTest.finish();
     }
 
     var file, blob = new Blob(['hey'], {type: 'text/plain'});
 
     var fd = new FormData();
     fd.append("empty", blob);
-    fd.append("explicit", blob, "explicit-file-name");
+    fd.append("explicit", blob, "explicit-file-name-will-be-ignored");
     fd.append("explicit-empty", blob, "");
     file = new File([blob], 'testname',  {type: 'text/plain'});
     fd.append("file-name", file);
+    // XXXnsm Empty filename will get converted to "blob" when sending to
+    // server, which I guess is acceptable since no spec seems to define
+    // anything. Blink does the same.
     file = new File([blob], '',  {type: 'text/plain'});
     fd.append("empty-file-name", file);
     file = new File([blob], 'testname',  {type: 'text/plain'});
     fd.append("file-name-overwrite", file, "overwrite");
     xhr.responseType = 'json';
     xhr.send(fd);
 }
 
 function runTest() {
   testHas();
   testGet();
   testGetAll();
   testDelete();
   testSet();
   testIterate();
+  testFilename();
   // Finally, send an XHR and verify the response matches.
   testSend();
 }
 
 runTest()
 </script>
 </body>
 </html>