Merge pull request #161 from Workiva/default-request-encoding

CP-1966 Handle request with no encoding and no charset

CHANGELOG.md

@@ -1,5 +1,19 @@
 # Changelog
 
+## [2.6.1](https://github.com/Workvia/w_transport/compare/2.6.0...2.6.1)
+_TBD_
+
+- **Bug Fix:** A request's `encoding` property can no longer be set to null.
+  This would have most likely caused an RTE when the request was sent, so now an
+  `ArgumentError` will be thrown immediately.
+
+- **Bug Fix:** As of 2.6.0, if you were to set a request's content-type manually
+  without a charset or with an unknown charset, it was possible to hit an RTE
+  due to a null `encoding`. The `HttpBody` class has been updated to be more
+  resilient to a missing encoding or charset. Additionally, all request classes
+  will now pass in the value of its `encoding` property, which should now always
+  be non-null.
+
 ## [2.6.0](https://github.com/Workvia/w_transport/compare/2.5.1...2.6.0)
 _June 20, 2016_
 

lib/src/http/common/form_request.dart

@@ -70,6 +70,7 @@ abstract class CommonFormRequest extends CommonRequest implements FormRequest {
     if (body != null) {
       this.fields = body;
     }
-    return new HttpBody.fromBytes(contentType, _encodedQuery);
+    return new HttpBody.fromBytes(contentType, _encodedQuery,
+        fallbackEncoding: encoding);
   }
 }

lib/src/http/common/request.dart

