Adapt RFC 6265 in path handling for cookies

src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Cookies.cs

@@ -293,18 +293,19 @@ public async Task GetAsyncWithRedirect_SetCookieContainer_CorrectCookiesSent()
         {
             const string path1 = "/foo";
             const string path2 = "/bar";
+            const string unusedPath = "/unused";
 
             await LoopbackServerFactory.CreateClientAndServerAsync(async url =>
             {
                 Uri url1 = new Uri(url, path1);
                 Uri url2 = new Uri(url, path2);
-                Uri unusedUrl = new Uri(url, "/unused");
+                Uri unusedUrl = new Uri(url, unusedPath);
 
                 HttpClientHandler handler = CreateHttpClientHandler();
                 handler.CookieContainer = new CookieContainer();
-                handler.CookieContainer.Add(url1, new Cookie("cookie1", "value1"));
-                handler.CookieContainer.Add(url2, new Cookie("cookie2", "value2"));
-                handler.CookieContainer.Add(unusedUrl, new Cookie("cookie3", "value3"));
+                handler.CookieContainer.Add(url1, new Cookie("cookie1", "value1", path1));
+                handler.CookieContainer.Add(url2, new Cookie("cookie2", "value2", path2));
+                handler.CookieContainer.Add(unusedUrl, new Cookie("cookie3", "value3", unusedPath));
 
                 using (HttpClient client = CreateHttpClient(handler))
                 {
@@ -393,6 +394,111 @@ public async Task GetAsync_ReceiveMultipleSetCookieHeaders_CookieAdded()
             });
         }
 
+        // Default path should be calculated according to https://tools.ietf.org/html/rfc6265#section-5.1.4
+        // When a cookie is being sent without an explicitly defined Path for a URL with URL-Path /path/sub,
+        // the cookie should be added with Path=/path.
+        [Fact]
+        public async Task GetAsync_NoPathDefined_CookieAddedWithDefaultPath()
+        {
+            await LoopbackServerFactory.CreateServerAsync(async (server, serverUrl) =>
+            {
+                Uri requestUrl = new Uri(serverUrl, "path/sub");
+                HttpClientHandler handler = CreateHttpClientHandler();
+
+                using (HttpClient client = CreateHttpClient(handler))
+                {
+                    Task<HttpResponseMessage> getResponseTask = client.GetAsync(requestUrl);
+                    Task<HttpRequestData> serverTask = server.HandleRequestAsync(
+                        HttpStatusCode.OK,
+                        new HttpHeaderData[]
+                        {
+                            new HttpHeaderData("Set-Cookie", "A=1"),
+                        },
+                        s_simpleContent);
+                    await TestHelper.WhenAllCompletedOrAnyFailed(getResponseTask, serverTask);
+
+                    CookieCollection collection = handler.CookieContainer.GetCookies(requestUrl);
+                    Cookie cookie = collection.Single();
+                    Assert.Equal("/path", cookie.Path);
+                }
+            });
+        }
+
+        // According to RFC6265, cookie path is not expected to match the request's path,
+        // these cookies should be accepted by the client.
+        [Fact]
+        public async Task GetAsync_CookiePathDoesNotMatchRequestPath_CookieAccepted()
+        {
+            await LoopbackServerFactory.CreateServerAsync(async (server, serverUrl) =>
+            {
+                Uri requestUrl = new Uri(serverUrl, "original");
+                Uri otherUrl = new Uri(serverUrl, "other");
+                HttpClientHandler handler = CreateHttpClientHandler();
+
+                using (HttpClient client = CreateHttpClient(handler))
+                {
+                    Task<HttpResponseMessage> getResponseTask = client.GetAsync(requestUrl);
+                    Task<HttpRequestData> serverTask = server.HandleRequestAsync(
+                        HttpStatusCode.OK,
+                        new[]
+                        {
+                            new HttpHeaderData("Set-Cookie", "A=1; Path=/other"),
+                        },
+                        s_simpleContent);
+                    await TestHelper.WhenAllCompletedOrAnyFailed(getResponseTask, serverTask);
+
+                    CookieCollection collection = handler.CookieContainer.GetCookies(otherUrl);
+                    Cookie cookie = collection.Single();
+                    Assert.Equal("/other", cookie.Path);
+                }
+            });
+        }
+
+        // Based on the OIDC login scenario described comments:
+        // https://github.com/dotnet/runtime/pull/39250#issuecomment-659783480
+        // https://github.com/dotnet/runtime/issues/26141#issuecomment-612097147
+        [Fact]
+        public async Task GetAsync_Redirect_CookiesArePreserved()
+        {
+            HttpClientHandler handler = CreateHttpClientHandler();
+
+            string loginPath = "/login/user";
+            string returnPath = "/return";
+
+            await LoopbackServerFactory.CreateClientAndServerAsync(async serverUrl =>
+                {
+                    Uri loginUrl = new Uri(serverUrl, loginPath);
+                    Uri returnUrl = new Uri(serverUrl, returnPath);
+                    CookieContainer cookies = handler.CookieContainer;
+
+                    using (HttpClient client = CreateHttpClient(handler))
+                    {
+                        client.DefaultRequestHeaders.ConnectionClose = true; // to avoid issues with connection pooling
+                        HttpResponseMessage response = await client.GetAsync(loginUrl);
+                        string content = await response.Content.ReadAsStringAsync();
+                        Assert.Equal(s_simpleContent, content);
+
+                        Cookie cookie = cookies.GetCookies(returnUrl).Single();
+                        Assert.Equal("LoggedIn", cookie.Name);
+                    }
+                },
+                async server =>
+                {
+                    HttpRequestData requestData1 = await server.HandleRequestAsync(HttpStatusCode.Found, new[]
+                    {
+                        new HttpHeaderData("Location", returnPath),
+                        new HttpHeaderData("Set-Cookie", "LoggedIn=true; Path=/return"),
+                    });
+
+                    Assert.Equal(0, requestData1.GetHeaderValueCount("Cookie"));
+
+                    HttpRequestData requestData2 = await server.HandleRequestAsync(content: s_simpleContent, headers: new[]{
+                        new HttpHeaderData("Set-Cookie", "LoggedIn=true; Path=/return"),
+                    });
+                    Assert.Equal("LoggedIn=true", requestData2.GetSingleHeaderValue("Cookie"));
+                });
+        }
+
         [Fact]
         public async Task GetAsync_ReceiveSetCookieHeader_CookieUpdated()
         {

src/libraries/System.Net.Primitives/src/System/Net/Cookie.cs

@@ -498,7 +498,24 @@ internal bool VerifySetDefaults(CookieVariant variant, Uri uri, bool isLocalDoma
                 switch (m_cookieVariant)
                 {
                     case CookieVariant.Plain:
-                        m_path = path;
+                        // As per RFC6265 5.1.4. (https://tools.ietf.org/html/rfc6265#section-5.1.4):
+                        // | 2. If the uri-path is empty or if the first character of the uri-
+                        // |    path is not a %x2F ("/") character, output %x2F ("/") and skip
+                        // |    the remaining steps.
+                        // | 3. If the uri-path contains no more than one %x2F ("/") character,
+                        // |    output %x2F ("/") and skip the remaining step.
+                        // Note: Normally Uri.AbsolutePath contains at least one "/" after parsing,
+                        //       but it's possible construct Uri with an empty path using a custom UriParser
+                        int lastSlash;
+                        if (path.Length == 0 || path[0] != '/' || (lastSlash = path.LastIndexOf('/')) == 0)
+                        {
+                            m_path = "/";
+                            break;
+                        }
+
+                        // | 4. Output the characters of the uri-path from the first character up
+                        // |    to, but not including, the right-most %x2F ("/").
+                        m_path = path.Substring(0, lastSlash);
                         break;
                     case CookieVariant.Rfc2109:
                         m_path = path.Substring(0, path.LastIndexOf('/')); // May be empty
@@ -511,18 +528,6 @@ internal bool VerifySetDefaults(CookieVariant variant, Uri uri, bool isLocalDoma
                         break;
                 }
             }
-            else
-            {
-                // Check current path (implicit/explicit) against given URI.
-                if (!path.StartsWith(CookieParser.CheckQuoted(m_path), StringComparison.Ordinal))
-                {
-                    if (shouldThrow)
-                    {
-                        throw new CookieException(SR.Format(SR.net_cookie_attribute, CookieFields.PathAttributeName, m_path));
-                    }
-                    return false;
-                }
-            }
 
             // Set the default port if Port attribute was present but had no value.
             if (setDefault && (m_port_implicit == false && m_port.Length == 0))

src/libraries/System.Net.Primitives/src/System/Net/CookieContainer.cs

@@ -6,6 +6,7 @@
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.IO;
+using System.Linq;
 using System.Net.NetworkInformation;
 using System.Text;
 
@@ -888,7 +889,7 @@ private void BuildCookieCollectionFromDomainMatches(Uri uri, bool isSecure, int
                     for (int e = 0; e < listCount; e++)
                     {
                         string path = (string)list.GetKey(e);
-                        if (uri.AbsolutePath.StartsWith(CookieParser.CheckQuoted(path), StringComparison.Ordinal))
+                        if (PathMatch(uri.AbsolutePath, path))
                         {
                             CookieCollection cc = (CookieCollection)list.GetByIndex(e)!;
                             cc.TimeStamp(CookieCollection.Stamp.Set);
@@ -908,6 +909,26 @@ private void BuildCookieCollectionFromDomainMatches(Uri uri, bool isSecure, int
             }
         }
 
+        // Implement path-matching according to https://tools.ietf.org/html/rfc6265#section-5.1.4:
+        // | A request-path path-matches a given cookie-path if at least one of the following conditions holds:
+        // | - The cookie-path and the request-path are identical.
+        // | - The cookie-path is a prefix of the request-path, and the last character of the cookie-path is %x2F ("/").
+        // | - The cookie-path is a prefix of the request-path, and the first character of the request-path that is not included in the cookie-path is a %x2F ("/") character.
+        // The latter conditions are needed to make sure that
+        // PathMatch("/fooBar, "/foo") == false
+        // but:
+        // PathMatch("/foo/bar", "/foo") == true, PathMatch("/foo/bar", "/foo/") == true
+        private static bool PathMatch(string requestPath, string cookiePath)
+        {
+            cookiePath = CookieParser.CheckQuoted(cookiePath);
+
+            if (!requestPath.StartsWith(cookiePath, StringComparison.Ordinal))
+                return false;
+            return requestPath.Length == cookiePath.Length ||
+                   cookiePath.Length > 0 && cookiePath[^1] == '/' ||
+                   requestPath[cookiePath.Length] == '/';
+        }
+
         private void MergeUpdateCollections(ref CookieCollection? destination, CookieCollection source, int port, bool isSecure, bool isPlainOnly)
         {
             lock (source)

src/libraries/System.Net.Primitives/tests/FunctionalTests/CookieContainerTest.cs

@@ -140,7 +140,6 @@ public static IEnumerable<object[]> SetCookiesInvalidData()
             yield return new object[] { u5, "$=value" }; // Invalid name
             yield return new object[] { new Uri("http://url.com"), "na\tme=value; domain=.domain.com" }; // Invalid name
             yield return new object[] { new Uri("http://url.com"), "name=value; domain=.domain.com" }; // Domain not the same
-            yield return new object[] { new Uri("http://url.com/path"), "name=value; domain=.url.com; path=/root" }; // Path not the same
             yield return new object[] { new Uri("http://url.com:90"), "name=value; port=\"80\"" }; // Port not the same
             yield return new object[] { new Uri("http://url.com"), "name=value; domain=" }; // Empty domain
             yield return new object[] { u6, "name11=value11; version=invalidversion" }; // Invalid version

src/libraries/System.Net.Primitives/tests/UnitTests/CookieContainerTest.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using System.Net;
 using System.Threading.Tasks;
 
@@ -36,6 +37,18 @@ public class CookieContainerTest
         private const string CookieValue1 = "CookieValue1";
         private const string CookieValue2 = "CookieValue2";
 
+        // Creates cookies as following:
+        // c1  | .url1.com     | name1=value
+        // c2  | .url1.com     | name2=value; Secure=True
+        // c3  | .url1.com     | name3=value; $Version=1;  $Domain=.url1.com; $Port="80, 90, 100, 443"
+        // c4  | 127.0.0.1     | name4=value
+        // c5  | 127.0.0.1     | name4=value; Path=/path
+        // c6  | .url3.com     | name6=value
+        // c7  | .url3.com     | name7=value
+        // c8  | url4.com      | name8=value
+        // c9  | url4.com      | name9=value
+        // c10 | url5.com/path | name10=value
+        // c11 | url5.com/path | name11=value; $Version=1;  $Port="80, 90, 100"
         private CookieContainer CreateCount11Container()
         {
             CookieContainer cc1 = new CookieContainer();
@@ -100,9 +113,9 @@ public static IEnumerable<object[]> SetCookiesData()
                 new Cookie[]
                 {
                     c10,
+                    c11,
                     new Cookie("name98", "value98"),
                     new Cookie("name99", "value99"),
-                    c11
                 }
             }; // Simple
 
@@ -636,7 +649,7 @@ public void GetCookies_DifferentPaths_GetsMatchedPathsIncludingEmptyPath()
         {
             // Internally, paths are stored alphabetically sorted - so expect a cookie collection sorted by the path of each cookie
             // This test ensures that all cookies with paths (that the path specified by the uri starts with) are returned
-            Cookie c1 = new Cookie("name1", "value", "/aa", ".url.com");
+            Cookie c1 = new Cookie("name1", "value", "/a/a", ".url.com");
             Cookie c2 = new Cookie("name2", "value", "/a", ".url.com");
             Cookie c3 = new Cookie("name3", "value", "/b", ".url.com"); // Should be ignored - no match with the URL's path
             Cookie c4 = new Cookie("name4", "value", "/", ".url.com"); // Should NOT be ignored (has no path specified)
@@ -647,7 +660,7 @@ public void GetCookies_DifferentPaths_GetsMatchedPathsIncludingEmptyPath()
             cc1.Add(c3);
             cc1.Add(c4);
 
-            CookieCollection cc2 = cc1.GetCookies(new Uri("http://url.com/aaa"));
+            CookieCollection cc2 = cc1.GetCookies(new Uri("http://url.com/a/a/a"));
             Assert.Equal(3, cc2.Count);
             Assert.Equal(c1, cc2[0]);
             Assert.Equal(c2, cc2[1]);
@@ -762,5 +775,81 @@ private void VerifyGetCookies(CookieContainer cc1, Uri uri, Cookie[] expected)
                 Assert.Equal(c1.Value, c2.Value);
             }
         }
+
+        // Test default-path calculation as defined in
+        // https://tools.ietf.org/html/rfc6265#section-5.1.4
+        public static readonly TheoryData<string, string> DefaultPathData = new TheoryData<string, string>()
+        {
+            {"http://url1.com", "/"},
+            {"http://url1.com/abc", "/"},
+            {"http://url1.com/abc/xy", "/abc"},
+            {"http://url1.com/abc/xy/foo", "/abc/xy"},
+            {"http://127.0.0.1", "/"},
+            {"http://127.0.0.1/a/s/d/f", "/a/s/d"},
+        };
+
+        [Theory]
+        [MemberData(nameof(DefaultPathData))]
+        public void GetCookies_DefaultPathCalculationFollowsRfc6265(string uriString, string expectedPath)
+        {
+            Cookie cookie = new Cookie("n", "v");
+            Uri uri = new Uri(uriString);
+
+            CookieContainer container = new CookieContainer();
+            container.Add(uri, cookie);
+
+            Cookie actualCookie = container.GetCookies(uri).Single();
+            Assert.Equal(expectedPath, actualCookie.Path);
+        }
+
+        // Test path-match as defined in
+        // https://tools.ietf.org/html/rfc6265#section-5.1.4
+        public static readonly TheoryData<bool, string[], string, int> PathMatchData = new TheoryData<bool, string[], string, int>
+        {
+            {false, new [] {"/"}, "/", 1},
+            {false, new [] {"/asd/fg/hjk/l", "/x"}, "/asd/fg/hjk/l", 1},
+            {false, new [] {"/a", "/x"}, "/a/hello", 1},
+            {false, new [] {"/a/foo", "/a/lol"}, "/a/foo/1/2/3", 1},
+            {false, new [] {"/a/", "/x"}, "/a/hello", 1},
+            {false, new [] {"", "/x"}, "/y", 1},
+            {false, new [] {"//"}, "/", 0},
+            {false, new [] {"//"}, "//", 1},
+            {false, new [] {"", "/", "//", "///"}, "///", 4},
+
+            // Should not match the second half of the criteria:
+            // "The cookie-path is a prefix of the request-path, and the first
+            // character of the request-path that is not included in the cookie-
+            // path is a %x2F ("/") character."
+            {false, new [] {"/a/foo", "/x"}, "/a/foo123", 0},
+
+            {true, new [] {"/"}, "/", 1},
+            {true, new [] {"/a/b", "/a/c", "/x/y"}, "/a/hello", 2},
+            {true, new [] {"/a/foo/b", "/a/foo/c", "/a/foo/42", "/a/lol/42"}, "/a/foo/hello", 3},
+        };
+
+        [Theory]
+        [MemberData(nameof(PathMatchData))]
+        public void GetCookies_PathMatchingFollowsRfc6265(bool useDefaultPath, string[] cookiePaths, string requestPath, int expectedMatches)
+        {
+            CookieContainer container = new CookieContainer();
+
+            int i = 0;
+            foreach (string cookiePath in cookiePaths)
+            {
+                if (useDefaultPath)
+                {
+                    container.Add(new Uri("http://test.com" + cookiePath), new Cookie($"c{i}", "value"));
+                }
+                else
+                {
+                    container.Add(new Uri("http://test.com"), new Cookie($"c{i}", "value", cookiePath));
+                }
+                i++;
+            }
+
+            Uri requestUri = new Uri("http://test.com" + requestPath);
+            CookieCollection collection = container.GetCookies(requestUri);
+            Assert.Equal(expectedMatches, collection.Count);
+        }
     }
 }