Bug/ios networkextension by batmaninpink · Pull Request #1158 · dotnet/macios

batmaninpink · 2016-11-10T14:23:16Z

This is similar to the issue I found here:

https://bugzilla.xamarin.com/show_bug.cgi?id=44874

I am trying to create a custom VPN iOS app with Xamarin, with these two commits it's possible to bring up the VPN.

dnfclas · 2016-11-10T14:23:20Z

Hi @batmaninpink, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. Real humans will now evaluate your PR.

TTYL, DNFBOT;

monojenkins · 2016-11-10T14:26:01Z

Hello! I'm the build bot for the Mono project.

I need approval from a Mono team member to build this pull request. A team member should reply with "approve" to approve a build of this pull request, "whitelist" to whitelist this and all future pull requests from this contributor, or "build" to explicitly request a build, even if one has already been done.

Contributors can ignore this message.

rolfbjarne · 2016-11-10T14:27:08Z

approve

monojenkins · 2016-11-10T15:24:03Z

Build failure

batmaninpink · 2016-11-10T17:19:51Z

ok, tests wanted the default initializer, so I brought it back

monojenkins · 2016-11-10T18:52:30Z

Build failure

spouliot · 2016-11-10T19:42:38Z

The tests can't seems to create a default NEPacketTunnelProvider instance. @VincentDondain can you check with the entitlements if the behaviour differs ? in that case we'll need to update the test (to merge the PR)

VincentDondain · 2016-11-17T13:17:42Z


 	[iOS (9,0)][Mac (10,11, onlyOn64 : true)]
 	[BaseType (typeof (NETunnelNetworkSettings))]
+	[DisableDefaultCtor]


As far as I can tell the default constructor is fine on this type + adding [DisableDefaultCtor] is a breaking change. You would need to add a non-generated constructor outside the binding files with [Obsolete] to preserve API compatibility.

Ok, I kept the default constructor, although it's useless since the resulting NETunnelNetworkSettings object can't be used to initiate the tunneling interface.

We'll mark it as obsolete (and remove it from future profiles)

VincentDondain · 2016-11-17T13:23:12Z

@spouliot NEPacketTunnelProvider *a = [[NEPacketTunnelProvider alloc] init]; return nil without these entitlements:

<key>com.apple.developer.networking.networkextension</key>
<array>
    <string>packet-tunnel-provider</string>
    <string>app-proxy-provider</string>
    <string>content-filter-provider</string>
</array>

Note: you're also supposed to call it from its "extension process" otherwise you get: NEProvider objects cannot be instantiated from non-extension processes.

Hence from an iOS App Extension that has the com.apple.networkextension.packet-tunnel extension point.

monojenkins · 2016-11-17T15:20:51Z

Build failure

monojenkins · 2016-11-17T19:45:05Z

Build failure

monojenkins · 2016-11-17T21:08:00Z

Build failure

spouliot · 2016-11-29T16:23:56Z

+
+			case "NEPacketTunnelProvider":
+				return true;
+


This is fine but in the wrong place. The bots reports the same error for XM, so it's the base class that needs to handle this (so both XI and XM ignore that .ctor).

Right, thanks. Fixed (I think).

…ructor as parent class

monojenkins · 2016-11-29T17:25:15Z

Build failure

monojenkins · 2016-11-29T18:23:08Z

Build success

batmaninpink · 2016-11-29T19:49:38Z

thanks, would it possible to get this into beta build?

spouliot · 2016-11-30T13:15:59Z

@batmaninpink I'll first add the obsolete attribute on the API mentioned before. The next scheduled beta is for C9 (in alpha now) but since it's API adjustments only I'll likely backport them to xcode8.2, which is likely to reach stable before C9 (rumours points to Apple releasing it in December).

…bsolete The returned instance cannot be used [1]. However removing it would be a breaking change so we're obsoleting the API (until a new version of the profile). [1] dotnet#1158 (comment)

