From 7a2671ad3600326a5f5b6794faaeafc480415ad5 Mon Sep 17 00:00:00 2001 From: Natalia Kondratyeva Date: Tue, 11 Aug 2020 15:26:25 +0300 Subject: [PATCH] Remove max header length check from WebHeaderCollection (#40529) Removed conditional max header length check from System.Net.WebHeaderCollection as it is redundant and produces puzzling behavior. Removed unused values of WebHeaderCollectionType enum. Contributes to #1486 --- .../tests/HttpListenerResponseTests.cs | 21 ++++++ .../src/Resources/Strings.resx | 8 +-- .../src/System/Net/WebHeaderCollection.cs | 82 +--------------------- .../tests/WebHeaderCollectionTest.cs | 20 ++++++ 4 files changed, 45 insertions(+), 86 deletions(-) diff --git a/src/libraries/System.Net.HttpListener/tests/HttpListenerResponseTests.cs b/src/libraries/System.Net.HttpListener/tests/HttpListenerResponseTests.cs index b654fd4..5b7fd28 100644 --- a/src/libraries/System.Net.HttpListener/tests/HttpListenerResponseTests.cs +++ b/src/libraries/System.Net.HttpListener/tests/HttpListenerResponseTests.cs @@ -435,5 +435,26 @@ namespace System.Net.Tests } } } + + [Fact] + public async Task AddLongHeader_DoesNotThrow() + { + string longString = new string('a', 65536); + + using (HttpListenerResponse response = await GetResponse()) + { + // WebHeaderCollection.Add(String,String) is called inside + response.AddHeader("Long-Header", longString); + + // WebHeaderCollection[HttpResponseHeader] is called inside + response.Redirect("someValueToChangeType"); // this will implicitly change WebHeaderCollection._type + + // WebHeaderCollection.Add(String,String) is called inside + response.AddHeader("Long-Header-2", longString); + + Assert.Equal(longString, response.Headers["Long-Header"]); + Assert.Equal(longString, response.Headers["Long-Header-2"]); + } + } } } diff --git a/src/libraries/System.Net.WebHeaderCollection/src/Resources/Strings.resx b/src/libraries/System.Net.WebHeaderCollection/src/Resources/Strings.resx index cab58a4..eb1e5a6 100644 --- a/src/libraries/System.Net.WebHeaderCollection/src/Resources/Strings.resx +++ b/src/libraries/System.Net.WebHeaderCollection/src/Resources/Strings.resx @@ -75,13 +75,7 @@ Specified value has invalid HTTP Header characters. - - Header values cannot be longer than {0} characters. - Specified value does not have a ':' separator. - - The {0} header must be modified using the appropriate property or method. - - \ No newline at end of file + diff --git a/src/libraries/System.Net.WebHeaderCollection/src/System/Net/WebHeaderCollection.cs b/src/libraries/System.Net.WebHeaderCollection/src/System/Net/WebHeaderCollection.cs index 8997629..1a97af3 100644 --- a/src/libraries/System.Net.WebHeaderCollection/src/System/Net/WebHeaderCollection.cs +++ b/src/libraries/System.Net.WebHeaderCollection/src/System/Net/WebHeaderCollection.cs @@ -16,15 +16,7 @@ namespace System.Net { Unknown, WebRequest, - WebResponse, - HttpWebRequest, - HttpWebResponse, - HttpListenerRequest, - HttpListenerResponse, - FtpWebRequest, - FtpWebResponse, - FileWebRequest, - FileWebResponse, + WebResponse } public class WebHeaderCollection : NameValueCollection, ISerializable @@ -49,9 +41,7 @@ namespace System.Net { _type = WebHeaderCollectionType.WebRequest; } - return _type == WebHeaderCollectionType.WebRequest || - _type == WebHeaderCollectionType.HttpWebRequest || - _type == WebHeaderCollectionType.HttpListenerRequest; + return _type == WebHeaderCollectionType.WebRequest; } } @@ -85,9 +75,7 @@ namespace System.Net { _type = WebHeaderCollectionType.WebResponse; } - return _type == WebHeaderCollectionType.WebResponse || - _type == WebHeaderCollectionType.HttpWebResponse || - _type == WebHeaderCollectionType.HttpListenerResponse; + return _type == WebHeaderCollectionType.WebResponse; } } @@ -141,15 +129,7 @@ namespace System.Net } name = HttpValidationHelpers.CheckBadHeaderNameChars(name); - ThrowOnRestrictedHeader(name); value = HttpValidationHelpers.CheckBadHeaderValueChars(value); - if (_type == WebHeaderCollectionType.WebResponse) - { - if (value != null && value.Length > ushort.MaxValue) - { - throw new ArgumentOutOfRangeException(nameof(value), value, SR.Format(CultureInfo.InvariantCulture, SR.net_headers_toolong, ushort.MaxValue)); - } - } InvalidateCachedArrays(); InnerCollection.Set(name, value); } @@ -169,13 +149,6 @@ namespace System.Net { throw new InvalidOperationException(SR.net_headers_rsp); } - if (_type == WebHeaderCollectionType.WebResponse) - { - if (value != null && value.Length > ushort.MaxValue) - { - throw new ArgumentOutOfRangeException(nameof(value), value, SR.Format(CultureInfo.InvariantCulture, SR.net_headers_toolong, ushort.MaxValue)); - } - } this.Set(header.GetName(), value); } @@ -344,13 +317,6 @@ namespace System.Net { throw new InvalidOperationException(SR.net_headers_rsp); } - if (_type == WebHeaderCollectionType.WebResponse) - { - if (value != null && value.Length > ushort.MaxValue) - { - throw new ArgumentOutOfRangeException(nameof(value), value, SR.Format(CultureInfo.InvariantCulture, SR.net_headers_toolong, ushort.MaxValue)); - } - } this.Add(header.GetName(), value); } @@ -369,15 +335,7 @@ namespace System.Net string name = header.Substring(0, colpos); string value = header.Substring(colpos + 1); name = HttpValidationHelpers.CheckBadHeaderNameChars(name); - ThrowOnRestrictedHeader(name); value = HttpValidationHelpers.CheckBadHeaderValueChars(value); - if (_type == WebHeaderCollectionType.WebResponse) - { - if (value != null && value.Length > ushort.MaxValue) - { - throw new ArgumentOutOfRangeException(nameof(header), value, SR.Format(CultureInfo.InvariantCulture, SR.net_headers_toolong, ushort.MaxValue)); - } - } InvalidateCachedArrays(); InnerCollection.Add(name, value); } @@ -396,15 +354,7 @@ namespace System.Net } name = HttpValidationHelpers.CheckBadHeaderNameChars(name); - ThrowOnRestrictedHeader(name); value = HttpValidationHelpers.CheckBadHeaderValueChars(value); - if (_type == WebHeaderCollectionType.WebResponse) - { - if (value != null && value.Length > ushort.MaxValue) - { - throw new ArgumentOutOfRangeException(nameof(value), value, SR.Format(CultureInfo.InvariantCulture, SR.net_headers_toolong, ushort.MaxValue)); - } - } InvalidateCachedArrays(); InnerCollection.Add(name, value); } @@ -413,35 +363,10 @@ namespace System.Net { headerName = HttpValidationHelpers.CheckBadHeaderNameChars(headerName); headerValue = HttpValidationHelpers.CheckBadHeaderValueChars(headerValue); - if (_type == WebHeaderCollectionType.WebResponse) - { - if (headerValue != null && headerValue.Length > ushort.MaxValue) - { - throw new ArgumentOutOfRangeException(nameof(headerValue), headerValue, SR.Format(CultureInfo.InvariantCulture, SR.net_headers_toolong, ushort.MaxValue)); - } - } InvalidateCachedArrays(); InnerCollection.Add(headerName, headerValue); } - internal void ThrowOnRestrictedHeader(string headerName) - { - if (_type == WebHeaderCollectionType.HttpWebRequest) - { - if (HeaderInfo[headerName].IsRequestRestricted) - { - throw new ArgumentException(SR.Format(SR.net_headerrestrict, headerName), nameof(headerName)); - } - } - else if (_type == WebHeaderCollectionType.HttpListenerResponse) - { - if (HeaderInfo[headerName].IsResponseRestricted) - { - throw new ArgumentException(SR.Format(SR.net_headerrestrict, headerName), nameof(headerName)); - } - } - } - // Remove - // Routine Description: // Removes give header with validation to see if they are "proper" headers. @@ -463,7 +388,6 @@ namespace System.Net { throw new ArgumentNullException(nameof(name)); } - ThrowOnRestrictedHeader(name); name = HttpValidationHelpers.CheckBadHeaderNameChars(name); if (_innerCollection != null) { diff --git a/src/libraries/System.Net.WebHeaderCollection/tests/WebHeaderCollectionTest.cs b/src/libraries/System.Net.WebHeaderCollection/tests/WebHeaderCollectionTest.cs index 5e8b333..6adb462 100644 --- a/src/libraries/System.Net.WebHeaderCollection/tests/WebHeaderCollectionTest.cs +++ b/src/libraries/System.Net.WebHeaderCollection/tests/WebHeaderCollectionTest.cs @@ -752,5 +752,25 @@ namespace System.Net.Tests Assert.Equal(new[] { "firstName" }, w.AllKeys); Assert.Equal("first", w["firstName"]); } + + [Fact] + public void AddLongString_DoesNotThrow() + { + string longString = new string('a', 65536); + WebHeaderCollection headerCollection = new WebHeaderCollection(); + + headerCollection.Add("Long-Header", longString); + headerCollection["Long-Header-2"] = longString; + + headerCollection.Add(HttpResponseHeader.SetCookie, "someValueToChangeType"); // this will implicitly change _type + + headerCollection.Add("Long-Header-3", longString); + headerCollection["Long-Header-4"] = longString; + + Assert.Equal(longString, headerCollection["Long-Header"]); + Assert.Equal(longString, headerCollection["Long-Header-2"]); + Assert.Equal(longString, headerCollection["Long-Header-3"]); + Assert.Equal(longString, headerCollection["Long-Header-4"]); + } } } -- 2.7.4