From 3595452af7d2b13258dcaf40e408513c3da25003 Mon Sep 17 00:00:00 2001 From: Kiran Challa Date: Wed, 18 May 2016 16:54:13 -0700 Subject: [PATCH] Change priority for request token source lookup. Header token now takes priority over form field token. --- .../Internal/DefaultAntiforgeryTokenStore.cs | 16 ++-- .../DefaultAntiforgeryTokenStoreTest.cs | 96 +++++++------------ 2 files changed, 45 insertions(+), 67 deletions(-) diff --git a/src/Microsoft.AspNetCore.Antiforgery/Internal/DefaultAntiforgeryTokenStore.cs b/src/Microsoft.AspNetCore.Antiforgery/Internal/DefaultAntiforgeryTokenStore.cs index cd20eea956..17c2e1465a 100644 --- a/src/Microsoft.AspNetCore.Antiforgery/Internal/DefaultAntiforgeryTokenStore.cs +++ b/src/Microsoft.AspNetCore.Antiforgery/Internal/DefaultAntiforgeryTokenStore.cs @@ -44,8 +44,16 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal var cookieToken = httpContext.Request.Cookies[_options.CookieName]; + // We want to delay reading the form as much as possible, for example in case of large file uploads, + // request token could be part of the header. StringValues requestToken; - if (httpContext.Request.HasFormContentType) + if (_options.HeaderName != null) + { + requestToken = httpContext.Request.Headers[_options.HeaderName]; + } + + // Fall back to reading form instead + if (requestToken.Count == 0 && httpContext.Request.HasFormContentType) { // Check the content-type before accessing the form collection to make sure // we report errors gracefully. @@ -53,12 +61,6 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal requestToken = form[_options.FormFieldName]; } - // Fall back to header if the form value was not provided. - if (requestToken.Count == 0 && _options.HeaderName != null) - { - requestToken = httpContext.Request.Headers[_options.HeaderName]; - } - return new AntiforgeryTokenSet(requestToken, cookieToken, _options.FormFieldName, _options.HeaderName); } diff --git a/test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTokenStoreTest.cs b/test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTokenStoreTest.cs index 2b1f6152a3..09c27585fe 100644 --- a/test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTokenStoreTest.cs +++ b/test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTokenStoreTest.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Internal; @@ -99,39 +100,15 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal } [Fact] - public async Task GetRequestTokens_NonFormContentType_HeaderDisabled_ReturnsNullToken() - { - // Arrange - var httpContext = GetHttpContext("cookie-name", "cookie-value"); - httpContext.Request.ContentType = "application/json"; - - // Will not be accessed - httpContext.Request.Form = null; - - var options = new AntiforgeryOptions() - { - CookieName = "cookie-name", - FormFieldName = "form-field-name", - HeaderName = null, - }; - - var tokenStore = new DefaultAntiforgeryTokenStore(new TestOptionsManager(options)); - - // Act - var tokenSet = await tokenStore.GetRequestTokensAsync(httpContext); - - // Assert - Assert.Equal("cookie-value", tokenSet.CookieToken); - Assert.Null(tokenSet.RequestToken); - } - - [Fact] - public async Task GetRequestTokens_FormContentType_FallbackHeaderToken() + public async Task GetRequestTokens_HeaderTokenTakensPriority_OverFormToken() { // Arrange var httpContext = GetHttpContext("cookie-name", "cookie-value"); httpContext.Request.ContentType = "application/x-www-form-urlencoded"; - httpContext.Request.Form = FormCollection.Empty; + httpContext.Request.Form = new FormCollection(new Dictionary + { + { "form-field-name", "form-value" }, + }); // header value has priority. httpContext.Request.Headers.Add("header-name", "header-value"); var options = new AntiforgeryOptions() @@ -151,6 +128,34 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal Assert.Equal("header-value", tokens.RequestToken); } + [Fact] + public async Task GetRequestTokens_NoHeaderToken_FallsBackToFormToken() + { + // Arrange + var httpContext = GetHttpContext("cookie-name", "cookie-value"); + httpContext.Request.ContentType = "application/x-www-form-urlencoded"; + httpContext.Request.Form = new FormCollection(new Dictionary + { + { "form-field-name", "form-value" }, + }); + + var options = new AntiforgeryOptions() + { + CookieName = "cookie-name", + FormFieldName = "form-field-name", + HeaderName = "header-name", + }; + + var tokenStore = new DefaultAntiforgeryTokenStore(new TestOptionsManager(options)); + + // Act + var tokens = await tokenStore.GetRequestTokensAsync(httpContext); + + // Assert + Assert.Equal("cookie-value", tokens.CookieToken); + Assert.Equal("form-value", tokens.RequestToken); + } + [Fact] public async Task GetRequestTokens_NonFormContentType_UsesHeaderToken() { @@ -180,7 +185,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal } [Fact] - public async Task GetRequestTokens_NonFormContentType_NoHeaderToken_ReturnsNullToken() + public async Task GetRequestTokens_NoHeaderToken_NonFormContentType_ReturnsNullToken() { // Arrange var httpContext = GetHttpContext("cookie-name", "cookie-value"); @@ -207,7 +212,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal } [Fact] - public async Task GetRequestTokens_BothFieldsEmpty_ReturnsNullTokens() + public async Task GetRequestTokens_BothHeaderValueAndFormFieldsEmpty_ReturnsNullTokens() { // Arrange var httpContext = GetHttpContext("cookie-name", "cookie-value"); @@ -231,35 +236,6 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal Assert.Null(tokenSet.RequestToken); } - [Fact] - public async Task GetFormToken_FormFieldIsValid_ReturnsToken() - { - // Arrange - var httpContext = GetHttpContext("cookie-name", "cookie-value"); - httpContext.Request.ContentType = "application/x-www-form-urlencoded"; - httpContext.Request.Form = new FormCollection(new Dictionary - { - { "form-field-name", "form-value" }, - }); - httpContext.Request.Headers.Add("header-name", "header-value"); // form value has priority. - - var options = new AntiforgeryOptions() - { - CookieName = "cookie-name", - FormFieldName = "form-field-name", - HeaderName = "header-name", - }; - - var tokenStore = new DefaultAntiforgeryTokenStore(new TestOptionsManager(options)); - - // Act - var tokens = await tokenStore.GetRequestTokensAsync(httpContext); - - // Assert - Assert.Equal("cookie-value", tokens.CookieToken); - Assert.Equal("form-value", tokens.RequestToken); - } - [Theory] [InlineData(true, true)] [InlineData(false, null)]