Bug 1321640: Part 2 - Clean up the header block and param parsing code. r?mixedpuppy draft
authorKris Maglione <maglione.k@gmail.com>
Thu, 01 Dec 2016 13:11:08 -0800
changeset 446871 d26cd5daa9da149e362b8668154db07c74c84fc9
parent 446870 6c0e22e89a3274cb28717651b5e5703880f8473e
child 538897 e4a67d01e75a755bdd8dda8583b20513a40298c9
push id37913
push usermaglione.k@gmail.com
push dateFri, 02 Dec 2016 02:15:14 +0000
reviewersmixedpuppy
bugs1321640
milestone53.0a1
Bug 1321640: Part 2 - Clean up the header block and param parsing code. r?mixedpuppy MozReview-Commit-ID: 9fI5JQWCVop
toolkit/modules/addons/WebRequestUpload.jsm
--- a/toolkit/modules/addons/WebRequestUpload.jsm
+++ b/toolkit/modules/addons/WebRequestUpload.jsm
@@ -8,34 +8,121 @@ const EXPORTED_SYMBOLS = ["WebRequestUpl
 
 /* exported WebRequestUpload */
 
 const Ci = Components.interfaces;
 const Cc = Components.classes;
 const Cu = Components.utils;
 const Cr = Components.results;
 
+Cu.importGlobalProperties(["TextEncoder"]);
+
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
 Cu.import("resource://gre/modules/ExtensionUtils.jsm");
 
 const {
   DefaultMap,
 } = ExtensionUtils;
 
+XPCOMUtils.defineLazyServiceGetter(this, "mimeHeader", "@mozilla.org/network/mime-hdrparam;1",
+                                   "nsIMIMEHeaderParam");
+
 const BinaryInputStream = Components.Constructor(
   "@mozilla.org/binaryinputstream;1", "nsIBinaryInputStream",
   "setInputStream");
 const ConverterInputStream = Components.Constructor(
   "@mozilla.org/intl/converter-input-stream;1", "nsIConverterInputStream",
   "init");
 
 var WebRequestUpload;
 
 /**
+ * Parses the given raw header block, and stores the value of each
+ * lower-cased header name in the resulting map.
+ */
+class Headers extends Map {
+  constructor(headerText) {
+    super();
+
+    if (headerText) {
+      this.parseHeaders(headerText);
+    }
+  }
+
+  parseHeaders(headerText) {
+    let lines = headerText.split("\r\n");
+
+    let lastHeader;
+    for (let line of lines) {
+      // The first empty line indicates the end of the header block.
+      if (line === "") {
+        return;
+      }
+
+      // Lines starting with whitespace are appended to the previous
+      // header.
+      if (/^\s/.test(line)) {
+        if (lastHeader) {
+          let val = this.get(lastHeader);
+          this.set(lastHeader, `${val}\r\n${line}`);
+        }
+        continue;
+      }
+
+      let match = /^(.*?)\s*:\s+(.*)/.exec(line);
+      if (match) {
+        lastHeader = match[1].toLowerCase();
+        this.set(lastHeader, match[2]);
+      }
+    }
+  }
+
+  /**
+   * If the given header exists, and contains the given parameter,
+   * returns the value of that parameter.
+   *
+   * @param {string} name
+   *        The lower-cased header name.
+   * @param {string} paramName
+   *        The name of the parameter to retrieve, or empty to retrieve
+   *        the first (possibly unnamed) parameter.
+   * @returns {string | null}
+   */
+  getParam(name, paramName) {
+    return Headers.getParam(this.get(name), paramName);
+  }
+
+  /**
+   * If the given header value is non-null, and contains the given
+   * parameter, returns the value of that parameter.
+   *
+   * @param {string | null} header
+   *        The text of the header from which to retrieve the param.
+   * @param {string} paramName
+   *        The name of the parameter to retrieve, or empty to retrieve
+   *        the first (possibly unnamed) parameter.
+   * @returns {string | null}
+   */
+  static getParam(header, paramName) {
+    if (header) {
+      // The service expects this to be a raw byte string, so convert to
+      // UTF-8.
+      let bytes = new TextEncoder().encode(header);
+      let binHeader = String.fromCharCode(...bytes);
+
+      return mimeHeader.getParameter(binHeader, paramName, null,
+                                     false, {});
+    }
+
+    return null;
+  }
+}
+
+/**
  * Creates a new Object with a corresponding property for every
  * key-value pair in the given Map.
  *
  * @param {Map} map
  *        The map to convert.
  * @returns {Object}
  */
 function mapToObject(map) {
@@ -218,50 +305,50 @@ function parseFormData(stream, channel, 
    *        The (possibly multi-part) stream to parse.
    * @param {string} boundary
    *        The boundary at which to split the parts.
    * @returns {Map<string, Array<string>>}
    */
   function parseMultiPart(stream, boundary) {
     let formData = new DefaultMap(() => []);
 
-    let unslash = str => str.replace(/\\"/g, '"');
-
     for (let part of getParts(stream, boundary, "\r\n")) {
+      if (part === "") {
+        // The first part will always be empty.
+        continue;
+      }
       if (part === "--\r\n") {
+        // This indicates the end of the stream.
         break;
       }
 
-      let match = part.match(/^\r\nContent-Disposition: form-data; name="(.*)"\r\n(?:Content-Type: (\S+))?.*\r\n/i);
-      if (!match) {
-        continue;
+      let end = part.indexOf("\r\n\r\n");
+
+      // All valid parts must begin with \r\n, and we can't process form
+      // fields without any header block.
+      if (!part.startsWith("\r\n") || end <= 0) {
+        throw new Error("Invalid MIME stream");
       }
 
-      let [header, name, contentType] = match;
-      let value = "";
-      if (contentType) {
-        let fileName;
-        // Since escaping inside Content-Disposition subfields is still poorly defined and buggy (see Bug 136676),
-        // currently we always consider backslash-prefixed quotes as escaped even if that's not generally true
-        // (i.e. in a field whose value actually ends with a backslash).
-        // Therefore in this edge case we may end coalescing name and filename, which is marginally better than
-        // potentially truncating the name field at the wrong point, at least from a XSS filter POV.
-        match = name.match(/^(.*[^\\])"; filename="(.*)/);
-        if (match) {
-          [, name, fileName] = match;
-        }
+      let content = part.slice(end + 4);
+      let headerText = part.slice(2, end);
+      let headers = new Headers(headerText);
 
-        if (fileName) {
-          value = unslash(fileName);
-        }
-      } else {
-        value = part.slice(header.length);
+      let name = headers.getParam("content-disposition", "name");
+      if (!name || headers.getParam("content-disposition", "") !== "form-data") {
+        throw new Error("Invalid MIME stream");
       }
 
-      formData.get(unslash(name)).push(value);
+      if (headers.has("content-type")) {
+        // For file upload fields, we return the filename, rather than the
+        // file data.
+        let filename = headers.getParam("content-disposition", "filename");
+        content = filename || "";
+      }
+      formData.get(name).push(content);
     }
 
     return formData;
   }
 
   /**
    * Parses the given stream as x-www-form-urlencoded, and returns a map of its fields.
    *
@@ -299,30 +386,26 @@ function parseFormData(stream, channel, 
       rewind(stream);
       stream = stream.data;
     }
 
     let contentType;
     try {
       contentType = channel.getRequestHeader("Content-Type");
     } catch (e) {
-      let match = /^Content-Type:\s+(.+)/i.exec(headers);
-      contentType = match && match[1];
+      contentType = new Headers(headers).get("content-type");
     }
 
-    let match = /^(?:multipart\/form-data;\s*boundary=(\S*)|(application\/x-www-form-urlencoded))/i.exec(contentType);
-    if (match) {
-      let boundary = match[1];
-      if (boundary) {
+    switch (Headers.getParam(contentType, "")) {
+      case "multipart/form-data":
+        let boundary = Headers.getParam(contentType, "boundary");
         return parseMultiPart(stream, `\r\n--${boundary}`);
-      }
 
-      if (match[2]) {
+      case "application/x-www-form-urlencoded":
         return parseUrlEncoded(stream);
-      }
     }
   } finally {
     for (let stream of touchedStreams) {
       rewind(stream);
     }
   }
 
   return null;