From cbc15fad94eff5e62f9a3d7c9a1dc2a53fd3b460 Mon Sep 17 00:00:00 2001 From: Manuel de la Pena Date: Thu, 23 Mar 2017 17:07:03 +0100 Subject: [PATCH 1/4] [Foundation] Ensure that if a request is canceled we cancel the NSUrlSessionTask If the request is canceled, the inflight data is cleaned up. That allows us to know that the NSUrlSessionTask should be canceled, which will execute the DidCompleteWithError that will take care of the cleanup of resources and will ensure that everything is back to a known situation. --- src/Foundation/NSUrlSessionHandler.cs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/Foundation/NSUrlSessionHandler.cs b/src/Foundation/NSUrlSessionHandler.cs index 0b14a909d865..484f1d25d007 100644 --- a/src/Foundation/NSUrlSessionHandler.cs +++ b/src/Foundation/NSUrlSessionHandler.cs @@ -215,6 +215,8 @@ InflightData GetInflightData (NSUrlSessionTask task) if (sessionHandler.inflightRequests.TryGetValue (task, out inflight)) return inflight; + // if we did not manage to get the inflight data, we either got an error or have been canceled, lets cancel the task, that will execute DidCompleteWithError + task.Cancel (); return null; } @@ -222,6 +224,9 @@ public override void DidReceiveResponse (NSUrlSession session, NSUrlSessionDataT { var inflight = GetInflightData (dataTask); + if (inflight == null) + return; + try { // ensure that we did not cancel the request, if we did, do cancel the task if (inflight.CancellationToken.IsCancellationRequested) { @@ -279,6 +284,9 @@ public override void DidReceiveData (NSUrlSession session, NSUrlSessionDataTask { var inflight = GetInflightData (dataTask); + if (inflight == null) + return; + inflight.Stream.Add (data); SetResponse (inflight); } @@ -338,6 +346,11 @@ public override void WillPerformHttpRedirection (NSUrlSession session, NSUrlSess public override void DidReceiveChallenge (NSUrlSession session, NSUrlSessionTask task, NSUrlAuthenticationChallenge challenge, Action completionHandler) { + var inflight = GetInflightData (task); + + if (inflight == null) + return; + // case for the basic auth failing up front. As per apple documentation: // The URL Loading System is designed to handle various aspects of the HTTP protocol for you. As a result, you should not modify the following headers using // the addValue(_:forHTTPHeaderField:) or setValue(_:forHTTPHeaderField:) methods: @@ -352,7 +365,7 @@ public override void DidReceiveChallenge (NSUrlSession session, NSUrlSessionTask // header, it means that we do not have the correct credentials, in any other case just do what it is expected. if (challenge.PreviousFailureCount == 0) { - var authHeader = GetInflightData (task)?.Request?.Headers?.Authorization; + var authHeader = inflight?.Request?.Headers?.Authorization; if (!(string.IsNullOrEmpty (authHeader?.Scheme) && string.IsNullOrEmpty (authHeader?.Parameter))) { completionHandler (NSUrlSessionAuthChallengeDisposition.RejectProtectionSpace, null); return; @@ -363,7 +376,7 @@ public override void DidReceiveChallenge (NSUrlSession session, NSUrlSessionTask if (sessionHandler.Credentials != null) { var credentialsToUse = sessionHandler.Credentials as NetworkCredential; if (credentialsToUse == null) { - var uri = GetInflightData (task).Request.RequestUri; + var uri = inflight.Request.RequestUri; credentialsToUse = sessionHandler.Credentials.GetCredential (uri, "NTLM"); } var credential = new NSUrlCredential (credentialsToUse.UserName, credentialsToUse.Password, NSUrlCredentialPersistence.ForSession); From e8448366d6994900181fa314a282b5b04d462a3d Mon Sep 17 00:00:00 2001 From: Manuel de la Pena Date: Thu, 23 Mar 2017 17:36:29 +0100 Subject: [PATCH 2/4] Remove not needed ? operator. --- src/Foundation/NSUrlSessionHandler.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Foundation/NSUrlSessionHandler.cs b/src/Foundation/NSUrlSessionHandler.cs index 484f1d25d007..c0238aed4f60 100644 --- a/src/Foundation/NSUrlSessionHandler.cs +++ b/src/Foundation/NSUrlSessionHandler.cs @@ -365,7 +365,7 @@ public override void DidReceiveChallenge (NSUrlSession session, NSUrlSessionTask // header, it means that we do not have the correct credentials, in any other case just do what it is expected. if (challenge.PreviousFailureCount == 0) { - var authHeader = inflight?.Request?.Headers?.Authorization; + var authHeader = inflight.Request?.Headers?.Authorization; if (!(string.IsNullOrEmpty (authHeader?.Scheme) && string.IsNullOrEmpty (authHeader?.Parameter))) { completionHandler (NSUrlSessionAuthChallengeDisposition.RejectProtectionSpace, null); return; From 2ea761e242157b39737ca2d1cc5667ab4d0e3041 Mon Sep 17 00:00:00 2001 From: Manuel de la Pena Date: Thu, 23 Mar 2017 18:05:51 +0100 Subject: [PATCH 3/4] The task could be null, be ready for that. --- src/Foundation/NSUrlSessionHandler.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Foundation/NSUrlSessionHandler.cs b/src/Foundation/NSUrlSessionHandler.cs index c0238aed4f60..6e8ec122ca7f 100644 --- a/src/Foundation/NSUrlSessionHandler.cs +++ b/src/Foundation/NSUrlSessionHandler.cs @@ -216,7 +216,7 @@ InflightData GetInflightData (NSUrlSessionTask task) return inflight; // if we did not manage to get the inflight data, we either got an error or have been canceled, lets cancel the task, that will execute DidCompleteWithError - task.Cancel (); + task?.Cancel (); return null; } From 65937f2554a39a9b6ff9a62bac50a29bb01b3f1a Mon Sep 17 00:00:00 2001 From: Manuel de la Pena Date: Thu, 23 Mar 2017 21:22:21 +0100 Subject: [PATCH 4/4] Do check for managed cancelations in the GetInflightData so that it is check in all handlers. --- src/Foundation/NSUrlSessionHandler.cs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/Foundation/NSUrlSessionHandler.cs b/src/Foundation/NSUrlSessionHandler.cs index 6e8ec122ca7f..af34d97eb033 100644 --- a/src/Foundation/NSUrlSessionHandler.cs +++ b/src/Foundation/NSUrlSessionHandler.cs @@ -212,8 +212,12 @@ InflightData GetInflightData (NSUrlSessionTask task) var inflight = default (InflightData); lock (sessionHandler.inflightRequestsLock) - if (sessionHandler.inflightRequests.TryGetValue (task, out inflight)) + if (sessionHandler.inflightRequests.TryGetValue (task, out inflight)) { + // ensure that we did not cancel the request, if we did, do cancel the task + if (inflight.CancellationToken.IsCancellationRequested) + task?.Cancel (); return inflight; + } // if we did not manage to get the inflight data, we either got an error or have been canceled, lets cancel the task, that will execute DidCompleteWithError task?.Cancel (); @@ -228,11 +232,6 @@ public override void DidReceiveResponse (NSUrlSession session, NSUrlSessionDataT return; try { - // ensure that we did not cancel the request, if we did, do cancel the task - if (inflight.CancellationToken.IsCancellationRequested) { - dataTask.Cancel (); - } - var urlResponse = (NSHttpUrlResponse)response; var status = (int)urlResponse.StatusCode;