From 754bf0033dfef68a1ab7ec3d094b3c3ae4ca2498 Mon Sep 17 00:00:00 2001 From: Jan Ove Halvorsen Date: Thu, 4 Jan 2024 09:00:52 +0100 Subject: [PATCH 01/10] Added a null check in ResolveHasInvalidAntiforgeryValidationFeature() --- src/Http/Http/src/Features/FormFeature.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Http/Http/src/Features/FormFeature.cs b/src/Http/Http/src/Features/FormFeature.cs index a9128f8f3f01..c5dca940a913 100644 --- a/src/Http/Http/src/Features/FormFeature.cs +++ b/src/Http/Http/src/Features/FormFeature.cs @@ -326,6 +326,7 @@ private static bool HasMultipartFormContentType([NotNullWhen(true)] MediaTypeHea private bool ResolveHasInvalidAntiforgeryValidationFeature() { + if (_request is null) return false; var hasInvokedMiddleware = _request.HttpContext.Items.ContainsKey("__AntiforgeryMiddlewareWithEndpointInvoked"); var hasInvalidToken = _request.HttpContext.Features.Get() is { IsValid: false }; return hasInvokedMiddleware && hasInvalidToken; From 69d6914578079084315180d747111ab5367db234 Mon Sep 17 00:00:00 2001 From: Jan Ove Halvorsen Date: Mon, 8 Jan 2024 15:15:58 +0100 Subject: [PATCH 02/10] Added braces to the if statement --- src/Http/Http/src/Features/FormFeature.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Http/Http/src/Features/FormFeature.cs b/src/Http/Http/src/Features/FormFeature.cs index c5dca940a913..26f49a2573a6 100644 --- a/src/Http/Http/src/Features/FormFeature.cs +++ b/src/Http/Http/src/Features/FormFeature.cs @@ -326,7 +326,10 @@ private static bool HasMultipartFormContentType([NotNullWhen(true)] MediaTypeHea private bool ResolveHasInvalidAntiforgeryValidationFeature() { - if (_request is null) return false; + if (_request is null) + { + return false; + } var hasInvokedMiddleware = _request.HttpContext.Items.ContainsKey("__AntiforgeryMiddlewareWithEndpointInvoked"); var hasInvalidToken = _request.HttpContext.Features.Get() is { IsValid: false }; return hasInvokedMiddleware && hasInvalidToken; From e99d7195414188fa21fafee7f2ed274c00c444cd Mon Sep 17 00:00:00 2001 From: Jan Ove Halvorsen Date: Tue, 9 Jan 2024 14:43:45 +0100 Subject: [PATCH 03/10] Changed the null annotation for the _request field and some logic to handle it --- src/Http/Http/src/Features/FormFeature.cs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/Http/Http/src/Features/FormFeature.cs b/src/Http/Http/src/Features/FormFeature.cs index 26f49a2573a6..2557cca0fb0c 100644 --- a/src/Http/Http/src/Features/FormFeature.cs +++ b/src/Http/Http/src/Features/FormFeature.cs @@ -16,7 +16,7 @@ namespace Microsoft.AspNetCore.Http.Features; /// public class FormFeature : IFormFeature { - private readonly HttpRequest _request; + private readonly HttpRequest? _request; private readonly Endpoint? _endpoint; private FormOptions _options; private Task? _parsedFormTask; @@ -31,7 +31,6 @@ public FormFeature(IFormCollection form) ArgumentNullException.ThrowIfNull(form); Form = form; - _request = default!; _options = FormOptions.Default; } @@ -71,7 +70,15 @@ private MediaTypeHeaderValue? ContentType { get { - _ = MediaTypeHeaderValue.TryParse(_request.ContentType, out var mt); + // Set directly + if (Form is not null) + { + _ = MediaTypeHeaderValue.TryParse("application/x-www-form-urlencoded", out var mt); + } + else + { + _ = MediaTypeHeaderValue.TryParse(_request.ContentType, out var mt); + } return mt; } } @@ -87,6 +94,11 @@ public bool HasFormContentType return true; } + if (_request is null) + { + return false; + } + var contentType = ContentType; return HasApplicationFormContentType(contentType) || HasMultipartFormContentType(contentType); } From 01ba77c55c762050e35344d1467f1486a58e2bec Mon Sep 17 00:00:00 2001 From: Jan Ove Halvorsen Date: Tue, 9 Jan 2024 15:15:58 +0100 Subject: [PATCH 04/10] Fix bug in ContentType property getter --- src/Http/Http/src/Features/FormFeature.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Http/Http/src/Features/FormFeature.cs b/src/Http/Http/src/Features/FormFeature.cs index 2557cca0fb0c..30609d969a54 100644 --- a/src/Http/Http/src/Features/FormFeature.cs +++ b/src/Http/Http/src/Features/FormFeature.cs @@ -70,15 +70,18 @@ private MediaTypeHeaderValue? ContentType { get { + MediaTypeHeaderValue? mt = default; + // Set directly if (Form is not null) { - _ = MediaTypeHeaderValue.TryParse("application/x-www-form-urlencoded", out var mt); + mt = new MediaTypeHeaderValue("application/x-www-form-urlencoded"); } else { - _ = MediaTypeHeaderValue.TryParse(_request.ContentType, out var mt); + _ = MediaTypeHeaderValue.TryParse(_request.ContentType, out mt); } + return mt; } } From f2ac33f1bd90cfe1612196773304e1c04987fe1d Mon Sep 17 00:00:00 2001 From: Jan Ove Halvorsen Date: Tue, 9 Jan 2024 15:50:08 +0100 Subject: [PATCH 05/10] Further tuning of the ContentType property getter and added a check in ReadFormAsync() to guard against _request equal to null --- src/Http/Http/src/Features/FormFeature.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Http/Http/src/Features/FormFeature.cs b/src/Http/Http/src/Features/FormFeature.cs index 30609d969a54..6d30224b747e 100644 --- a/src/Http/Http/src/Features/FormFeature.cs +++ b/src/Http/Http/src/Features/FormFeature.cs @@ -79,7 +79,10 @@ private MediaTypeHeaderValue? ContentType } else { - _ = MediaTypeHeaderValue.TryParse(_request.ContentType, out mt); + if (_request is not null) + { + _ = MediaTypeHeaderValue.TryParse(_request.ContentType, out mt); + } } return mt; @@ -158,6 +161,10 @@ public Task ReadFormAsync(CancellationToken cancellationToken) } else { + if (!HasFormContentType) + { + throw new InvalidOperationException("This request does not have a Content-Type header. Forms are available from requests with bodies like POSTs and a form Content-Type of either application/x-www-form-urlencoded or multipart/form-data."); + } _parsedFormTask = InnerReadFormAsync(cancellationToken); } } From 65e735f48b5751877de5e774945b9262a178ded1 Mon Sep 17 00:00:00 2001 From: Jan Ove Halvorsen Date: Tue, 9 Jan 2024 16:09:03 +0100 Subject: [PATCH 06/10] Improved the _request null check --- src/Http/Http/src/Features/FormFeature.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Http/Http/src/Features/FormFeature.cs b/src/Http/Http/src/Features/FormFeature.cs index 6d30224b747e..78fb2176ea81 100644 --- a/src/Http/Http/src/Features/FormFeature.cs +++ b/src/Http/Http/src/Features/FormFeature.cs @@ -161,10 +161,6 @@ public Task ReadFormAsync(CancellationToken cancellationToken) } else { - if (!HasFormContentType) - { - throw new InvalidOperationException("This request does not have a Content-Type header. Forms are available from requests with bodies like POSTs and a form Content-Type of either application/x-www-form-urlencoded or multipart/form-data."); - } _parsedFormTask = InnerReadFormAsync(cancellationToken); } } @@ -173,6 +169,11 @@ public Task ReadFormAsync(CancellationToken cancellationToken) private async Task InnerReadFormAsync(CancellationToken cancellationToken) { + if (_request is null) + { + throw new InvalidOperationException("Cannot read form from this request. Request is 'null'."); + } + HandleUncheckedAntiforgeryValidationFeature(); _options = _endpoint is null ? _options : GetFormOptionsFromMetadata(_options, _endpoint); From 59f4fb361f010dc9e5a4790e10a65b360b2b9cdc Mon Sep 17 00:00:00 2001 From: Jan Ove Halvorsen Date: Wed, 10 Jan 2024 09:32:07 +0100 Subject: [PATCH 07/10] Improved handling of content type and also changed the logic a bit --- src/Http/Http/src/Features/FormFeature.cs | 31 +++++++++++++---------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/src/Http/Http/src/Features/FormFeature.cs b/src/Http/Http/src/Features/FormFeature.cs index 78fb2176ea81..87233fa42bad 100644 --- a/src/Http/Http/src/Features/FormFeature.cs +++ b/src/Http/Http/src/Features/FormFeature.cs @@ -21,6 +21,7 @@ public class FormFeature : IFormFeature private FormOptions _options; private Task? _parsedFormTask; private IFormCollection? _form; + private MediaTypeHeaderValue? _formContentType; /// /// Initializes a new instance of . @@ -31,6 +32,7 @@ public FormFeature(IFormCollection form) ArgumentNullException.ThrowIfNull(form); Form = form; + _formContentType = new MediaTypeHeaderValue("application/x-www-form-urlencoded"); _options = FormOptions.Default; } @@ -70,22 +72,19 @@ private MediaTypeHeaderValue? ContentType { get { - MediaTypeHeaderValue? mt = default; + MediaTypeHeaderValue? mt = null; - // Set directly - if (Form is not null) - { - mt = new MediaTypeHeaderValue("application/x-www-form-urlencoded"); - } - else - { - if (_request is not null) - { - _ = MediaTypeHeaderValue.TryParse(_request.ContentType, out mt); - } - } + if (_request is not null) + { + _ = MediaTypeHeaderValue.TryParse(_request.ContentType, out mt); + } + + if (Form is not null && mt is null) + { + mt = _formContentType; + } - return mt; + return mt; } } @@ -124,6 +123,10 @@ public IFormCollection? Form { _parsedFormTask = null; _form = value; + if (_form is not null && _formContentType is null) + { + _formContentType = new MediaTypeHeaderValue("application/x-www-form-urlencoded"); + } } } From 7ebc49c9b9b369c5cb5f92ebe58f23db2032986b Mon Sep 17 00:00:00 2001 From: Jan Ove Halvorsen Date: Fri, 19 Jan 2024 08:45:46 +0100 Subject: [PATCH 08/10] Made some smaller changes based on comments received. _formContentType will now be set to null if Form is set to null. --- src/Http/Http/src/Features/FormFeature.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Http/Http/src/Features/FormFeature.cs b/src/Http/Http/src/Features/FormFeature.cs index 87233fa42bad..038dc4f317e5 100644 --- a/src/Http/Http/src/Features/FormFeature.cs +++ b/src/Http/Http/src/Features/FormFeature.cs @@ -79,7 +79,7 @@ private MediaTypeHeaderValue? ContentType _ = MediaTypeHeaderValue.TryParse(_request.ContentType, out mt); } - if (Form is not null && mt is null) + if (_form is not null && mt is null) { mt = _formContentType; } @@ -123,6 +123,10 @@ public IFormCollection? Form { _parsedFormTask = null; _form = value; + if (_form is null) + { + _formContentType = null; + } if (_form is not null && _formContentType is null) { _formContentType = new MediaTypeHeaderValue("application/x-www-form-urlencoded"); From bb2533696653516beebf8a650a79b37d80ced7a0 Mon Sep 17 00:00:00 2001 From: Jan Ove Halvorsen <110810564+johatuni@users.noreply.github.com> Date: Thu, 22 Feb 2024 14:18:02 +0100 Subject: [PATCH 09/10] Update src/Http/Http/src/Features/FormFeature.cs Co-authored-by: Andrew Casey --- src/Http/Http/src/Features/FormFeature.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Http/Http/src/Features/FormFeature.cs b/src/Http/Http/src/Features/FormFeature.cs index 038dc4f317e5..0b856f712070 100644 --- a/src/Http/Http/src/Features/FormFeature.cs +++ b/src/Http/Http/src/Features/FormFeature.cs @@ -129,7 +129,7 @@ public IFormCollection? Form } if (_form is not null && _formContentType is null) { - _formContentType = new MediaTypeHeaderValue("application/x-www-form-urlencoded"); + _formContentType ??= new MediaTypeHeaderValue("application/x-www-form-urlencoded"); } } } From 64e6a40ba80f5f81d2e82bfceb3badef85fc54ce Mon Sep 17 00:00:00 2001 From: Jan Ove Halvorsen <110810564+johatuni@users.noreply.github.com> Date: Thu, 22 Feb 2024 14:21:54 +0100 Subject: [PATCH 10/10] Apply suggestions from code review Co-authored-by: Andrew Casey --- src/Http/Http/src/Features/FormFeature.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Http/Http/src/Features/FormFeature.cs b/src/Http/Http/src/Features/FormFeature.cs index 0b856f712070..a7807f0ab9d1 100644 --- a/src/Http/Http/src/Features/FormFeature.cs +++ b/src/Http/Http/src/Features/FormFeature.cs @@ -21,7 +21,7 @@ public class FormFeature : IFormFeature private FormOptions _options; private Task? _parsedFormTask; private IFormCollection? _form; - private MediaTypeHeaderValue? _formContentType; + private MediaTypeHeaderValue? _formContentType; // null iff _form is null /// /// Initializes a new instance of . @@ -127,7 +127,7 @@ public IFormCollection? Form { _formContentType = null; } - if (_form is not null && _formContentType is null) + else { _formContentType ??= new MediaTypeHeaderValue("application/x-www-form-urlencoded"); }