Make MultipartRequest more closely adhere to browsers' behavior.

[email protected]
BUG=17899

Review URL: https://codereview.chromium.org//218993016

git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@34631 260f80e4-7a28-3924-810f-c04153c831b5

Author: nex3

pkg/http/CHANGELOG.md

@@ -7,3 +7,5 @@
   positional.
 
 * Make request headers case-insensitive.
+
+* Make `MultipartRequest` more closely adhere to browsers' encoding conventions.

pkg/http/lib/src/multipart_request.dart

@@ -13,6 +13,8 @@ import 'byte_stream.dart';
 import 'multipart_file.dart';
 import 'utils.dart';
 
+final _newlineRegExp = new RegExp(r"\r\n|\r|\n");
+
 /// A `multipart/form-data` request. Such a request has both string [fields],
 /// which function as normal form fields, and (potentially streamed) binary
 /// [files].
@@ -61,13 +63,13 @@ class MultipartRequest extends BaseRequest {
 
     fields.forEach((name, value) {
       length += "--".length + _BOUNDARY_LENGTH + "\r\n".length +
-          _headerForField(name, value).length +
+          UTF8.encode(_headerForField(name, value)).length +
           UTF8.encode(value).length + "\r\n".length;
     });
 
     for (var file in _files) {
       length += "--".length + _BOUNDARY_LENGTH + "\r\n".length +
-          _headerForFile(file).length +
+          UTF8.encode(_headerForFile(file)).length +
           file.length + "\r\n".length;
     }
 
@@ -91,8 +93,7 @@ class MultipartRequest extends BaseRequest {
     var controller = new StreamController<List<int>>(sync: true);
 
     void writeAscii(String string) {
-      assert(isPlainAscii(string));
-      controller.add(string.codeUnits);
+      controller.add(UTF8.encode(string));
     }
 
     writeUtf8(String string) => controller.add(UTF8.encode(string));
@@ -133,11 +134,8 @@ class MultipartRequest extends BaseRequest {
   /// Returns the header string for a field. The return value is guaranteed to
   /// contain only ASCII characters.
   String _headerForField(String name, String value) {
-    // http://tools.ietf.org/html/rfc2388 mandates some complex encodings for
-    // field names and file names, but in practice user agents seem to just
-    // URL-encode them so we do the same.
     var header =
-        'content-disposition: form-data; name="${Uri.encodeFull(name)}"';
+        'content-disposition: form-data; name="${_browserEncode(name)}"';
     if (!isPlainAscii(value)) {
       header = '$header\r\ncontent-type: text/plain; charset=utf-8';
     }
@@ -148,14 +146,24 @@ class MultipartRequest extends BaseRequest {
   /// contain only ASCII characters.
   String _headerForFile(MultipartFile file) {
     var header = 'content-type: ${file.contentType}\r\n'
-      'content-disposition: form-data; name="${Uri.encodeFull(file.field)}"';
+      'content-disposition: form-data; name="${_browserEncode(file.field)}"';
 
     if (file.filename != null) {
-      header = '$header; filename="${Uri.encodeFull(file.filename)}"';
+      header = '$header; filename="${_browserEncode(file.filename)}"';
     }
     return '$header\r\n\r\n';
   }
 
+  /// Encode [value] in the same way browsers do.
+  String _browserEncode(String value) {
+    // http://tools.ietf.org/html/rfc2388 mandates some complex encodings for
+    // field names and file names, but in practice user agents seem not to
+    // follow this at all. Instead, they URL-encode `\r`, `\n`, and `\r\n` as
+    // `\r\n`; URL-encode `"`; and do nothing else (even for `%` or non-ASCII
+    // characters). We follow their behavior.
+    return value.replaceAll(_newlineRegExp, "%0D%0A").replaceAll('"', "%22");
+  }
+
   /// Returns a randomly-generated multipart boundary string
   String _boundaryString() {
     var prefix = "dart-http-boundary-";

pkg/http/test/multipart_test.dart

@@ -93,7 +93,33 @@ void main() {
 
     expect(request, bodyMatches('''
         --{{boundary}}
-        content-disposition: form-data; name="f%C3%AF%C4%93ld"
+        content-disposition: form-data; name="fïēld"
+
+        value
+        --{{boundary}}--
+        '''));
+  });
+
+  test('with a field name with newlines', () {
+    var request = new http.MultipartRequest('POST', dummyUrl);
+    request.fields['foo\nbar\rbaz\r\nbang'] = 'value';
+
+    expect(request, bodyMatches('''
+        --{{boundary}}
+        content-disposition: form-data; name="foo%0D%0Abar%0D%0Abaz%0D%0Abang"
+
+        value
+        --{{boundary}}--
+        '''));
+  });
+
+  test('with a field name with a quote', () {
+    var request = new http.MultipartRequest('POST', dummyUrl);
+    request.fields['foo"bar'] = 'value';
+
+    expect(request, bodyMatches('''
+        --{{boundary}}
+        content-disposition: form-data; name="foo%22bar"
 
         value
         --{{boundary}}--
@@ -122,7 +148,37 @@ void main() {
     expect(request, bodyMatches('''
         --{{boundary}}
         content-type: text/plain; charset=utf-8
-        content-disposition: form-data; name="file"; filename="f%C3%AFl%C4%93name.txt"
+        content-disposition: form-data; name="file"; filename="fïlēname.txt"
+
+        contents
+        --{{boundary}}--
+        '''));
+  });
+
+  test('with a filename with newlines', () {
+    var request = new http.MultipartRequest('POST', dummyUrl);
+    request.files.add(new http.MultipartFile.fromString('file', 'contents',
+        filename: 'foo\nbar\rbaz\r\nbang'));
+
+    expect(request, bodyMatches('''
+        --{{boundary}}
+        content-type: text/plain; charset=utf-8
+        content-disposition: form-data; name="file"; filename="foo%0D%0Abar%0D%0Abaz%0D%0Abang"
+
+        contents
+        --{{boundary}}--
+        '''));
+  });
+
+  test('with a filename with a quote', () {
+    var request = new http.MultipartRequest('POST', dummyUrl);
+    request.files.add(new http.MultipartFile.fromString('file', 'contents',
+        filename: 'foo"bar'));
+
+    expect(request, bodyMatches('''
+        --{{boundary}}
+        content-type: text/plain; charset=utf-8
+        content-disposition: form-data; name="file"; filename="foo%22bar"
 
         contents
         --{{boundary}}--