xamarin / xamarin-macios

.NET for iOS, Mac Catalyst, macOS, and tvOS provide open-source bindings of the Apple SDKs for use with .NET managed languages such as C#
Other
2.42k stars 507 forks source link

NSUrlSessionHandler not clearing cookies #9511

Open valentingrigorean opened 3 years ago

valentingrigorean commented 3 years ago

Steps to Reproduce

  1. Create a cookie container
  2. After an action (logout) clear the container( setting cookies expired = true)

Expected Behavior

The next request should have no cookies set.

Actual Behavior

Cookies are still set from previous requests.

Environment

=== Visual Studio Community 2019 for Mac ===

Version 8.7.4 (build 38)
Installation UUID: 9bba0c99-97e0-42af-9c71-da7f4f63c4c6
    GTK+ 2.24.23 (Raleigh theme)
    Xamarin.Mac 6.18.0.23 (d16-6 / 088c73638)

    Package version: 612000090

=== Mono Framework MDK ===

Runtime:
    Mono 6.12.0.90 (2020-02/d3daacdaa80) (64-bit)
    Package version: 612000090

=== Roslyn (Language Service) ===

3.7.0-6.20412.3+d3c3a44a4e7ad31cc75c59be0d3df4a19ff33878

=== NuGet ===

Version: 5.7.0.6702

=== .NET Core SDK ===

SDK: /usr/local/share/dotnet/sdk/3.1.401/Sdks
SDK Versions:
    3.1.401
    3.1.302
    3.1.301
    3.1.300
MSBuild SDKs: /Library/Frameworks/Mono.framework/Versions/6.12.0/lib/mono/msbuild/Current/bin/Sdks

=== .NET Core Runtime ===

Runtime: /usr/local/share/dotnet/dotnet
Runtime Versions:
    3.1.7
    3.1.6
    3.1.5
    3.1.4
    2.1.21
    2.1.20
    2.1.19
    2.1.18

=== Xamarin.Profiler ===

Version: 1.6.15.68
Location: /Applications/Xamarin Profiler.app/Contents/MacOS/Xamarin Profiler

=== Updater ===

Version: 11

=== Xamarin Designer ===

Version: 16.7.0.492
Hash: f5afe667d
Branch: remotes/origin/d16-7-vsmac
Build date: 2020-07-10 18:42:54 UTC

=== Apple Developer Tools ===

Xcode 11.6 (16141)
Build 11E708

=== Xamarin.Mac ===

Xamarin.Mac not installed. Can't find /Library/Frameworks/Xamarin.Mac.framework/Versions/Current/Version.

=== Xamarin.iOS ===

Version: 13.20.2.2 (Visual Studio Community)
Hash: 817b6f72a
Branch: d16-7
Build date: 2020-07-18 18:45:00-0400

=== Xamarin.Android ===

Version: 11.0.2.0 (Visual Studio Community)
Commit: xamarin-android/d16-7/025fde9
Android SDK: /Users/valentingrigorean/Library/Developer/Xamarin/android-sdk-macosx
    Supported Android versions:
        None installed

SDK Tools Version: 26.1.1
SDK Platform Tools Version: 30.0.4
SDK Build Tools Version: 30.0.2

Build Information: 
Mono: 83105ba
Java.Interop: xamarin/java.interop/d16-7@1f3388a
ProGuard: Guardsquare/proguard/proguard6.2.2@ebe9000
SQLite: xamarin/sqlite/3.32.1@1a3276b
Xamarin.Android Tools: xamarin/xamarin-android-tools/d16-7@017078f

=== Microsoft OpenJDK for Mobile ===

Java SDK: /Users/valentingrigorean/Library/Developer/Xamarin/jdk/microsoft_dist_openjdk_1.8.0.25
1.8.0-25
Android Designer EPL code available here:
https://github.com/xamarin/AndroidDesigner.EPL

=== Android SDK Manager ===

Version: 16.7.0.13
Hash: 8380518
Branch: remotes/origin/d16-7~2
Build date: 2020-08-19 22:18:28 UTC

=== Android Device Manager ===

Version: 16.7.0.24
Hash: bb090a3
Branch: remotes/origin/d16-7
Build date: 2020-08-19 22:18:52 UTC

=== Build Information ===

Release ID: 807040038
Git revision: 7f322926a2e0a5fa0c1e3833215a191883f7514a
Build date: 2020-08-20 10:10:20-04
Build branch: release-8.7
Xamarin extensions: 7f322926a2e0a5fa0c1e3833215a191883f7514a

=== Operating System ===

Mac OS X 10.15.6
Darwin 19.6.0 Darwin Kernel Version 19.6.0
    Thu Jun 18 20:49:00 PDT 2020
    root:xnu-6153.141.1~1/RELEASE_X86_64 x86_64

Build Logs

Example Project (If Possible)

valentingrigorean commented 3 years ago

The issue is here I think.

Current Impl:

if (session.Configuration.HttpCookieStorage != null) {
    var cookies = cookieContainer?.GetCookieHeader (request.RequestUri); // as per docs: An HTTP cookie header, with strings representing Cookie instances delimited by semicolons.
    if (!string.IsNullOrEmpty (cookies)) {
        var cookiePtr = NSString.CreateNative (Cookie);
        var cookiesPtr = NSString.CreateNative (cookies);
        nativeHeaders.LowlevelSetObject (cookiesPtr, cookiePtr);
        NSString.ReleaseNative (cookiePtr);
        NSString.ReleaseNative (cookiesPtr);
    }
}

Possible fix:

if (session.Configuration.HttpCookieStorage != null)
{
    var cookieHeader =
        cookieContainer?.GetCookieHeader(request
            .RequestUri); // as per docs: An HTTP cookie header, with strings representing Cookie instances delimited by semicolons.
    if (!string.IsNullOrEmpty(cookies))
    {
        var cookiePtr = NSString.CreateNative(Cookie);
        var cookiesPtr = NSString.CreateNative(cookieHeader);
        nativeHeaders.LowlevelSetObject(cookiesPtr, cookiePtr);
        NSString.ReleaseNative(cookiePtr);
        NSString.ReleaseNative(cookiesPtr);
    }
    else
    {
        var cookies = session.Configuration.HttpCookieStorage.CookiesForUrl(request.RequestUri);
        foreach (var cookie in cookies)
        {
            _configuration.HttpCookieStorage.DeleteCookie(cookie);
        }
    }
}
whitneyschmidt commented 3 years ago

cc @mandel-macaque thoughts?

mandel-macaque commented 3 years ago

We need to test. I believe what happens is that the cookier container is used, provides the cookies for the native one, native one is not invalidated and reuses the cookies. So I don't think the propose fix is correct but this is certainly an issue.

we need to link somehow the update of the managed container to the native one.