Wrong filename on file upload

url/third_party/mozilla/url_parse.cc

 /* Based on nsURLParsers.cc from Mozilla
  * -------------------------------------
  * The contents of this file are subject to the Mozilla Public License Version
  * 1.1 (the "License"); you may not use this file except in compliance with
  * the License. You may obtain a copy of the License at
  * http://www.mozilla.org/MPL/
  *
  * Software distributed under the License is distributed on an "AS IS" basis,
  * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
  * for the specific language governing rights and limitations under the
@@ skipping 608 matching lines @@
   if (!path.is_nonempty()) {
     file_name->reset();
     return;
   }
 
   // Search backwards for a parameter, which is a normally unused field in a
   // URL delimited by a semicolon. We parse the parameter as part of the
   // 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);
   return;
 }
 
 template<typename CHAR>
 bool DoExtractQueryKeyValue(const CHAR* spec,
                             Component* query,
                             Component* key,
@@ skipping 278 matching lines @@
 }
 
 void ParseAfterScheme(const base::char16* spec,
                       int spec_len,
                       int after_scheme,
                       Parsed* parsed) {
   DoParseAfterScheme(spec, spec_len, after_scheme, parsed);
 }
 
 }  // namespace url