@@ -197,6 +197,7 @@ abstract class CommonRequest extends Object
   /// will update the [contentType] `charset` parameter.
   set encoding(Encoding encoding) {
     verifyUnsent();
+    if (encoding == null) throw new ArgumentError.notNull('encoding');
     _encoding = encoding;
     if (!_contentTypeSetManually) {
       updateContentType(

lib/src/http/http_body.dart

@@ -56,21 +56,63 @@ class HttpBody extends BaseHttpBody {
   Encoding _encoding;
 
   /// Construct the body to an HTTP request or an HTTP response from bytes.
+  ///
+  /// If [encoding] is given, it will be used to encode/decode the body.
+  ///
+  /// Otherwise, the `charset` parameter from [contentType] will be mapped to an
+  /// Encoding supported by Dart.
+  ///
+  /// If a `charset` parameter is not available or the value is unrecognized,
+  /// [fallbackEncoding] will be used.
+  ///
+  /// If [fallbackEncoding] is `null`, UTF8 will be the default encoding.
+  ///
+  /// If an encoding cannot be parsed from the content-type header (via the
+  /// `charset` param), then [fallbackEncoding] will be used (UTF8 by default).
   HttpBody.fromBytes(MediaType this.contentType, List<int> bytes,
-      {Encoding fallbackEncoding}) {
-    _encoding = http_utils.parseEncodingFromContentType(contentType,
-        fallback: fallbackEncoding);
+      {Encoding encoding, Encoding fallbackEncoding}) {
+    if (fallbackEncoding == null) {
+      fallbackEncoding = UTF8;
+    }
+    if (encoding != null) {
+      _encoding = encoding;
+    } else {
+      _encoding = http_utils.parseEncodingFromContentType(contentType,
+          fallback: fallbackEncoding);
+    }
+
     if (bytes == null) {
       bytes = [];
     }
     _bytes = new Uint8List.fromList(bytes);
   }
 
   /// Construct the body to an HTTP request or an HTTP response from text.
+  ///
+  /// If [encoding] is given, it will be used to encode/decode the body.
+  ///
+  /// Otherwise, the `charset` parameter from [contentType] will be mapped to an
+  /// Encoding supported by Dart.
+  ///
+  /// If a `charset` parameter is not available or the value is unrecognized,
+  /// [fallbackEncoding] will be used.
+  ///
+  /// If [fallbackEncoding] is `null`, UTF8 will be the default encoding.
+  ///
+  /// If an encoding cannot be parsed from the content-type header (via the
+  /// `charset` param), then [fallbackEncoding] will be used (UTF8 by default).
   HttpBody.fromString(MediaType this.contentType, String body,
-      {Encoding fallbackEncoding}) {
-    _encoding = http_utils.parseEncodingFromContentType(contentType,
-        fallback: fallbackEncoding);
+      {Encoding encoding, Encoding fallbackEncoding}) {
+    if (fallbackEncoding == null) {
+      fallbackEncoding = UTF8;
+    }
+    if (encoding != null) {
+      _encoding = encoding;
+    } else {
+      _encoding = http_utils.parseEncodingFromContentType(contentType,
+          fallback: fallbackEncoding);
+    }
+
     if (body == null) {
       body = '';
     }
@@ -86,7 +128,7 @@ class HttpBody extends BaseHttpBody {
       var encoded;
       try {
         encoded = encoding.encode(_body);
-      } catch (e) {
+      } on ArgumentError {
         throw new ResponseFormatException(contentType, encoding, body: _body);
       }
       _bytes = new Uint8List.fromList(encoded);
@@ -99,7 +141,7 @@ class HttpBody extends BaseHttpBody {
     if (_body == null) {
       try {
         _body = encoding.decode(_bytes);
-      } catch (e) {
+      } on FormatException {
         throw new ResponseFormatException(contentType, encoding, bytes: _bytes);
       }
     }

lib/src/http/response_format_exception.dart

@@ -50,9 +50,10 @@ class ResponseFormatException implements Exception {
       bodyLine = 'Bytes: $bytes';
     }
 
-    String msg = description;
+    var msg = description;
+    var encodingName = encoding != null ? encoding.name : 'null';
     msg += '\n\tContent-Type: $contentType';
-    msg += '\n\tEncoding: ${encoding.name}';
+    msg += '\n\tEncoding: $encodingName';
     msg += '\n\t$bodyLine';
 
     return msg;

test/unit/http/form_request_test.dart

@@ -95,6 +95,13 @@ void main() {
         }, throwsUnsupportedError);
       });
 
+      test('setting encoding to null should throw', () {
+        var request = new FormRequest();
+        expect(() {
+          request.encoding = null;
+        }, throwsArgumentError);
+      });
+
       test('setting encoding should update content-type', () {
         FormRequest request = new FormRequest();
         expect(request.contentType.parameters['charset'], equals(UTF8.name));
@@ -143,6 +150,15 @@ void main() {
         }, throwsStateError);
       });
 
+      test('custom content-type without inferrable encoding', () async {
+        Uri uri = Uri.parse('/test');
+        MockTransports.http.expect('POST', uri);
+        var request = new FormRequest()
+          ..contentType = new MediaType('application', 'x-custom')
+          ..fields['foo'] = 'bar';
+        await request.post(uri: uri);
+      });
+
       test('clone()', () {
         var fields = {'f1': 'v1', 'f2': 'v2'};
         FormRequest orig = new FormRequest()..fields = fields;

test/unit/http/http_body_test.dart

@@ -36,18 +36,51 @@ void main() {
         expect(body.asBytes(), isEmpty);
       });
 
+      test('should use encoding if one is explicitly given', () {
+        var contentType = new MediaType('text', 'plain');
+
+        var stringBody =
+            new HttpBody.fromString(contentType, 'body', encoding: ASCII);
+        expect(stringBody.encoding, equals(ASCII));
+
+        var bytesBody = new HttpBody.fromBytes(contentType, UTF8.encode('body'),
+            encoding: ASCII);
+        expect(bytesBody.encoding, equals(ASCII));
+      });
+
       test('should parse encoding from content-type', () {
-        MediaType contentType =
+        var contentType =
             new MediaType('text', 'plain', {'charset': ASCII.name});
-        HttpBody body = new HttpBody.fromString(contentType, 'body');
-        expect(body.encoding.name, equals(ASCII.name));
+
+        var stringBody = new HttpBody.fromString(contentType, 'body');
+        expect(stringBody.encoding, equals(ASCII));
+
+        var bytesBody =
+            new HttpBody.fromBytes(contentType, UTF8.encode('body'));
+        expect(bytesBody.encoding, equals(ASCII));
       });
 
       test('should allow a fallback encoding', () {
-        MediaType contentType = new MediaType('text', 'plain');
-        HttpBody body = new HttpBody.fromString(contentType, 'body',
-            fallbackEncoding: LATIN1);
-        expect(body.encoding.name, equals(LATIN1.name));
+        var contentType = new MediaType('text', 'plain');
+
+        var stringBody = new HttpBody.fromString(contentType, 'body',
+            fallbackEncoding: ASCII);
+        expect(stringBody.encoding, equals(ASCII));
+
+        var bytesBody = new HttpBody.fromBytes(contentType, UTF8.encode('body'),
+            fallbackEncoding: ASCII);
+        expect(bytesBody.encoding, equals(ASCII));
+      });
+
+      test('should use UTF8 by default', () {
+        var contentType = new MediaType('text', 'plain');
+
+        var stringBody = new HttpBody.fromString(contentType, 'body');
+        expect(stringBody.encoding, equals(UTF8));
+
+        var bytesBody =
+            new HttpBody.fromBytes(contentType, UTF8.encode('body'));
+        expect(bytesBody.encoding, equals(UTF8));
       });
 
       test('content-length should be calculated automaticlaly', () {

test/unit/http/json_request_test.dart

@@ -121,6 +121,13 @@ void main() {
         }, throwsUnsupportedError);
       });
 
+      test('setting encoding to null should throw', () {
+        var request = new JsonRequest();
+        expect(() {
+          request.encoding = null;
+        }, throwsArgumentError);
+      });
+
       test('setting encoding should update content-type', () {
         JsonRequest request = new JsonRequest();
         expect(request.contentType.parameters['charset'], equals(UTF8.name));
@@ -169,6 +176,15 @@ void main() {
         }, throwsStateError);
       });
 
+      test('custom content-type without inferrable encoding', () async {
+        Uri uri = Uri.parse('/test');
+        MockTransports.http.expect('POST', uri);
+        var request = new JsonRequest()
+          ..contentType = new MediaType('application', 'x-custom')
+          ..body = {'foo': 'bar'};
+        await request.post(uri: uri);
+      });
+
       test('clone()', () {
         var body = [
           {'f1': 'v1', 'f2': 'v2'}

test/unit/http/plain_text_request_test.dart

@@ -117,6 +117,13 @@ void main() {
         }, throwsUnsupportedError);
       });
 
+      test('setting encoding to null should throw', () {
+        var request = new Request();
+        expect(() {
+          request.encoding = null;
+        }, throwsArgumentError);
+      });
+
       test('setting encoding should update content-type', () {
         Request request = new Request();
         expect(request.contentType.parameters['charset'], equals(UTF8.name));
@@ -165,6 +172,15 @@ void main() {
         }, throwsStateError);
       });
 
+      test('custom content-type without inferrable encoding', () async {
+        Uri uri = Uri.parse('/test');
+        MockTransports.http.expect('POST', uri);
+        var request = new Request()
+          ..contentType = new MediaType('application', 'x-custom')
+          ..body = 'body';
+        await request.post(uri: uri);
+      });
+
       test('clone()', () {
         var body = 'body';
         Request orig = new Request()..body = body;

test/unit/http/response_format_exception_test.dart

@@ -0,0 +1,71 @@
+// Copyright 2015 Workiva Inc.
+//
+// Licensed under the Apache License, Version 2.0 (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.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+library w_transport.test.unit.http.response_format_exception_test;
+
+import 'dart:convert';
+
+import 'package:http_parser/http_parser.dart' show MediaType;
+import 'package:test/test.dart';
+
+import 'package:w_transport/src/http/response_format_exception.dart';
+
+import '../../naming.dart';
+
+void main() {
+  Naming naming = new Naming()
+    ..testType = testTypeUnit
+    ..topic = topicHttp;
+
+  group(naming.toString(), () {
+    group('ResponseFormatException', () {
+      test('should detail why bytes could not be decoded', () {
+        var bytes = UTF8.encode('bodyçå®');
+        var contentType =
+            new MediaType('application', 'json', {'charset': ASCII.name});
+        var exception =
+            new ResponseFormatException(contentType, ASCII, bytes: bytes);
+        expect(exception.toString(), contains('Bytes could not be decoded'));
+        expect(exception.toString(), contains('Content-Type: $contentType'));
+        expect(exception.toString(), contains('Encoding: ${ASCII.name}'));
+        expect(
+            exception.toString(), contains(UTF8.encode('bodyçå®').toString()));
+      });
+
+      test('should detail why string could not be encoded', () {
+        var body = 'bodyçå®';
+        var contentType =
+            new MediaType('application', 'json', {'charset': ASCII.name});
+        var exception =
+            new ResponseFormatException(contentType, ASCII, body: body);
+        expect(exception.toString(), contains('Body could not be encoded'));
+        expect(exception.toString(), contains('Content-Type: $contentType'));
+        expect(exception.toString(), contains('Encoding: ${ASCII.name}'));
+        expect(exception.toString(), contains('bodyçå®'));
+      });
+
+      test('should warn if encoding is null', () {
+        var body = 'bodyçå®';
+        var contentType =
+            new MediaType('application', 'json', {'charset': ASCII.name});
+        var exception =
+            new ResponseFormatException(contentType, null, body: body);
+        expect(exception.toString(), contains('Body could not be encoded'));
+        expect(exception.toString(), contains('Content-Type: $contentType'));
+        expect(exception.toString(), contains('Encoding: null'));
+        expect(exception.toString(), contains('bodyçå®'));
+      });
+    });
+  });
+}

test/unit/http/streamed_request_test.dart

@@ -104,6 +104,13 @@ void main() {
         }, throwsStateError);
       });
 
+      test('setting encoding to null should throw', () {
+        var request = new StreamedRequest();
+        expect(() {
+          request.encoding = null;
+        }, throwsArgumentError);
+      });
+
       test('setting encoding should update content-type', () {
         StreamedRequest request = new StreamedRequest();
         expect(request.contentType.parameters['charset'], equals(UTF8.name));
@@ -152,6 +159,17 @@ void main() {
         }, throwsStateError);
       });
 
+      test('custom content-type without inferrable encoding', () async {
+        Uri uri = Uri.parse('/test');
+        MockTransports.http.expect('POST', uri);
+        var request = new StreamedRequest()
+          ..contentType = new MediaType('application', 'x-custom')
+          ..body = new Stream.fromIterable([
+            [1, 2]
+          ]);
+        await request.post(uri: uri);
+      });
+
       test('clone()', () {
         StreamedRequest request = new StreamedRequest();
         expect(request.clone, throwsUnsupportedError);