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,130 @@ 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()
+        {
+            if (PlatformDetection.IsNetFramework)
+            {
+                // CookieContainer does not follow RFC6265 on .NET Framework,
+                // therefore WinHttpHandler is not expected to function correctly
+                return;
+            }
+
+            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);
+
+                    Cookie cookie = GetFirstCookie(handler.CookieContainer, requestUrl);
+                    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()
+        {
+            if (PlatformDetection.IsNetFramework)
+            {
+                // CookieContainer does not follow RFC6265 on .NET Framework,
+                // therefore WinHttpHandler is not expected to function correctly
+                return;
+            }
+
+            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);
+
+                    Cookie cookie = GetFirstCookie(handler.CookieContainer, otherUrl);
+                    Assert.Equal("/other", cookie.Path);
+                }
+            });
+        }
+
+        // Based on the OIDC login scenario described in 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()
+        {
+            if (PlatformDetection.IsNetFramework)
+            {
+                // CookieContainer does not follow RFC6265 on .NET Framework,
+                // therefore WinHttpHandler is not expected to function correctly
+                return;
+            }
+
+            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 = GetFirstCookie(cookies, returnUrl);
+                        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()
         {
@@ -585,6 +710,18 @@ public async Task GetAsyncWithBasicAuth_ReceiveSetCookie_CookieSent()
             });
         }
 
+        // Workaround for Framework, where CookieCollection does not implement IEnumerable<Cookie>
+        static Cookie GetFirstCookie(CookieContainer container, Uri uri)
+        {
+            CookieCollection coll = container.GetCookies(uri);
+            System.Collections.IEnumerator e = coll.GetEnumerator();
+            if (e.MoveNext())
+            {
+                return (Cookie)e.Current;
+            }
+            throw new Exception($"No cookies found for for {uri}");
+        }
+
         //
         // MemberData stuff
         //

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