diff --git a/pkgs/cronet_http/example/pubspec.yaml b/pkgs/cronet_http/example/pubspec.yaml index dfaa39a8e7..79fc4fa89c 100644 --- a/pkgs/cronet_http/example/pubspec.yaml +++ b/pkgs/cronet_http/example/pubspec.yaml @@ -29,3 +29,9 @@ dev_dependencies: flutter: uses-material-design: true + +# Use the path version of `package:http` so that the behavior matches the +# expectations in http_client_conformance_tests. +dependency_overrides: + http: + path: ../../http/ diff --git a/pkgs/cupertino_http/example/pubspec.yaml b/pkgs/cupertino_http/example/pubspec.yaml index 38f75b0cf9..2ee3fba734 100644 --- a/pkgs/cupertino_http/example/pubspec.yaml +++ b/pkgs/cupertino_http/example/pubspec.yaml @@ -38,3 +38,9 @@ dev_dependencies: flutter: uses-material-design: true + +# Use the path version of `package:http` so that the behavior matches the +# expectations in http_client_conformance_tests. +dependency_overrides: + http: + path: ../../http/ diff --git a/pkgs/http/CHANGELOG.md b/pkgs/http/CHANGELOG.md index 6a2698a7f2..b3bc7e031f 100644 --- a/pkgs/http/CHANGELOG.md +++ b/pkgs/http/CHANGELOG.md @@ -1,3 +1,8 @@ +## 2.0.0-wip + +* **Breaking** Change the behavior of `Request.body` so that a charset + parameter is only added for text and XML media types. + ## 1.5.0 * Fixed a bug in `IOClient` where the `HttpClient`'s response stream was diff --git a/pkgs/http/lib/src/request.dart b/pkgs/http/lib/src/request.dart index b7e56ab559..0c856b0dbd 100644 --- a/pkgs/http/lib/src/request.dart +++ b/pkgs/http/lib/src/request.dart @@ -14,6 +14,22 @@ import 'utils.dart'; /// An HTTP request where the entire request body is known in advance. class Request extends BaseRequest { + /// Whether the given MIME type should have a 'charset' parameter. + bool _shouldHaveCharset(MediaType? contentType) => + contentType != null && + // RFC 8259, section 9 says that "charset" is not defined for JSON. + // Some uncommon non-text, non-xml types do specify charset + // (e.g. application/news-checkgroups) but the user will have to set the + // charset themselves for those types. + (contentType.type == 'text' || + // XML media types defined by RFC 7303. + // Note that some media types (e.g. cda+xml) specify that the + // charset, when present, must be utf-8. We do not enforce this. + contentType.mimeType == 'application/xml' || + contentType.mimeType == 'application/xml-external-parsed-entity' || + contentType.mimeType == 'application/xml-dtd' || + contentType.mimeType.endsWith('+xml')); + /// The size of the request body, in bytes. This is calculated from /// [bodyBytes]. /// @@ -61,7 +77,9 @@ class Request extends BaseRequest { _checkFinalized(); _defaultEncoding = value; var contentType = _contentType; - if (contentType == null) return; + if (contentType == null || !contentType.parameters.containsKey('charset')) { + return; + } _contentType = contentType.change(parameters: {'charset': value.name}); } @@ -93,20 +111,27 @@ class Request extends BaseRequest { /// This is converted to and from [bodyBytes] using [encoding]. /// /// When this is set, if the request does not yet have a `Content-Type` - /// header, one will be added with the type `text/plain`. Then the `charset` - /// parameter of the `Content-Type` header (whether new or pre-existing) will - /// be set to [encoding] if it wasn't already set. + /// header, one will be added with the type `text/plain` and appropriate + /// `charset` parameter. + /// + /// If request has `Content-Type` header with MIME media type name `text` or + /// is an XML MIME type (e.g. `application/xml` or `image/svg+xml`) without + /// `charset` parameter, then the `charset` parameter will be set to + /// [encoding]. /// - /// To set the body of the request, without setting the `Content-Type` header, - /// use [bodyBytes]. + /// To set the body of the request, without changing the `Content-Type` + /// header, use [bodyBytes]. String get body => encoding.decode(bodyBytes); set body(String value) { + // IANA defines known media types here: + // https://www.iana.org/assignments/media-types/media-types.xhtml bodyBytes = encoding.encode(value); var contentType = _contentType; if (contentType == null) { _contentType = MediaType('text', 'plain', {'charset': encoding.name}); - } else if (!contentType.parameters.containsKey('charset')) { + } else if (_shouldHaveCharset(_contentType) && + !contentType.parameters.containsKey('charset')) { _contentType = contentType.change(parameters: {'charset': encoding.name}); } } diff --git a/pkgs/http/pubspec.yaml b/pkgs/http/pubspec.yaml index f17bcb848b..9760b4685e 100644 --- a/pkgs/http/pubspec.yaml +++ b/pkgs/http/pubspec.yaml @@ -1,5 +1,5 @@ name: http -version: 1.5.0 +version: 2.0.0-wip description: A composable, multi-platform, Future-based API for HTTP requests. repository: https://github.com/dart-lang/http/tree/master/pkgs/http diff --git a/pkgs/http/test/io/http_test.dart b/pkgs/http/test/io/http_test.dart index 3f9aad815e..d4c19bd987 100644 --- a/pkgs/http/test/io/http_test.dart +++ b/pkgs/http/test/io/http_test.dart @@ -158,9 +158,7 @@ void main() { 'method': 'POST', 'path': '/', 'headers': { - 'content-type': [ - 'application/x-www-form-urlencoded; charset=utf-8' - ], + 'content-type': ['application/x-www-form-urlencoded'], 'content-length': ['40'], 'accept-encoding': ['gzip'], 'user-agent': ['Dart'], @@ -273,9 +271,7 @@ void main() { 'method': 'PUT', 'path': '/', 'headers': { - 'content-type': [ - 'application/x-www-form-urlencoded; charset=utf-8' - ], + 'content-type': ['application/x-www-form-urlencoded'], 'content-length': ['40'], 'accept-encoding': ['gzip'], 'user-agent': ['Dart'], @@ -388,9 +384,7 @@ void main() { 'method': 'PATCH', 'path': '/', 'headers': { - 'content-type': [ - 'application/x-www-form-urlencoded; charset=utf-8' - ], + 'content-type': ['application/x-www-form-urlencoded'], 'content-length': ['40'], 'accept-encoding': ['gzip'], 'user-agent': ['Dart'], diff --git a/pkgs/http/test/request_test.dart b/pkgs/http/test/request_test.dart index 59cb0988c5..a02d0b5820 100644 --- a/pkgs/http/test/request_test.dart +++ b/pkgs/http/test/request_test.dart @@ -189,23 +189,12 @@ void main() { expect(request.headers, containsPair('content-type', 'application/json')); }); - test( - 'is set to application/x-www-form-urlencoded with charset utf-8 if ' - 'bodyFields is set', () { - var request = http.Request('POST', dummyUrl) - ..bodyFields = {'hello': 'world'}; - expect(request.headers['Content-Type'], - equals('application/x-www-form-urlencoded; charset=utf-8')); - }); - - test( - 'is set to application/x-www-form-urlencoded with the given charset ' - 'if bodyFields and encoding are set', () { + test('is set to application/x-www-form-urlencoded if bodyFields is set', + () { var request = http.Request('POST', dummyUrl) - ..encoding = latin1 ..bodyFields = {'hello': 'world'}; expect(request.headers['Content-Type'], - equals('application/x-www-form-urlencoded; charset=iso-8859-1')); + equals('application/x-www-form-urlencoded')); }); test( @@ -218,20 +207,66 @@ void main() { equals('text/plain; charset=iso-8859-1')); }); - test('is modified to include utf-8 if body is set', () { + test('is modified to include utf-8 if body is set and mime type is text', + () { + var request = http.Request('POST', dummyUrl); + request.headers['Content-Type'] = 'text/plain'; + request.body = 'Hello World!'; + expect( + request.headers['Content-Type'], equals('text/plain; charset=utf-8')); + }); + + test('is modified to include utf-8 if body is set and mime type is xml', + () { + var request = http.Request('POST', dummyUrl); + request.headers['Content-Type'] = 'application/xml'; + request.body = ' channel) async { dynamic requestBody; if (requestBodyBytes.isEmpty) { requestBody = null; - } else if (request.headers.contentType?.charset != null) { - var encoding = - requiredEncodingForCharset(request.headers.contentType!.charset!); - requestBody = encoding.decode(requestBodyBytes); } else { - requestBody = requestBodyBytes; + requestBody = switch (( + request.headers.contentType?.mimeType, + request.headers.contentType?.charset + )) { + (_, var charset?) => + requiredEncodingForCharset(charset).decode(requestBodyBytes), + // This is not a complete set of mime types that default to utf-8, + // just the ones found in the tests. + ('application/json' || 'application/x-www-form-urlencoded', null) => + utf8.decode(requestBodyBytes), + _ => requestBodyBytes, + }; } - final headers = >{}; request.headers.forEach((name, values) { diff --git a/pkgs/http_client_conformance_tests/lib/src/request_body_tests.dart b/pkgs/http_client_conformance_tests/lib/src/request_body_tests.dart index 98da3785a3..2af53c6e78 100644 --- a/pkgs/http_client_conformance_tests/lib/src/request_body_tests.dart +++ b/pkgs/http_client_conformance_tests/lib/src/request_body_tests.dart @@ -78,8 +78,7 @@ void testRequestBody(Client client) { final serverReceivedContentType = await httpServerQueue.next; final serverReceivedBody = await httpServerQueue.next; - expect(serverReceivedContentType, - ['application/x-www-form-urlencoded; charset=utf-8']); + expect(serverReceivedContentType, ['application/x-www-form-urlencoded']); expect(serverReceivedBody, 'key=value'); }); @@ -90,8 +89,7 @@ void testRequestBody(Client client) { final serverReceivedContentType = await httpServerQueue.next; final serverReceivedBody = await httpServerQueue.next; - expect(serverReceivedContentType, - ['application/x-www-form-urlencoded; charset=plus2']); + expect(serverReceivedContentType, ['application/x-www-form-urlencoded']); expect(serverReceivedBody, 'gau;r]hqa'); // key=value }); @@ -149,7 +147,7 @@ void testRequestBody(Client client) { final serverReceivedContentType = await httpServerQueue.next; final serverReceivedBody = await httpServerQueue.next as String; - expect(serverReceivedContentType, ['image/png; charset=plus2']); + expect(serverReceivedContentType, ['image/png']); expect(serverReceivedBody.codeUnits, [1, 2, 3, 4, 5]); }); diff --git a/pkgs/http_client_conformance_tests/pubspec.yaml b/pkgs/http_client_conformance_tests/pubspec.yaml index 60f8a2fd7f..9f644c55c5 100644 --- a/pkgs/http_client_conformance_tests/pubspec.yaml +++ b/pkgs/http_client_conformance_tests/pubspec.yaml @@ -11,11 +11,12 @@ environment: dependencies: async: ^2.8.2 dart_style: ">=2.3.7 <4.0.0" - http: ^1.2.0 + # TODO(brianquinlan): Tighten this version constraint when package:http 2.0 + # is released. + http: ">=1.2.0 <3.0.0" stream_channel: ^2.1.1 test: ^1.21.2 -# TODO(brianquinlan): Remove dependency_overrides when package:http 1.5.0 is released. dependency_overrides: http: path: ../http diff --git a/pkgs/ok_http/example/pubspec.yaml b/pkgs/ok_http/example/pubspec.yaml index 873d9aaa5c..319c5faf5d 100644 --- a/pkgs/ok_http/example/pubspec.yaml +++ b/pkgs/ok_http/example/pubspec.yaml @@ -36,8 +36,8 @@ flutter: assets: - test_certs/ # Used in integration tests. -# TODO(brianquinlan): Remove this when a release version of `package:http` -# supports abortable requests. +# Use the path version of `package:http` so that the behavior matches the +# expectations in http_client_conformance_tests. dependency_overrides: http: path: ../../http/