Revert "[dart:io] Preserve header case in http header _builds()"

This reverts commit af19f96. Reason for revert: Breaks package:shelf tests. Original change's description: > [dart:io] Preserve header case in http header _builds() > > Bug: #33501 > Change-Id: I57d9bba251b76314bf40b81d1b09bd4643dce4d2 > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/141911 > Commit-Queue: Zichang Guo <[email protected]> > Reviewed-by: Siva Annamalai <[email protected]> > Reviewed-by: Jonas Termansen <[email protected]> [email protected],[email protected],[email protected] Change-Id: I9c0418a256cb53e415ed0d5aeab84c4b0b4a161d No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: #33501 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/142980 Reviewed-by: David Morgan <[email protected]> Commit-Queue: David Morgan <[email protected]>
dart-lang · Apr 9, 2020 · fb0902a · fb0902a
1 parent af19f96
commit fb0902a
Show file tree

Hide file tree

Showing 6 changed files with 40 additions and 69 deletions.
diff --git 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<String> values) {
-      String originalName = _originalHeaderName(name);
+    for (String name in _headers.keys) {
+      List<String> 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
@@ -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<String> 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
@@ -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<String> 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
@@ -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<String> 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
@@ -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
@@ -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) {