From a3c06b00ccc5eca95b38b70e4b22ab4f29527242 Mon Sep 17 00:00:00 2001 From: Kiran Challa Date: Tue, 8 Nov 2016 09:31:34 -0800 Subject: [PATCH] Fixes CookieTempDataProvider to handle empty request PathBase while setting cookie options --- .../ViewFeatures/CookieTempDataProvider.cs | 14 +++- .../TempDataInCookiesTest.cs | 83 +++++++++++++++++-- .../CookieTempDataProviderTest.cs | 14 +++- 3 files changed, 101 insertions(+), 10 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/CookieTempDataProvider.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/CookieTempDataProvider.cs index 81b012cb32..e59f37175e 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/CookieTempDataProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/CookieTempDataProvider.cs @@ -63,7 +63,9 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures var cookieOptions = new CookieOptions() { - Path = string.IsNullOrEmpty(_options.Value.Path) ? context.Request.PathBase.ToString() : _options.Value.Path, + // Check for PathBase as it can empty in which case the clients would not send the cookie + // in subsequent requests. + Path = string.IsNullOrEmpty(_options.Value.Path) ? GetPathBase(context) : _options.Value.Path, Domain = string.IsNullOrEmpty(_options.Value.Domain) ? null : _options.Value.Domain, HttpOnly = true, Secure = true @@ -82,5 +84,15 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures _chunkingCookieManager.DeleteCookie(context, CookieName, cookieOptions); } } + + private string GetPathBase(HttpContext httpContext) + { + var pathBase = httpContext.Request.PathBase.ToString(); + if (string.IsNullOrEmpty(pathBase)) + { + pathBase = "/"; + } + return pathBase; + } } } diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/TempDataInCookiesTest.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/TempDataInCookiesTest.cs index 5df7dcfa2c..3eedb90ff3 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/TempDataInCookiesTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/TempDataInCookiesTest.cs @@ -1,6 +1,8 @@ // Copyright (c) .NET Foundation. All rights reserved. // 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.Linq; using System.Net; using System.Net.Http; @@ -37,16 +39,26 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests // Assert 1 Assert.Equal(HttpStatusCode.OK, response.StatusCode); - var cookies = response.Headers.GetValues(HeaderNames.SetCookie); - var cookieTempDataProviderCookies = cookies.Where(cookie => cookie.Contains(CookieTempDataProvider.CookieName)); - Assert.NotNull(cookieTempDataProviderCookies); - + IEnumerable setCookieValues; + Assert.True(response.Headers.TryGetValues(HeaderNames.SetCookie, out setCookieValues)); + setCookieValues = setCookieValues.Where(cookie => cookie.Contains(CookieTempDataProvider.CookieName)); + Assert.NotEmpty(setCookieValues); // Verify that all the cookies from CookieTempDataProvider are within the maximum size - foreach (var cookie in cookieTempDataProviderCookies) + foreach (var cookie in setCookieValues) { Assert.True(cookie.Length <= ChunkingCookieManager.DefaultChunkSize); } + var cookieTempDataProviderCookies = setCookieValues + .Select(setCookieValue => SetCookieHeaderValue.Parse(setCookieValue)); + foreach (var cookieTempDataProviderCookie in cookieTempDataProviderCookies) + { + Assert.NotNull(cookieTempDataProviderCookie.Value); + Assert.Equal("/", cookieTempDataProviderCookie.Path); + Assert.Null(cookieTempDataProviderCookie.Domain); + Assert.True(cookieTempDataProviderCookie.Secure); + } + // Act 2 response = await Client.SendAsync(GetRequest("/TempData/GetLargeValueFromTempData", response)); @@ -54,6 +66,16 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests Assert.Equal(HttpStatusCode.OK, response.StatusCode); var body = await response.Content.ReadAsStringAsync(); Assert.Equal(expected, body); + Assert.True(response.Headers.TryGetValues(HeaderNames.SetCookie, out setCookieValues)); + var setCookieHeaderValue = setCookieValues + .Select(setCookieValue => SetCookieHeaderValue.Parse(setCookieValue)) + .FirstOrDefault(setCookieHeader => setCookieHeader.Name == CookieTempDataProvider.CookieName); + Assert.NotNull(setCookieHeaderValue); + Assert.Equal(string.Empty, setCookieHeaderValue.Value); + Assert.Equal("/", setCookieHeaderValue.Path); + Assert.Null(setCookieHeaderValue.Domain); + Assert.NotNull(setCookieHeaderValue.Expires); + Assert.True(setCookieHeaderValue.Expires < DateTimeOffset.Now); // expired cookie // Act 3 response = await Client.SendAsync(GetRequest("/TempData/GetLargeValueFromTempData", response)); @@ -61,5 +83,56 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests // Assert 3 Assert.Equal(HttpStatusCode.NoContent, response.StatusCode); } + + [Fact] + public async Task Redirect_RetainsTempData_EvenIfAccessed_AndSetsAppropriateCookieValues() + { + // Arrange + var nameValueCollection = new List> + { + new KeyValuePair("value", "Foo"), + }; + var content = new FormUrlEncodedContent(nameValueCollection); + + // Act 1 + var response = await Client.PostAsync("/TempData/SetTempData", content); + + // Assert 1 + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + IEnumerable setCookieValues; + Assert.True(response.Headers.TryGetValues(HeaderNames.SetCookie, out setCookieValues)); + var setCookieHeader = setCookieValues + .Select(setCookieValue => SetCookieHeaderValue.Parse(setCookieValue)) + .FirstOrDefault(setCookieHeaderValue => setCookieHeaderValue.Name == CookieTempDataProvider.CookieName); + Assert.NotNull(setCookieHeader); + Assert.Equal("/", setCookieHeader.Path); + Assert.Null(setCookieHeader.Domain); + Assert.True(setCookieHeader.Secure); + Assert.Null(setCookieHeader.Expires); + + // Act 2 + var redirectResponse = await Client.SendAsync(GetRequest("/TempData/GetTempDataAndRedirect", response)); + + // Assert 2 + Assert.Equal(HttpStatusCode.Redirect, redirectResponse.StatusCode); + + // Act 3 + response = await Client.SendAsync(GetRequest(redirectResponse.Headers.Location.ToString(), response)); + + // Assert 3 + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var body = await response.Content.ReadAsStringAsync(); + Assert.Equal("Foo", body); + Assert.True(response.Headers.TryGetValues(HeaderNames.SetCookie, out setCookieValues)); + setCookieHeader = setCookieValues + .Select(setCookieValue => SetCookieHeaderValue.Parse(setCookieValue)) + .FirstOrDefault(setCookieHeaderValue => setCookieHeaderValue.Name == CookieTempDataProvider.CookieName); + Assert.NotNull(setCookieHeader); + Assert.Equal(string.Empty, setCookieHeader.Value); + Assert.Equal("/", setCookieHeader.Path); + Assert.Null(setCookieHeader.Domain); + Assert.NotNull(setCookieHeader.Expires); + Assert.True(setCookieHeader.Expires < DateTimeOffset.Now); // expired cookie + } } } \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ViewFeatures/CookieTempDataProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ViewFeatures/CookieTempDataProviderTest.cs index ef9b0e2e54..6d9a646686 100644 --- a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ViewFeatures/CookieTempDataProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ViewFeatures/CookieTempDataProviderTest.cs @@ -91,9 +91,13 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures } [Theory] - [InlineData("/")] - [InlineData("/vdir1")] - public void SaveTempData_DefaultProviderOptions_SetsCookie_WithAppropriateCookieOptions(string pathBase) + [InlineData(null, "/")] + [InlineData("", "/")] + [InlineData("/", "/")] + [InlineData("/vdir1", "/vdir1")] + public void SaveTempData_DefaultProviderOptions_SetsCookie_WithAppropriateCookieOptions( + string pathBase, + string expectedCookiePath) { // Arrange var values = new Dictionary(); @@ -121,7 +125,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures Assert.NotNull(cookieInfo); Assert.Equal(expectedDataInCookie, cookieInfo.Value); Assert.Equal(expectedDataToProtect, dataProtector.PlainTextToProtect); - Assert.Equal(pathBase, cookieInfo.Options.Path); + Assert.Equal(expectedCookiePath, cookieInfo.Options.Path); Assert.True(cookieInfo.Options.Secure); Assert.True(cookieInfo.Options.HttpOnly); Assert.Null(cookieInfo.Options.Expires); @@ -129,6 +133,8 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures } [Theory] + [InlineData(null, null, null, "/", null)] + [InlineData("", null, null, "/", null)] [InlineData("/", null, null, "/", null)] [InlineData("/", "/vdir1", null, "/vdir1", null)] [InlineData("/", "/vdir1", ".abc.com", "/vdir1", ".abc.com")]