Wrong filename on file upload

url/third_party/mozilla/url_parse.cc

Index: url/third_party/mozilla/url_parse.cc
diff --git a/url/third_party/mozilla/url_parse.cc b/url/third_party/mozilla/url_parse.cc
index 62567969a6502b1d5bf4a781e078ca778f462fd9..acdcf12535b2bea8f97517a0372d22b304bdbd21 100644
--- a/url/third_party/mozilla/url_parse.cc
+++ b/url/third_party/mozilla/url_parse.cc
@@ -626,24 +626,21 @@ void DoExtractFileName(const CHAR* spec,
   // path, but here, we don't want to count it. The last semicolon is the
   // parameter. The path should start with a slash, so we don't need to check
   // the first one.
+  bool is_file_end_fixed = false;
   int file_end = path.end();
-  for (int i = path.end() - 1; i > path.begin; i--) {
-    if (spec[i] == ';') {
+  for (int i = path.end() - 1; i >= path.begin; i--) {
+    if (!is_file_end_fixed && spec[i] == ';') {

[brettw, 2014/09/16 22:37:10]

It seems that this behavior takes everything between the last slash and (if it
exists) the last semicolon. Are we sure it's not supposed to be the first
semicolon?

i.e. given "foo/bar;baz;lala" your patch returns "bar;baz" and I'm wondering if
it shouldn't be "bar". I think "bar" would also be easier to code since you
could remove the is_file_end_fixed flag and just change file_end (this would
also have the effect of trimming parameters for the "degenerate" case at the
bottom of this function, which I guess is good.

[arun87.kumar, 2014/09/17 11:39:12]

i too felt that incase of "foo/bar;baz;lala", the filename should be "bar".
But as per one of the already existing unit testcases (9th case) in
url_parse_unittest.cc, the input and output were as below.
{"http://www.google.com/foo/bar.html;foo;param#ref", "bar.html;foo"}.

So i had added is_file_end_fixed logic to pass this test. Also same behaviour
was seeen in other browsers like Firefox and IE.

Anyways, now i have modified the patch to exclude semicolon and its subsequent
string from filename.

       file_end = i;
-      break;
-    }
-  }
-
-  // Now search backwards from the filename end to the previous slash
-  // to find the beginning of the filename.
-  for (int i = file_end - 1; i >= path.begin; i--) {
-    if (IsURLSlash(spec[i])) {
+      is_file_end_fixed = true;
+    }else if (IsURLSlash(spec[i])) {

[brettw, 2014/09/16 22:37:10]

Space before "else"

[arun87.kumar, 2014/09/17 11:39:12]

Done.

+      // search backwards from the filename end to the previous slash

[brettw, 2014/09/16 22:37:09]

This comment should be updated (this code no longer does any searching).

[arun87.kumar, 2014/09/17 11:39:12]

Done.

+      // to find the beginning of the filename.
       // File name is everything following this character to the end
       *file_name = MakeRange(i + 1, file_end);
       return;
     }
   }
-
+  

[brettw, 2014/09/16 22:37:09]

Watch out for extra spaces.

[arun87.kumar, 2014/09/17 11:39:12]

Done.

   // No slash found, this means the input was degenerate (generally paths
   // will start with a slash). Let's call everything the file name.
   *file_name = MakeRange(path.begin, file_end);