…bsolete (#1267) The returned instance cannot be used [1]. However removing it would be a breaking change so we're obsoleting the API (until a new version of the profile). [1] #1158 (comment)

@VincentDondain

…h specific entitlements (#1158) * Enable default constructor on NEPacketTunnelProvider * NEPacketTunnelNetworkSettings needs the same constructor as parent class * skip NEPacketTunnelProvider in ctor init test From @VincentDondain `NEPacketTunnelProvider *a = [[NEPacketTunnelProvider alloc] init];` return `nil` without these entitlements: ``` <key>com.apple.developer.networking.networkextension</key> <array> <string>packet-tunnel-provider</string> <string>app-proxy-provider</string> <string>content-filter-provider</string> </array> ``` Note: you're also supposed to call it from its "extension process" otherwise you get: `NEProvider objects cannot be instantiated from non-extension processes.` Hence from an iOS App Extension that has the `com.apple.networkextension.packet-tunnel` extension point.

…bsolete (#1267) The returned instance cannot be used [1]. However removing it would be a breaking change so we're obsoleting the API (until a new version of the profile). [1] #1158 (comment)

batmaninpink · 2016-11-30T16:37:58Z

@spouliot thanks, this should also go into xcode8.2 since you fix the others: #936

spouliot · 2016-11-30T18:41:46Z

@batmaninpink good point, cherry-picked. thanks!

@VincentDondain

* Bump Xcode to 8.2 and update API files to match C8SR1 (#1096) * Bump Xcode to 8.2 and update API files to match C8SR1 * [tests] Tweak tests so there's no failure when running against Xcode 8.2 and the new SDK * [homekit] Update for iOS 10.2 beta 1 (#1088) - Also use smart enums for HMServiceType, HMCharacteristicType and HMAccessoryCategoryType. * [watchconnectivity] Small API update for Xcode 8.2 beta 1 (#1097) * [uikit] Small update for Xcode 8.2 beta 1 (#1098) * UICloudSharingPermissionOptions is not part in watchOS. It's an enum so it's not a big issue (worth a breaking change) -> XAMCORE_4_0 * Remove TODO on UICloudSharingController as Apple fixed our rdar 27929711 * [watchos][passkit] Small update for Xcode 8.2 beta 1 (#1099) As suspected CanAddFelicaPass is part of watchOS 3.1, closing our reported rdar 28608634 * [generator] Add more control over generated enums/fields (#836) This allows us to convert some existing manual conversion code into generated code and never miss a new constant being added [1]. The additional control comes in two forms: * allow [Field (null)]: a null NSString constant will return this enum value instead of throwing an ArgumentNullException; * a new `[DefaultEnumValue]` attribute allow marking the constant to be returned if the enum value is not known; [1] Vincent found some missing in HomeKit when adding the new ones from iOS 10. This commits also adds documentation for the existing (missing) and new attributes. * [generator] Allow identical (numeric) values in smart enums. Fixes #46285 (#1102) Considering the following binding code: public enum HMAccessoryCategoryType { [Field ("HMAccessoryCategoryTypeGarageDoorOpener")] DoorOpener, GarageDoorOpener = DoorOpener, } We must 1. Ensure that `HMAccessoryCategoryType.DoorOpener.GetConstant () == HMAccessoryCategoryType.GarageDoorOpener.GetConstant ()`; This is done by using the numeric value of the enum member (instead of the name) 2. Ensure that `HMAccessoryCategoryTypeExtensions.GetValue ("HMAccessoryCategoryTypeGarageDoorOpener")` always return the same enum value, i.e. it can **not** change between XI versions (e.g. due to reflection ordering) as it could break comparison code; This is done by only adding a map to the member that has a [Field] and means that: 2.1. the _favorite_ enum member should be the one with the [Field]; and 2.2. a [Field] value can only be used once per enum (or else we report an BI1046 error). This also solve the duplicate code generation for the constant loading code; Reference: * https://bugzilla.xamarin.com/show_bug.cgi?id=46285 * [watchos][watchkit] Small updates for Xcode 8.2 beta 1 (#1103) Normally the removal of `HandleAction` would be a breaking change (and require a stub). However Xamarin.WatchOS.dll is not final before C9 so this fix will be backported to `master` and `cycle9` branches. * [generator] Copy [Obsolete] attributes on smart enums. Fixes #46292 (#1104) Covers attributes on the type itself and on its members. Reference: * https://bugzilla.xamarin.com/show_bug.cgi?id=46292 * [tvos][storekit] Small update for Xcode 8.2 beta 1 (#1113) Effectively there's no API change, but a new (for tvOS) header was added even if it does not include stuff added to tvOS. This requires a few updates for xtro to gives correct results. Also added comments in bindings since this makes things clearer. * [replaykit] Add new FinishBroadcast method to RPBroadcastSampleHandler from Xcode 8.2 beta 1 (#1101) * [Photos] Update Photos to Xcode 8.2 Beta 1 (#1106) * [security] Add two new constants to SSLSessionConfig from Xcode 8.2 beta 1 (#1100) * [videosubscriberaccount] Small update for Xcode 8.2 beta 1 (#1115) * [messages] Update to iOS 10.2 beta 1 (#1112) * [modelio] Update for iOS 10.2 beta 1 (#1126) * [spritekit] Update for iOS 10.2 beta 1 (#1111) * [mtouch] Add workarounds for file descriptor leak in System.Diagnostics.Process. (#1137) This is a backport of these commits from master: 4d06d05, 1fdd17e and 39ec76e. It fixes this mtouch test: 1) Test Error : MTouchTests.MTouch.ScriptedTests [...] error MT0000: Unexpected error - Please file a bug report at http://bugzilla.xamarin.com System.IO.IOException: Too many open files at System.Diagnostics.Process.CreatePipe (System.IntPtr& read, System.IntPtr& write, System.Boolean writeDirection) [0x00094] in /private/tmp/source-mono-4.6.0-c8sr0/bockbuild-mono-4.6.0-branch-c8sr0/profiles/mono-mac-xamarin/build-root/mono-x86/mcs/class/System/System.Diagnostics/Process.cs:659 at System.Diagnostics.Process.StartWithCreateProcess (System.Diagnostics.ProcessStartInfo startInfo) [0x0012c] in /private/tmp/source-mono-4.6.0-c8sr0/bockbuild-mono-4.6.0-branch-c8sr0/profiles/mono-mac-xamarin/build-root/mono-x86/mcs/class/System/System.Diagnostics/Process.cs:716 * [homekit] Update for tvOS 10.1 beta 1 (#1139) - Also add missing HMCharacteristicTypeSecuritySystemAlarmType. * [mediaplayer] Enable MediaPlayer on macOS (#1138) - Enable some MediaPlayer-related UIKit APIs. - tools/common/Frameworks.cs fix to handle build version digit. - Lots of [NoMac] and [Mac (10,12,2)]. * [homekit] Remove bugzilla comments (#1152) Since bug #46292: "[generator] Obsolete attribute doesn't make it to generated file for smart enums" and #46285: "[generator] Smart enum does not handle 2 equal values" are both fixed, the comments can be removed. Also we don't need to have the [Field] attribute on 2 enum values anymore. * [Mac 10.12.2] CloudKit nullability info (#1153) * [mediaplayer] Fixes due to xtro tests (#1160) - MPMusicPlayerController_MPPlaybackControl category isn't available on tvOS. - Add missing `MPMediaPlaylist` selectors. * Bump to Xcode 8.2 beta 2. (#1193) * [Intents] Update to Xcode 8.2 Beta 1 (#1192) * [CloudKit] Update bindings for Xcode 8.2 (#1197) * [HomeKit] Update to Xcode 8.2 Beta 2 * [Photos] Update Photos to Xcode 8.2 Beta 2 * [Homekit] Remove Apple's Suplhur typo from ApiTypoTest * [Intents] Update to Xcode 8.2 Beta 2 * [passkit] Update for iOS 10.2 beta 2 * [AVFoundation] Update bindings for Xcode 8.2 (#1196) * [AVFoundation] Update bindings for Xcode 8.2 * Fix typo. * Move property to the correct class. * [homekit] Add "[NoWatch]" to "[WatchOS (3,1,1)]" APIs (#1219) As xtro pointed out (see https://gist.github.com/spouliot/b634e5d1a5590a629d6c6c00e4eadbef) APIs that were marked as available for watchOS 3.1.1 weren't in fact available. Note: it's not impossible that a future version of Xcode actually adds those. We'll then be able to just revert this commit. * [homekit] Add back properties removed when dropping manual enum code (#1226) references: * API diff @ https://jenkins.mono-project.com/job/xamarin-macios-pr-builder/2071/API_diff/ * commit 0504586 * [opentk] Bump to remove deprecated glGetQueryObjectivEXT symbol (#1239) reference: !unknown-pinvoke! glGetQueryObjectivEXT bound * [cloudkit][watchos] Re-introduce [DesignatedInitializer] on CKFetchWebAuthTokenOperation (#1240) Header files were not updated (in Xcode 8.2) for watchOS 3.1 so some changes should be delayed (in case the fork is permanent) reference: !missing-designated-initializer! CKFetchWebAuthTokenOperation::initWithAPIToken: is missing an [DesignatedInitializer] attribute * [tests][xtro] Fix category name mapping (e.g. casing) (#1244) reference: !missing-selector! NSMutableURLRequest::bindToHotspotHelperCommand: not bound * [tests][xtro] Update data files (#1245) * [AVKit] Add missing enum for Xcode 8.2 beta 1. (#1246) * [AVKit] Add missing enum for Xcode 8.2 beta 1. * As per review, move away from the enum implementation, add a static class with the valid strings. * [intents] Add obsoleted default ctor for INRideDriver and INRideStatus (#1262) Xcode 8.2 (beta 1) changed the default `init` to throw the following `NSInvalidArgumentException Reason: *** -[__NSPlaceholderDictionary initWithObjects:forKeys:count:]: attempt to insert nil object from objects[1]` So we introduce compatibility stubs to avoid the native exception, warn the developers and maintain binary compatibility * [apidiff] Update xml definitions to match C8SR1 (stable) (#1263) * [networkextension] Fix some API that works only from an extension with specific entitlements (#1158) * Enable default constructor on NEPacketTunnelProvider * NEPacketTunnelNetworkSettings needs the same constructor as parent class * skip NEPacketTunnelProvider in ctor init test From @VincentDondain `NEPacketTunnelProvider *a = [[NEPacketTunnelProvider alloc] init];` return `nil` without these entitlements: ``` <key>com.apple.developer.networking.networkextension</key> <array> <string>packet-tunnel-provider</string> <string>app-proxy-provider</string> <string>content-filter-provider</string> </array> ``` Note: you're also supposed to call it from its "extension process" otherwise you get: `NEProvider objects cannot be instantiated from non-extension processes.` Hence from an iOS App Extension that has the `com.apple.networkextension.packet-tunnel` extension point. * [networkextension] Mark NEPacketTunnelNetworkSettings default .ctor obsolete (#1267) The returned instance cannot be used [1]. However removing it would be a breaking change so we're obsoleting the API (until a new version of the profile). [1] #1158 (comment) * Merge pull request #936 from VincentDondain/master-fix-bug-44874 [networkextension] Enable default contructor on NETunnelProviderManager * Merge xcode82 into cycle8-xi for C8SR2

dnfclas added the cla-not-required label Nov 10, 2016

batmaninpink force-pushed the bug/ios-networkextension branch from 7257346 to d4bec5c Compare November 10, 2016 17:18

spouliot assigned VincentDondain Nov 10, 2016

batmaninpink force-pushed the bug/ios-networkextension branch from d4bec5c to 38593fc Compare November 15, 2016 21:05

VincentDondain suggested changes Nov 17, 2016

View reviewed changes

batmaninpink force-pushed the bug/ios-networkextension branch from 38593fc to 6e2ab66 Compare November 17, 2016 13:45

batmaninpink force-pushed the bug/ios-networkextension branch 2 times, most recently from 60d47ec to 3a790fb Compare November 17, 2016 19:42

spouliot suggested changes Nov 29, 2016

View reviewed changes

Per Allansson added 2 commits November 29, 2016 18:01

[networkextension] Enable default constructor on NEPacketTunnelProvider

b7ba3a5

[networkextension] NEPacketTunnelNetworkSettings needs the same const…

dcc9512

…ructor as parent class

batmaninpink force-pushed the bug/ios-networkextension branch from 3a790fb to b4ca6c1 Compare November 29, 2016 17:03

[networkextension] skip NEPacketTunnelProvider in ctor init test

6cdad61

batmaninpink force-pushed the bug/ios-networkextension branch from b4ca6c1 to 6cdad61 Compare November 29, 2016 17:22

spouliot approved these changes Nov 29, 2016

View reviewed changes

spouliot merged commit 26e640c into dotnet:master Nov 29, 2016

batmaninpink deleted the bug/ios-networkextension branch November 29, 2016 19:50

spouliot mentioned this pull request Nov 30, 2016

[networkextension] Mark NEPacketTunnelNetworkSettings default .ctor obsolete #1267

Merged

dalexsoto added the community Community contribution ❤ label Feb 6, 2019

Uh oh!

Conversation

batmaninpink commented Nov 10, 2016

Uh oh!

dnfclas commented Nov 10, 2016

Uh oh!

monojenkins commented Nov 10, 2016

Uh oh!

rolfbjarne commented Nov 10, 2016

Uh oh!

monojenkins commented Nov 10, 2016

Uh oh!

batmaninpink commented Nov 10, 2016

Uh oh!

monojenkins commented Nov 10, 2016

Uh oh!

spouliot commented Nov 10, 2016

Uh oh!

VincentDondain Nov 17, 2016

Choose a reason for hiding this comment

Uh oh!

batmaninpink Nov 17, 2016

Choose a reason for hiding this comment

Uh oh!

spouliot Nov 29, 2016

Choose a reason for hiding this comment

Uh oh!

VincentDondain commented Nov 17, 2016

Uh oh!

monojenkins commented Nov 17, 2016

Uh oh!

monojenkins commented Nov 17, 2016

Uh oh!

monojenkins commented Nov 17, 2016

Uh oh!

spouliot Nov 29, 2016

Choose a reason for hiding this comment

Uh oh!

batmaninpink Nov 29, 2016

Choose a reason for hiding this comment

Uh oh!

monojenkins commented Nov 29, 2016

Uh oh!

monojenkins commented Nov 29, 2016

Uh oh!

batmaninpink commented Nov 29, 2016

Uh oh!

spouliot commented Nov 30, 2016

Uh oh!

batmaninpink commented Nov 30, 2016

Uh oh!

spouliot commented Nov 30, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants