From fb0902a05dfa85752b965a29866b03fbed5d4e7e Mon Sep 17 00:00:00 2001 From: David Morgan Date: Thu, 9 Apr 2020 10:38:29 +0000 Subject: [PATCH] Revert "[dart:io] Preserve header case in http header _builds()" This reverts commit af19f9638d6d74ad569c93bc907060aa7299a946. Reason for revert: Breaks package:shelf tests. Original change's description: > [dart:io] Preserve header case in http header _builds() > > Bug: https://github.com/dart-lang/sdk/issues/33501 > Change-Id: I57d9bba251b76314bf40b81d1b09bd4643dce4d2 > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/141911 > Commit-Queue: Zichang Guo > Reviewed-by: Siva Annamalai > Reviewed-by: Jonas Termansen TBR=sortie@google.com,asiva@google.com,zichangguo@google.com Change-Id: I9c0418a256cb53e415ed0d5aeab84c4b0b4a161d No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: https://github.com/dart-lang/sdk/issues/33501 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/142980 Reviewed-by: David Morgan Commit-Queue: David Morgan --- sdk/lib/_http/http_headers.dart | 13 +++++----- sdk/lib/_http/http_parser.dart | 17 ++++++------ sdk_nnbd/lib/_http/http_headers.dart | 10 +++---- sdk_nnbd/lib/_http/http_parser.dart | 17 ++++++------ .../io/http_connection_header_test.dart | 26 +++++-------------- .../io/http_connection_header_test.dart | 26 +++++-------------- 6 files changed, 40 insertions(+), 69 deletions(-) diff --git a/sdk/lib/_http/http_headers.dart b/sdk/lib/_http/http_headers.dart index ad887b92c8cf..23cd58ea2f4f 100644 --- a/sdk/lib/_http/http_headers.dart +++ b/sdk/lib/_http/http_headers.dart @@ -137,7 +137,6 @@ class _HttpHeaders implements HttpHeaders { void set persistentConnection(bool persistentConnection) { _checkMutable(); if (persistentConnection == _persistentConnection) return; - final originalName = _originalHeaderName(HttpHeaders.connectionHeader); if (persistentConnection) { if (protocolVersion == "1.1") { remove(HttpHeaders.connectionHeader, "close"); @@ -147,11 +146,11 @@ class _HttpHeaders implements HttpHeaders { "Trying to set 'Connection: Keep-Alive' on HTTP 1.0 headers with " "no ContentLength"); } - add(originalName, "keep-alive", preserveHeaderCase: true); + add(HttpHeaders.connectionHeader, "keep-alive"); } } else { if (protocolVersion == "1.1") { - add(originalName, "close", preserveHeaderCase: true); + add(HttpHeaders.connectionHeader, "close"); } else { remove(HttpHeaders.connectionHeader, "keep-alive"); } @@ -490,10 +489,10 @@ class _HttpHeaders implements HttpHeaders { } void _build(BytesBuilder builder) { - _headers.forEach((String name, List values) { - String originalName = _originalHeaderName(name); + for (String name in _headers.keys) { + List values = _headers[name]; bool fold = _foldHeader(name); - var nameData = originalName.codeUnits; + var nameData = name.codeUnits; builder.add(nameData); builder.addByte(_CharCode.COLON); builder.addByte(_CharCode.SP); @@ -514,7 +513,7 @@ class _HttpHeaders implements HttpHeaders { } builder.addByte(_CharCode.CR); builder.addByte(_CharCode.LF); - }); + } } String toString() { diff --git a/sdk/lib/_http/http_parser.dart b/sdk/lib/_http/http_parser.dart index 9328e7a6e33b..74582d7b7000 100644 --- a/sdk/lib/_http/http_parser.dart +++ b/sdk/lib/_http/http_parser.dart @@ -635,7 +635,7 @@ class _HttpParser extends Stream<_HttpIncoming> { _index--; // Make the new state see the LF again. } else { // Start of new header field. - _addWithValidation(_headerField, byte); + _addWithValidation(_headerField, _toLowerCaseByte(byte)); _state = _State.HEADER_FIELD; } break; @@ -647,7 +647,7 @@ class _HttpParser extends Stream<_HttpIncoming> { if (!_isTokenChar(byte)) { throw HttpException("Invalid header field name, with $byte"); } - _addWithValidation(_headerField, byte); + _addWithValidation(_headerField, _toLowerCaseByte(byte)); } break; @@ -684,15 +684,14 @@ class _HttpParser extends Stream<_HttpIncoming> { } else { String headerField = new String.fromCharCodes(_headerField); String headerValue = new String.fromCharCodes(_headerValue); - String lowerCaseHeader = headerField.toLowerCase(); - if (lowerCaseHeader == HttpHeaders.contentLengthHeader) { + if (headerField == HttpHeaders.contentLengthHeader) { // Content Length header should not have more than one occurance // or coexist with Transfer Encoding header. if (_contentLength || _transferEncoding) { _statusCode = HttpStatus.badRequest; } _contentLength = true; - } else if (lowerCaseHeader == HttpHeaders.transferEncodingHeader) { + } else if (headerField == HttpHeaders.transferEncodingHeader) { _transferEncoding = true; if (_caseInsensitiveCompare("chunked".codeUnits, _headerValue)) { _chunked = true; @@ -701,7 +700,7 @@ class _HttpParser extends Stream<_HttpIncoming> { _statusCode = HttpStatus.badRequest; } } - if (lowerCaseHeader == HttpHeaders.connectionHeader) { + if (headerField == HttpHeaders.connectionHeader) { List tokens = _tokenizeFieldValue(headerValue); final bool isResponse = _messageType == _MessageType.RESPONSE; final bool isUpgradeCode = @@ -714,10 +713,10 @@ class _HttpParser extends Stream<_HttpIncoming> { (isUpgrade && isResponse && isUpgradeCode)) { _connectionUpgrade = true; } - _headers.add(headerField, tokens[i], preserveHeaderCase: true); + _headers.add(headerField, tokens[i]); } } else { - _headers.add(headerField, headerValue, preserveHeaderCase: true); + _headers.add(headerField, headerValue); } _headerField.clear(); _headerValue.clear(); @@ -730,7 +729,7 @@ class _HttpParser extends Stream<_HttpIncoming> { } else { // Start of new header field. _state = _State.HEADER_FIELD; - _addWithValidation(_headerField, byte); + _addWithValidation(_headerField, _toLowerCaseByte(byte)); } } break; diff --git a/sdk_nnbd/lib/_http/http_headers.dart b/sdk_nnbd/lib/_http/http_headers.dart index ae8b4829bdba..98ddbf5570f1 100644 --- a/sdk_nnbd/lib/_http/http_headers.dart +++ b/sdk_nnbd/lib/_http/http_headers.dart @@ -130,7 +130,6 @@ class _HttpHeaders implements HttpHeaders { void set persistentConnection(bool persistentConnection) { _checkMutable(); if (persistentConnection == _persistentConnection) return; - final originalName = _originalHeaderName(HttpHeaders.connectionHeader); if (persistentConnection) { if (protocolVersion == "1.1") { remove(HttpHeaders.connectionHeader, "close"); @@ -140,11 +139,11 @@ class _HttpHeaders implements HttpHeaders { "Trying to set 'Connection: Keep-Alive' on HTTP 1.0 headers with " "no ContentLength"); } - add(originalName, "keep-alive", preserveHeaderCase: true); + add(HttpHeaders.connectionHeader, "keep-alive"); } } else { if (protocolVersion == "1.1") { - add(originalName, "close", preserveHeaderCase: true); + add(HttpHeaders.connectionHeader, "close"); } else { remove(HttpHeaders.connectionHeader, "keep-alive"); } @@ -501,10 +500,9 @@ class _HttpHeaders implements HttpHeaders { } void _build(BytesBuilder builder) { - _headers.forEach((String name, List values) { - String originalName = _originalHeaderName(name); + _headers.forEach((name, values) { bool fold = _foldHeader(name); - var nameData = originalName.codeUnits; + var nameData = name.codeUnits; builder.add(nameData); builder.addByte(_CharCode.COLON); builder.addByte(_CharCode.SP); diff --git a/sdk_nnbd/lib/_http/http_parser.dart b/sdk_nnbd/lib/_http/http_parser.dart index 5c1218bf826a..96891a160250 100644 --- a/sdk_nnbd/lib/_http/http_parser.dart +++ b/sdk_nnbd/lib/_http/http_parser.dart @@ -634,7 +634,7 @@ class _HttpParser extends Stream<_HttpIncoming> { _index = _index - 1; // Make the new state see the LF again. } else { // Start of new header field. - _addWithValidation(_headerField, byte); + _addWithValidation(_headerField, _toLowerCaseByte(byte)); _state = _State.HEADER_FIELD; } break; @@ -646,7 +646,7 @@ class _HttpParser extends Stream<_HttpIncoming> { if (!_isTokenChar(byte)) { throw HttpException("Invalid header field name, with $byte"); } - _addWithValidation(_headerField, byte); + _addWithValidation(_headerField, _toLowerCaseByte(byte)); } break; @@ -683,15 +683,14 @@ class _HttpParser extends Stream<_HttpIncoming> { } else { String headerField = new String.fromCharCodes(_headerField); String headerValue = new String.fromCharCodes(_headerValue); - String lowerCaseHeader = headerField.toLowerCase(); - if (lowerCaseHeader == HttpHeaders.contentLengthHeader) { + if (headerField == HttpHeaders.contentLengthHeader) { // Content Length header should not have more than one occurance // or coexist with Transfer Encoding header. if (_contentLength || _transferEncoding) { _statusCode = HttpStatus.badRequest; } _contentLength = true; - } else if (lowerCaseHeader == HttpHeaders.transferEncodingHeader) { + } else if (headerField == HttpHeaders.transferEncodingHeader) { _transferEncoding = true; if (_caseInsensitiveCompare("chunked".codeUnits, _headerValue)) { _chunked = true; @@ -701,7 +700,7 @@ class _HttpParser extends Stream<_HttpIncoming> { } } var headers = _headers!; - if (lowerCaseHeader == HttpHeaders.connectionHeader) { + if (headerField == HttpHeaders.connectionHeader) { List tokens = _tokenizeFieldValue(headerValue); final bool isResponse = _messageType == _MessageType.RESPONSE; final bool isUpgradeCode = @@ -714,10 +713,10 @@ class _HttpParser extends Stream<_HttpIncoming> { (isUpgrade && isResponse && isUpgradeCode)) { _connectionUpgrade = true; } - headers.add(headerField, tokens[i], preserveHeaderCase: true); + headers.add(headerField, tokens[i]); } } else { - headers.add(headerField, headerValue, preserveHeaderCase: true); + headers.add(headerField, headerValue); } _headerField.clear(); _headerValue.clear(); @@ -730,7 +729,7 @@ class _HttpParser extends Stream<_HttpIncoming> { } else { // Start of new header field. _state = _State.HEADER_FIELD; - _addWithValidation(_headerField, byte); + _addWithValidation(_headerField, _toLowerCaseByte(byte)); } } break; diff --git a/tests/standalone/io/http_connection_header_test.dart b/tests/standalone/io/http_connection_header_test.dart index 9b8c1f1a6c2b..55c8cd85d536 100644 --- a/tests/standalone/io/http_connection_header_test.dart +++ b/tests/standalone/io/http_connection_header_test.dart @@ -3,19 +3,15 @@ // BSD-style license that can be found in the LICENSE file. // -import "dart:io"; - import "package:expect/expect.dart"; +import "dart:isolate"; +import "dart:io"; -void setConnectionHeaders(HttpHeaders headers, bool requestHeader) { +void setConnectionHeaders(HttpHeaders headers) { headers.add(HttpHeaders.connectionHeader, "my-connection-header1"); headers.add("My-Connection-Header1", "some-value1"); - // Check connection Header is changed to upper-cased representation. - headers.add("Connection", "my-connection-header2", preserveHeaderCase: true); - headers.add("My-Connection-Header2", "some-value2", preserveHeaderCase: true); - if (requestHeader) { - headers.set("Host", headers['host']![0], preserveHeaderCase: true); - } + headers.add(HttpHeaders.connectionHeader, "my-connection-header2"); + headers.add("My-Connection-Header2", "some-value2"); } void checkExpectedConnectionHeaders( @@ -45,20 +41,12 @@ void test(int totalConnections, bool clientPersistentConnection) { checkExpectedConnectionHeaders( request.headers, request.persistentConnection); - // PreserveHeaderCase preserves the Case of header. - final string = request.headers.toString(); - Expect.isTrue(string.contains('Connection:')); - Expect.isTrue(string.contains('Host:')); - Expect.isTrue(string.contains('My-Connection-Header2: some-value2')); - Expect.isTrue( - string.contains('My-Connection-Header1: some-value1'.toLowerCase())); - // Generate response. If the client signaled non-persistent // connection the server should not need to set it. if (request.persistentConnection) { request.response.persistentConnection = false; } - setConnectionHeaders(request.response.headers, false); + setConnectionHeaders(request.response.headers); request.response.close(); }); @@ -68,7 +56,7 @@ void test(int totalConnections, bool clientPersistentConnection) { client .get("127.0.0.1", server.port, "/") .then((HttpClientRequest request) { - setConnectionHeaders(request.headers, true); + setConnectionHeaders(request.headers); request.persistentConnection = clientPersistentConnection; return request.close(); }).then((HttpClientResponse response) { diff --git a/tests/standalone_2/io/http_connection_header_test.dart b/tests/standalone_2/io/http_connection_header_test.dart index 06b295f16262..a42e45811caa 100644 --- a/tests/standalone_2/io/http_connection_header_test.dart +++ b/tests/standalone_2/io/http_connection_header_test.dart @@ -3,19 +3,15 @@ // BSD-style license that can be found in the LICENSE file. // -import "dart:io"; - import "package:expect/expect.dart"; +import "dart:isolate"; +import "dart:io"; -void setConnectionHeaders(HttpHeaders headers, bool requestHeader) { +void setConnectionHeaders(HttpHeaders headers) { headers.add(HttpHeaders.connectionHeader, "my-connection-header1"); headers.add("My-Connection-Header1", "some-value1"); - // Check connection Header is changed to upper-cased representation. - headers.add("Connection", "my-connection-header2", preserveHeaderCase: true); - headers.add("My-Connection-Header2", "some-value2", preserveHeaderCase: true); - if (requestHeader) { - headers.set("Host", headers['host'][0], preserveHeaderCase: true); - } + headers.add(HttpHeaders.connectionHeader, "my-connection-header2"); + headers.add("My-Connection-Header2", "some-value2"); } void checkExpectedConnectionHeaders( @@ -45,20 +41,12 @@ void test(int totalConnections, bool clientPersistentConnection) { checkExpectedConnectionHeaders( request.headers, request.persistentConnection); - // PreserveHeaderCase preserves the Case of header. - final string = request.headers.toString(); - Expect.isTrue(string.contains('Connection:')); - Expect.isTrue(string.contains('Host:')); - Expect.isTrue(string.contains('My-Connection-Header2: some-value2')); - Expect.isTrue( - string.contains('My-Connection-Header1: some-value1'.toLowerCase())); - // Generate response. If the client signaled non-persistent // connection the server should not need to set it. if (request.persistentConnection) { request.response.persistentConnection = false; } - setConnectionHeaders(request.response.headers, false); + setConnectionHeaders(request.response.headers); request.response.close(); }); @@ -68,7 +56,7 @@ void test(int totalConnections, bool clientPersistentConnection) { client .get("127.0.0.1", server.port, "/") .then((HttpClientRequest request) { - setConnectionHeaders(request.headers, true); + setConnectionHeaders(request.headers); request.persistentConnection = clientPersistentConnection; return request.close(); }).then((HttpClientResponse response) {