From 8b3c308c226dd61532e543697d9d35bb94aba823 Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Tue, 31 May 2016 16:25:40 -0700 Subject: [PATCH] Limit value number instead of key number in form reader --- .../Features/FormFeature.cs | 10 +- .../Features/FormOptions.cs | 4 +- .../FormReader.cs | 10 +- .../KeyValueAccumulator.cs | 8 +- .../MultipartReader.cs | 2 +- .../Features/FormFeatureTests.cs | 104 +++++++++++++----- .../FormReaderTests.cs | 26 +++-- 7 files changed, 114 insertions(+), 50 deletions(-) diff --git a/src/Microsoft.AspNetCore.Http/Features/FormFeature.cs b/src/Microsoft.AspNetCore.Http/Features/FormFeature.cs index 649f50d849..1aee72ff50 100644 --- a/src/Microsoft.AspNetCore.Http/Features/FormFeature.cs +++ b/src/Microsoft.AspNetCore.Http/Features/FormFeature.cs @@ -148,7 +148,7 @@ namespace Microsoft.AspNetCore.Http.Features var encoding = FilterEncoding(contentType.Encoding); using (var formReader = new FormReader(_request.Body, encoding) { - KeyCountLimit = _options.KeyCountLimit, + ValueCountLimit = _options.ValueCountLimit, KeyLengthLimit = _options.KeyLengthLimit, ValueLengthLimit = _options.ValueLengthLimit, }) @@ -200,9 +200,9 @@ namespace Microsoft.AspNetCore.Http.Features { files = new FormFileCollection(); } - if (files.Count >= _options.KeyCountLimit) + if (files.Count >= _options.ValueCountLimit) { - throw new InvalidDataException($"Form key count limit {_options.KeyCountLimit} exceeded."); + throw new InvalidDataException($"Form value count limit {_options.ValueCountLimit} exceeded."); } files.Add(file); } @@ -222,9 +222,9 @@ namespace Microsoft.AspNetCore.Http.Features // The value length limit is enforced by MultipartBodyLengthLimit var value = await reader.ReadToEndAsync(); formAccumulator.Append(key, value); - if (formAccumulator.Count > _options.KeyCountLimit) + if (formAccumulator.ValueCount > _options.ValueCountLimit) { - throw new InvalidDataException($"Form key count limit {_options.KeyCountLimit} exceeded."); + throw new InvalidDataException($"Form value count limit {_options.ValueCountLimit} exceeded."); } } } diff --git a/src/Microsoft.AspNetCore.Http/Features/FormOptions.cs b/src/Microsoft.AspNetCore.Http/Features/FormOptions.cs index dc249781e7..17e521b215 100644 --- a/src/Microsoft.AspNetCore.Http/Features/FormOptions.cs +++ b/src/Microsoft.AspNetCore.Http/Features/FormOptions.cs @@ -33,10 +33,10 @@ namespace Microsoft.AspNetCore.Http.Features public long BufferBodyLengthLimit { get; set; } = DefaultBufferBodyLengthLimit; /// - /// A limit for the number of form entries to allow. Entries with the same key will be combined. + /// A limit for the number of form entries to allow. /// Forms that exceed this limit will throw an when parsed. /// - public int KeyCountLimit { get; set; } = FormReader.DefaultKeyCountLimit; + public int ValueCountLimit { get; set; } = FormReader.DefaultValueCountLimit; /// /// A limit on the length of individual keys. Forms containing keys that exceed this limit will diff --git a/src/Microsoft.AspNetCore.WebUtilities/FormReader.cs b/src/Microsoft.AspNetCore.WebUtilities/FormReader.cs index 9080ca1daf..958a4971fa 100644 --- a/src/Microsoft.AspNetCore.WebUtilities/FormReader.cs +++ b/src/Microsoft.AspNetCore.WebUtilities/FormReader.cs @@ -17,7 +17,7 @@ namespace Microsoft.AspNetCore.WebUtilities /// public class FormReader : IDisposable { - public const int DefaultKeyCountLimit = 1024; + public const int DefaultValueCountLimit = 1024; public const int DefaultKeyLengthLimit = 1024 * 2; public const int DefaultValueLengthLimit = 1024 * 1024 * 4; @@ -78,9 +78,9 @@ namespace Microsoft.AspNetCore.WebUtilities } /// - /// The limit on the number of form keys to allow in ReadForm or ReadFormAsync. + /// The limit on the number of form values to allow in ReadForm or ReadFormAsync. /// - public int KeyCountLimit { get; set; } = DefaultKeyCountLimit; + public int ValueCountLimit { get; set; } = DefaultValueCountLimit; /// /// The limit on the length of form keys. @@ -293,9 +293,9 @@ namespace Microsoft.AspNetCore.WebUtilities if (ReadSucceded()) { accumulator.Append(_currentKey, _currentValue); - if (accumulator.Count > KeyCountLimit) + if (accumulator.ValueCount > ValueCountLimit) { - throw new InvalidDataException($"Form key count limit {KeyCountLimit} exceeded."); + throw new InvalidDataException($"Form value count limit {ValueCountLimit} exceeded."); } } } diff --git a/src/Microsoft.AspNetCore.WebUtilities/KeyValueAccumulator.cs b/src/Microsoft.AspNetCore.WebUtilities/KeyValueAccumulator.cs index 1420344346..5ae402e523 100644 --- a/src/Microsoft.AspNetCore.WebUtilities/KeyValueAccumulator.cs +++ b/src/Microsoft.AspNetCore.WebUtilities/KeyValueAccumulator.cs @@ -59,11 +59,15 @@ namespace Microsoft.AspNetCore.WebUtilities // First value for this key _accumulator[key] = new StringValues(value); } + + ValueCount++; } - public bool HasValues => _accumulator != null; + public bool HasValues => ValueCount > 0; - public int Count => _accumulator?.Count ?? 0; + public int KeyCount => _accumulator?.Count ?? 0; + + public int ValueCount { get; private set; } public Dictionary GetResults() { diff --git a/src/Microsoft.AspNetCore.WebUtilities/MultipartReader.cs b/src/Microsoft.AspNetCore.WebUtilities/MultipartReader.cs index 59c9cc0042..2da50a5360 100644 --- a/src/Microsoft.AspNetCore.WebUtilities/MultipartReader.cs +++ b/src/Microsoft.AspNetCore.WebUtilities/MultipartReader.cs @@ -104,7 +104,7 @@ namespace Microsoft.AspNetCore.WebUtilities var name = line.Substring(0, splitIndex); var value = line.Substring(splitIndex + 1, line.Length - splitIndex - 1).Trim(); accumulator.Append(name, value); - if (accumulator.Count > HeadersCountLimit) + if (accumulator.KeyCount > HeadersCountLimit) { throw new InvalidDataException($"Multipart headers count limit {HeadersCountLimit} exceeded."); } diff --git a/test/Microsoft.AspNetCore.Http.Tests/Features/FormFeatureTests.cs b/test/Microsoft.AspNetCore.Http.Tests/Features/FormFeatureTests.cs index 1c3bdac123..79bdb9f947 100644 --- a/test/Microsoft.AspNetCore.Http.Tests/Features/FormFeatureTests.cs +++ b/test/Microsoft.AspNetCore.Http.Tests/Features/FormFeatureTests.cs @@ -2,10 +2,10 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; using System.IO; using System.Text; using System.Threading.Tasks; -using Microsoft.AspNetCore.Http.Internal; using Xunit; namespace Microsoft.AspNetCore.Http.Features @@ -48,33 +48,35 @@ namespace Microsoft.AspNetCore.Http.Features } private const string MultipartContentType = "multipart/form-data; boundary=WebKitFormBoundary5pDRpGheQXaM8k3T"; - private const string EmptyMultipartForm = -"--WebKitFormBoundary5pDRpGheQXaM8k3T--"; + + private const string EmptyMultipartForm = "--WebKitFormBoundary5pDRpGheQXaM8k3T--"; + // Note that CRLF (\r\n) is required. You can't use multi-line C# strings here because the line breaks on Linux are just LF. + private const string MultipartFormEnd = "--WebKitFormBoundary5pDRpGheQXaM8k3T--\r\n"; + + private const string MultipartFormField = "--WebKitFormBoundary5pDRpGheQXaM8k3T\r\n" + +"Content-Disposition: form-data; name=\"description\"\r\n" + +"\r\n" + +"Foo\r\n"; + + private const string MultipartFormFile = "--WebKitFormBoundary5pDRpGheQXaM8k3T\r\n" + +"Content-Disposition: form-data; name=\"myfile1\"; filename=\"temp.html\"\r\n" + +"Content-Type: text/html\r\n" + +"\r\n" + +"Hello World\r\n"; + private const string MultipartFormWithField = -"--WebKitFormBoundary5pDRpGheQXaM8k3T\r\n" + -"Content-Disposition: form-data; name=\"description\"\r\n" + -"\r\n" + -"Foo\r\n" + -"--WebKitFormBoundary5pDRpGheQXaM8k3T--"; + MultipartFormField + + MultipartFormEnd; + private const string MultipartFormWithFile = -"--WebKitFormBoundary5pDRpGheQXaM8k3T\r\n" + -"Content-Disposition: form-data; name=\"myfile1\"; filename=\"temp.html\"\r\n" + -"Content-Type: text/html\r\n" + -"\r\n" + -"Hello World\r\n" + -"--WebKitFormBoundary5pDRpGheQXaM8k3T--"; + MultipartFormFile + + MultipartFormEnd; + private const string MultipartFormWithFieldAndFile = -"--WebKitFormBoundary5pDRpGheQXaM8k3T\r\n" + -"Content-Disposition: form-data; name=\"description\"\r\n" + -"\r\n" + -"Foo\r\n" + -"--WebKitFormBoundary5pDRpGheQXaM8k3T\r\n" + -"Content-Disposition: form-data; name=\"myfile1\"; filename=\"temp.html\"\r\n" + -"Content-Type: text/html\r\n" + -"\r\n" + -"Hello World\r\n" + -"--WebKitFormBoundary5pDRpGheQXaM8k3T--"; + MultipartFormField + + MultipartFormFile + + MultipartFormEnd; [Theory] [InlineData(true)] @@ -243,6 +245,55 @@ namespace Microsoft.AspNetCore.Http.Features await responseFeature.CompleteAsync(); } + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task ReadFormAsync_ValueCountLimitExceeded_Throw(bool bufferRequest) + { + var formContent = new List(); + formContent.AddRange(Encoding.UTF8.GetBytes(MultipartFormField)); + formContent.AddRange(Encoding.UTF8.GetBytes(MultipartFormField)); + formContent.AddRange(Encoding.UTF8.GetBytes(MultipartFormField)); + formContent.AddRange(Encoding.UTF8.GetBytes(MultipartFormEnd)); + + var context = new DefaultHttpContext(); + var responseFeature = new FakeResponseFeature(); + context.Features.Set(responseFeature); + context.Request.ContentType = MultipartContentType; + context.Request.Body = new NonSeekableReadStream(formContent.ToArray()); + + IFormFeature formFeature = new FormFeature(context.Request, new FormOptions() { BufferBody = bufferRequest, ValueCountLimit = 2 }); + context.Features.Set(formFeature); + + var exception = await Assert.ThrowsAsync (() => context.Request.ReadFormAsync()); + Assert.Equal(exception.Message, "Form value count limit 2 exceeded."); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task ReadFormAsync_ValueCountLimitExceededWithFiles_Throw(bool bufferRequest) + { + var formContent = new List(); + formContent.AddRange(Encoding.UTF8.GetBytes(MultipartFormFile)); + formContent.AddRange(Encoding.UTF8.GetBytes(MultipartFormFile)); + formContent.AddRange(Encoding.UTF8.GetBytes(MultipartFormFile)); + formContent.AddRange(Encoding.UTF8.GetBytes(MultipartFormEnd)); + + + var context = new DefaultHttpContext(); + var responseFeature = new FakeResponseFeature(); + context.Features.Set(responseFeature); + context.Request.ContentType = MultipartContentType; + context.Request.Body = new NonSeekableReadStream(formContent.ToArray()); + + IFormFeature formFeature = new FormFeature(context.Request, new FormOptions() { BufferBody = bufferRequest, ValueCountLimit = 2 }); + context.Features.Set(formFeature); + + var exception = await Assert.ThrowsAsync (() => context.Request.ReadFormAsync()); + Assert.Equal(exception.Message, "Form value count limit 2 exceeded."); + } + [Theory] // FileBufferingReadStream transitions to disk storage after 30kb, and stops pooling buffers at 1mb. [InlineData(true, 1024)] @@ -313,10 +364,7 @@ namespace Microsoft.AspNetCore.Http.Features { var stream = new MemoryStream(); var header = -"--WebKitFormBoundary5pDRpGheQXaM8k3T\r\n" + -"Content-Disposition: form-data; name=\"description\"\r\n" + -"\r\n" + -"Foo\r\n" + +MultipartFormField + "--WebKitFormBoundary5pDRpGheQXaM8k3T\r\n" + "Content-Disposition: form-data; name=\"myfile1\"; filename=\"temp.html\"\r\n" + "Content-Type: text/html\r\n" + diff --git a/test/Microsoft.AspNetCore.WebUtilities.Tests/FormReaderTests.cs b/test/Microsoft.AspNetCore.WebUtilities.Tests/FormReaderTests.cs index f28f307f05..fbe6af6cce 100644 --- a/test/Microsoft.AspNetCore.WebUtilities.Tests/FormReaderTests.cs +++ b/test/Microsoft.AspNetCore.WebUtilities.Tests/FormReaderTests.cs @@ -65,28 +65,40 @@ namespace Microsoft.AspNetCore.WebUtilities [Theory] [InlineData(true)] [InlineData(false)] - public async Task ReadFormAsync_KeyCountLimitMet_Success(bool bufferRequest) + public async Task ReadFormAsync_ValueCountLimitMet_Success(bool bufferRequest) { - var body = MakeStream(bufferRequest, "foo=1&bar=2&baz=3&baz=4"); + var body = MakeStream(bufferRequest, "foo=1&bar=2&baz=3"); - var formCollection = await ReadFormAsync(new FormReader(body) { KeyCountLimit = 3 }); + var formCollection = await ReadFormAsync(new FormReader(body) { ValueCountLimit = 3 }); Assert.Equal("1", formCollection["foo"].ToString()); Assert.Equal("2", formCollection["bar"].ToString()); - Assert.Equal("3,4", formCollection["baz"].ToString()); + Assert.Equal("3", formCollection["baz"].ToString()); Assert.Equal(3, formCollection.Count); } [Theory] [InlineData(true)] [InlineData(false)] - public async Task ReadFormAsync_KeyCountLimitExceeded_Throw(bool bufferRequest) + public async Task ReadFormAsync_ValueCountLimitExceeded_Throw(bool bufferRequest) { var body = MakeStream(bufferRequest, "foo=1&baz=2&bar=3&baz=4&baf=5"); var exception = await Assert.ThrowsAsync( - () => ReadFormAsync(new FormReader(body) { KeyCountLimit = 3 })); - Assert.Equal("Form key count limit 3 exceeded.", exception.Message); + () => ReadFormAsync(new FormReader(body) { ValueCountLimit = 3 })); + Assert.Equal("Form value count limit 3 exceeded.", exception.Message); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task ReadFormAsync_ValueCountLimitExceededSameKey_Throw(bool bufferRequest) + { + var body = MakeStream(bufferRequest, "baz=1&baz=2&baz=3&baz=4"); + + var exception = await Assert.ThrowsAsync( + () => ReadFormAsync(new FormReader(body) { ValueCountLimit = 3 })); + Assert.Equal("Form value count limit 3 exceeded.", exception.Message); } [Theory]