From d2c0b9c7963369ccb89ca8274c55d4ca43d377d4 Mon Sep 17 00:00:00 2001 From: Martin Baulig Date: Thu, 31 Oct 2019 12:31:51 -0400 Subject: [PATCH 1/4] Bring HttpClient from CoreFX. This bumps Mono to use https://github.com/mono/mono/pull/17645 (which is the 2019-10 backport of https://github.com/mono/mono/pull/17628). The big user-visible change is in regards to certificate validation, everything below are just some minor adjustments to tests. ## SocketsHttpHandler CoreFX uses a completely new `HttpClientHandler` implementation called `SocketsHttpHandler`, which you can find at https://github.com/dotnet/corefx/tree/release/3.0/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler. Since this is not based on the web stack anymore, it does not use any of the related APIs such as `ServicePointManager` or `WebException`. ## Certificate Validation Changes There is a new API called `HttpClientHandler.ServerCertificateCustomValidationCallback`. - https://docs.microsoft.com/en-us/dotnet/api/system.net.http.httpclienthandler.servercertificatecustomvalidationcallback?view=netframework-4.8 - https://github.com/dotnet/corefx/blob/c1778515a3bee34cc09c757b5563d0af0c8b1e99/src/System.Net.Http/src/System/Net/Http/HttpClientHandler.Unix.cs#L154 - https://github.com/dotnet/corefx/blob/c1778515a3bee34cc09c757b5563d0af0c8b1e99/src/System.Net.Http/src/System/Net/Http/HttpClientHandler.Windows.cs#L383 The `ServicePointManager.ServerCertificateValidationCallback` is no longer invoked and on certificate validation failure, `AuthenticationException` (from `System.Security.Authentication`) is thrown instead of `WebException`. At the moment, the `NSUrlSessionHandler` still uses it's own validation callback and also still throws `WebException` on failure; we should probably look into making this consistent with the other handlers. ## Minor adjustments related to internal Mono APIs * `HttpContent.SerializeToStreamAsync()` is now `protected` (changed from `protected internal`). - src/Foundation/NSUrlSessionHandler.cs: changed overload accordingly. - src/System.Net.Http/CFContentStream.cs: likewise. * `HttpHeaders.GetKnownHeaderKind()` is an internal Mono API. There is a new internal API called `System.Net.Http.PlatformHelper.IsContentHeader(key)` which exists in both the old as well as the new implementation. The correct way of doing it with the CoreFX handler is `HeaderDescriptor.TryGet (key, out var descriptor) && descriptor.HeaderType == HttpHeaderType.Content` ## Minor adjustments to tests. * `HttpClientHandler.MaxRequestContentBufferSize` is now longer supported, you can set it to any non-negative value, the getter will always return 0. See https://github.com/dotnet/corefx/blob/c1778515a3bee34cc09c757b5563d0af0c8b1e99/src/System.Net.Http/src/System/Net/Http/HttpClientHandler.Core.cs#L18. - tests/linker/ios/link sdk/HttpClientHandlerTest.cs: removed assertion from test. * `HttpMessageInvoker.handler` is a `protected private` field - in the CoreFX handler, it is called `_handler` and `private`. This is accessed via reflection by some of the tests, which are now using the new name. - tests/mmptest/src/MMPTest.cs: here - tests/mtouch/MTouch.cs: here * tests/monotouch-test/System.Net.Http/MessageHandlers.cs: Adjust `RejectSslCertificatesServicePointManager` to reflect the certificate validation changes described above. - FIXME: There was an `Assert.Ignore()` related to `NSUrlSessionHandler` and macOS 10.10; I removed that to reenable the test because the description linked to an old issue in the private repo that was referenced by several "Merged" PR's, so it looked to me that this might have already been fixed - and I also didn't see why it would fail there. --- mk/mono.mk | 5 ++- src/Foundation/NSUrlSessionHandler.cs | 4 --- src/System.Net.Http/CFContentStream.cs | 2 +- src/System.Net.Http/CFNetworkHandler.cs | 2 +- .../ios/link sdk/HttpClientHandlerTest.cs | 1 - tests/mmptest/src/MMPTest.cs | 4 +-- .../System.Net.Http/MessageHandlers.cs | 34 ++++++++++++------- tests/mtouch/MTouch.cs | 4 +-- 8 files changed, 30 insertions(+), 26 deletions(-) diff --git a/mk/mono.mk b/mk/mono.mk index 7eb96b377731..7885496c7f63 100644 --- a/mk/mono.mk +++ b/mk/mono.mk @@ -1,6 +1,5 @@ NEEDED_MONO_VERSION := 4847b8e7a721a488c228080a932124337966e5a8 -NEEDED_MONO_BRANCH := 2019-10 - +NEEDED_MONO_BRANCH := backport-pr-17628-to-2019-10 MONO_DIRECTORY := mono -MONO_MODULE := https://github.com/mono/mono +MONO_MODULE := https://github.com/monojenkins/mono diff --git a/src/Foundation/NSUrlSessionHandler.cs b/src/Foundation/NSUrlSessionHandler.cs index f98b6ef62b96..059b1ead0f12 100644 --- a/src/Foundation/NSUrlSessionHandler.cs +++ b/src/Foundation/NSUrlSessionHandler.cs @@ -831,11 +831,7 @@ protected override void Dispose (bool disposing) base.Dispose (disposing); } -#if __WATCHOS__ protected override Task SerializeToStreamAsync (Stream stream, TransportContext context) -#else - protected internal override Task SerializeToStreamAsync (Stream stream, TransportContext context) -#endif { if (contentCopied) { if (!content.CanSeek) { diff --git a/src/System.Net.Http/CFContentStream.cs b/src/System.Net.Http/CFContentStream.cs index 42f0142ed564..bdd3518ad57b 100644 --- a/src/System.Net.Http/CFContentStream.cs +++ b/src/System.Net.Http/CFContentStream.cs @@ -125,7 +125,7 @@ public void Close () data_event.Set (); } - protected internal override async Task SerializeToStreamAsync (Stream stream, TransportContext context) + protected override async Task SerializeToStreamAsync (Stream stream, TransportContext context) { while (data_event.WaitOne ()) { data_mutex.WaitOne (); diff --git a/src/System.Net.Http/CFNetworkHandler.cs b/src/System.Net.Http/CFNetworkHandler.cs index 7af25c9628fc..edca57709388 100644 --- a/src/System.Net.Http/CFNetworkHandler.cs +++ b/src/System.Net.Http/CFNetworkHandler.cs @@ -358,7 +358,7 @@ void HandleHasBytesAvailableEvent (object sender, CFStream.StreamEventArgs e) var key = entry.Key.ToString (); var value = entry.Value == null ? string.Empty : entry.Value.ToString (); HttpHeaders item_headers; - if (HttpHeaders.GetKnownHeaderKind (key) == Headers.HttpHeaderKind.Content) { + if (HeaderDescriptor.TryGet (key, out var descriptor) && descriptor.HeaderType == HttpHeaderType.Content) { item_headers = response_msg.Content.Headers; } else { item_headers = response_msg.Headers; diff --git a/tests/linker/ios/link sdk/HttpClientHandlerTest.cs b/tests/linker/ios/link sdk/HttpClientHandlerTest.cs index e9f2e68461d0..6a347f65ed4e 100644 --- a/tests/linker/ios/link sdk/HttpClientHandlerTest.cs +++ b/tests/linker/ios/link sdk/HttpClientHandlerTest.cs @@ -30,7 +30,6 @@ public void HttpClient () Assert.That (handler.AutomaticDecompression, Is.EqualTo (DecompressionMethods.None), "AutomaticDecompression"); Assert.That (handler.ClientCertificateOptions, Is.EqualTo (ClientCertificateOption.Manual), "ClientCertificateOptions"); Assert.That (handler.MaxAutomaticRedirections, Is.EqualTo (50), "MaxAutomaticRedirections"); - Assert.That (handler.MaxRequestContentBufferSize, Is.EqualTo (Int32.MaxValue), "MaxRequestContentBufferSize"); Assert.Null (handler.Proxy, "Proxy"); Assert.True (handler.SupportsAutomaticDecompression, "SupportsAutomaticDecompression"); Assert.True (handler.SupportsProxy, "SupportsProxy"); diff --git a/tests/mmptest/src/MMPTest.cs b/tests/mmptest/src/MMPTest.cs index c1e3d03c6389..ac7907feb299 100644 --- a/tests/mmptest/src/MMPTest.cs +++ b/tests/mmptest/src/MMPTest.cs @@ -474,9 +474,9 @@ public void HttpClientHandler (string mmpHandler, string expectedHandler) References = " ", TestCode = $@" var client = new System.Net.Http.HttpClient (); - var field = client.GetType ().BaseType.GetField (""handler"", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); + var field = client.GetType ().BaseType.GetField (""_handler"", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); if (field == null) - throw new System.Exception (""Could not find the field 'handler' in HttpClient's base type (which should be 'HttpMessageInvoker').""); + throw new System.Exception (""Could not find the field '_handler' in HttpClient's base type (which should be 'HttpMessageInvoker').""); var fieldValue = field.GetValue (client); if (fieldValue == null) throw new System.Exception (""Unexpected null value found in 'HttpMessageInvoker.handler' field.""); diff --git a/tests/monotouch-test/System.Net.Http/MessageHandlers.cs b/tests/monotouch-test/System.Net.Http/MessageHandlers.cs index 206e3a4ee36f..63c33eba6cb3 100644 --- a/tests/monotouch-test/System.Net.Http/MessageHandlers.cs +++ b/tests/monotouch-test/System.Net.Http/MessageHandlers.cs @@ -14,6 +14,7 @@ using NUnit.Framework; using System.Net.Http.Headers; +using System.Security.Authentication; using System.Text; using Foundation; #if MONOMAC @@ -169,29 +170,35 @@ public void RejectSslCertificatesServicePointManager (Type handlerType) TestRuntime.AssertSystemVersion (PlatformName.MacOSX, 10, 9, throwIfOtherPlatform: false); TestRuntime.AssertSystemVersion (PlatformName.iOS, 7, 0, throwIfOtherPlatform: false); -#if __MACOS__ - if (handlerType == typeof (NSUrlSessionHandler) && TestRuntime.CheckSystemVersion (PlatformName.MacOSX, 10, 10, 0) && !TestRuntime.CheckSystemVersion (PlatformName.MacOSX, 10, 11, 0)) - Assert.Ignore ("Fails on macOS 10.10: https://github.com/xamarin/maccore/issues/1645"); -#endif - - bool servicePointManagerCbWasExcuted = false; + bool validationCbWasExecuted = false; + bool customValidationCbWasExecuted = false; + bool invalidServicePointManagerCbWasExcuted = false; bool done = false; Exception ex = null; + Type expectedExceptionType = null; HttpResponseMessage result = null; var handler = GetHandler (handlerType); - if (handler is NSUrlSessionHandler ns) { - ns.TrustOverride += (a,b) => { - servicePointManagerCbWasExcuted = true; + if (handler is HttpClientHandler ch) { + expectedExceptionType = typeof (AuthenticationException); + ch.ServerCertificateCustomValidationCallback = (sender, certificate, chain, errors) => { + validationCbWasExecuted = true; // return false, since we want to test that the exception is raised return false; }; - } else { ServicePointManager.ServerCertificateValidationCallback = (sender, certificate, chain, errors) => { - servicePointManagerCbWasExcuted = true; + invalidServicePointManagerCbWasExcuted = true; + return false; + }; + } else if (handler is NSUrlSessionHandler ns) { + expectedExceptionType = typeof (WebException); + ns.TrustOverride += (a,b) => { + validationCbWasExecuted = true; // return false, since we want to test that the exception is raised return false; }; + } else { + Assert.Fail ($"Invalid HttpMessageHandler: '{handler.GetType ()}'."); } TestRuntime.RunAsync (DateTime.Now.AddSeconds (30), async () => @@ -213,11 +220,14 @@ public void RejectSslCertificatesServicePointManager (Type handlerType) if (!done) { // timeouts happen in the bots due to dns issues, connection issues etc.. we do not want to fail Assert.Inconclusive ("Request timedout."); } else { + // the ServicePointManager.ServerCertificateValidationCallback will never be executed. + Assert.False(invalidServicePointManagerCbWasExcuted); + Assert.True(validationCbWasExecuted); // assert the exception type Assert.IsNotNull (ex, (result == null)? "Expected exception is missing and got no result" : $"Expected exception but got {result.Content.ReadAsStringAsync ().Result}"); Assert.IsInstanceOfType (typeof (HttpRequestException), ex); Assert.IsNotNull (ex.InnerException); - Assert.IsInstanceOfType (typeof (WebException), ex.InnerException); + Assert.IsInstanceOfType (expectedExceptionType, ex.InnerException); } } diff --git a/tests/mtouch/MTouch.cs b/tests/mtouch/MTouch.cs index 1a740988c5f3..5af0880017ba 100644 --- a/tests/mtouch/MTouch.cs +++ b/tests/mtouch/MTouch.cs @@ -3940,9 +3940,9 @@ public class HandlerTest public void Test () {{ var client = new System.Net.Http.HttpClient (); - var field = client.GetType ().BaseType.GetField (""handler"", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); + var field = client.GetType ().BaseType.GetField (""_handler"", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); if (field == null) - throw new System.Exception (""Could not find the field 'handler' in HttpClient's base type (which should be 'HttpMessageInvoker').""); + throw new System.Exception (""Could not find the field '_handler' in HttpClient's base type (which should be 'HttpMessageInvoker').""); var fieldValue = field.GetValue (client); if (fieldValue == null) throw new System.Exception (""Unexpected null value found in 'HttpMessageInvoker.handler' field.""); From a2c00dd7fae09d8333320360fa3ad9b4f9a7d475 Mon Sep 17 00:00:00 2001 From: Martin Baulig Date: Fri, 1 Nov 2019 15:03:51 -0400 Subject: [PATCH 2/4] Bump MIN_MONO_VERSION to 6.8.0.41 and point MIN_MONO_URL to the PR. --- Make.config | 4 ++-- mk/mono.mk | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Make.config b/Make.config index f724433b8a81..4daf1319a3a2 100644 --- a/Make.config +++ b/Make.config @@ -70,9 +70,9 @@ MONO_HASH := $(NEEDED_MONO_VERSION) # Minimum Mono version for building XI/XM -MIN_MONO_VERSION=6.8.0.0 +MIN_MONO_VERSION=6.8.0.41 MAX_MONO_VERSION=6.8.99 -MIN_MONO_URL=https://xamjenkinsartifact.azureedge.net/build-package-osx-mono/2019-10/40/4847b8e7a721a488c228080a932124337966e5a8/MonoFramework-MDK-6.8.0.36.macos10.xamarin.universal.pkg +MIN_MONO_URL=https://xamjenkinsartifact.azureedge.net/build-package-osx-mono-pullrequest/pr/452/272807d8feba935dcef4ceac6c937ad556354b4d/MonoFramework-MDK-pr@272807d8feb-dirty-6.8.0.41.macos10.xamarin.universal.pkg # Minimum Mono version for Xamarin.Mac apps using the system mono MIN_XM_MONO_VERSION=6.4.0.94 diff --git a/mk/mono.mk b/mk/mono.mk index 7885496c7f63..98f96fa5d60e 100644 --- a/mk/mono.mk +++ b/mk/mono.mk @@ -1,4 +1,4 @@ -NEEDED_MONO_VERSION := 4847b8e7a721a488c228080a932124337966e5a8 +NEEDED_MONO_VERSION := 5dcbd5af1b427391b086358d8a501fabcc464040 NEEDED_MONO_BRANCH := backport-pr-17628-to-2019-10 MONO_DIRECTORY := mono From 80512c4351caac9b68db4da50264f32477ca7154 Mon Sep 17 00:00:00 2001 From: Martin Baulig Date: Fri, 1 Nov 2019 15:22:13 -0400 Subject: [PATCH 3/4] Add ENABLE_IOS=1 and ENABLE_MAC=1. --- builds/Makefile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/builds/Makefile b/builds/Makefile index 95e4d1532dd6..a34b888fbb0a 100644 --- a/builds/Makefile +++ b/builds/Makefile @@ -109,6 +109,9 @@ $(SDK_CONFIG): echo "DISABLE_ANDROID=1" > $@ echo "DISABLE_DESKTOP=1" >> $@ echo "DISABLE_WASM=1" >> $@ + echo "ENABLE_IOS=1" >> $@ + echo "ENABLE_MAC=1" >> $@ + ifdef DISABLE_BUILDS_MAKEFILE_DEP BUILDS_MAKEFILE_DEP = From 065afaa5d243468337e3bfdd05f0503f6d0dda4d Mon Sep 17 00:00:00 2001 From: Steve Pfister Date: Tue, 5 Nov 2019 10:07:05 -0500 Subject: [PATCH 4/4] Added switch to disable packaged mono build --- jenkins/build.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jenkins/build.sh b/jenkins/build.sh index 64ee4cb302e7..c134d617ff03 100755 --- a/jenkins/build.sh +++ b/jenkins/build.sh @@ -124,9 +124,9 @@ make reset make git-clean-all make print-versions -echo "Configuring the build with: $CONFIGURE_FLAGS" +echo "Configuring Martin's disabled packaged mono build with: $CONFIGURE_FLAGS" # shellcheck disable=SC2086 -./configure $CONFIGURE_FLAGS +./configure $CONFIGURE_FLAGS --disable-packaged-mono # If we're building mono from source, we might not have it cloned yet make reset