Skip to content

Commit

Permalink
[Generator] Ensure that selectors fields do not have overlapping names.
Browse files Browse the repository at this point in the history
Fixes xamarin#18645

The way the generator names private fields for selectors results on not
compilable code when there are two selectoors A and B such that selector
B == AHandle.

That is, if selector A is "user", selector B will be
"userHandle". This happens because Handle is quite a common postfix in
security when working with user accounts (userHandler and others) and
we need to ensure that the Handle postfix added by the generator is unique.

As stated in the bug we could try to fix this by keeping track of the variables in the context
in wich the generator is creating code. The problem with such approach
is that it will make very hard to predict the name of the fields making
the manual code harder to write. The PR contains examples of such manual
code.

To fix the situation we have moved from using Handle to XHandle as a
postfix in the field names whihc reduces the chances of finding a
similar corner case in the future.

A test has been added to show the case in which we found the bug.

Fixes xamarin#18645
  • Loading branch information
mandel-macaque authored and vs-mobiletools-engineering-service2 committed Aug 8, 2023
1 parent 26b46ed commit 70f6d30
Show file tree
Hide file tree
Showing 9 changed files with 62 additions and 15 deletions.
8 changes: 4 additions & 4 deletions src/AppKit/Compat.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ public virtual void SetTextBlocks (NSTextBlock [] array)
throw new ArgumentNullException (nameof (array));
var nsa_array = NSArray.FromNSObjects (array);
if (IsDirectBinding) {
global::ObjCRuntime.Messaging.void_objc_msgSend_IntPtr (this.Handle, selSetTextBlocks_Handle, nsa_array.Handle);
global::ObjCRuntime.Messaging.void_objc_msgSend_IntPtr (this.Handle, selSetTextBlocks_XHandle, nsa_array.Handle);
} else {
global::ObjCRuntime.Messaging.void_objc_msgSendSuper_IntPtr (this.SuperHandle, selSetTextBlocks_Handle, nsa_array.Handle);
global::ObjCRuntime.Messaging.void_objc_msgSendSuper_IntPtr (this.SuperHandle, selSetTextBlocks_XHandle, nsa_array.Handle);
}
nsa_array.Dispose ();
}
Expand All @@ -35,9 +35,9 @@ public virtual void SetTextLists (NSTextList [] array)
throw new ArgumentNullException (nameof (array));
var nsa_array = NSArray.FromNSObjects (array);
if (IsDirectBinding) {
global::ObjCRuntime.Messaging.void_objc_msgSend_IntPtr (this.Handle, selSetTextLists_Handle, nsa_array.Handle);
global::ObjCRuntime.Messaging.void_objc_msgSend_IntPtr (this.Handle, selSetTextLists_XHandle, nsa_array.Handle);
} else {
global::ObjCRuntime.Messaging.void_objc_msgSendSuper_IntPtr (this.SuperHandle, selSetTextLists_Handle, nsa_array.Handle);
global::ObjCRuntime.Messaging.void_objc_msgSendSuper_IntPtr (this.SuperHandle, selSetTextLists_XHandle, nsa_array.Handle);
}
nsa_array.Dispose ();
}
Expand Down
4 changes: 2 additions & 2 deletions src/Foundation/NSArray.cs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ static public NSArray FromIntPtrs (NativeHandle [] vals)
internal static nuint GetCount (IntPtr handle)
{
#if MONOMAC
return (nuint) Messaging.UIntPtr_objc_msgSend (handle, selCountHandle);
return (nuint) Messaging.UIntPtr_objc_msgSend (handle, selCountXHandle);
#else
return (nuint) Messaging.UIntPtr_objc_msgSend (handle, Selector.GetHandle ("count"));
#endif
Expand All @@ -259,7 +259,7 @@ internal static NativeHandle GetAtIndex (NativeHandle handle, nuint i)
return Messaging.NativeHandle_objc_msgSend_UIntPtr (handle, Selector.GetHandle ("objectAtIndex:"), (UIntPtr) i);
#else
#if MONOMAC
return Messaging.IntPtr_objc_msgSend_UIntPtr (handle, selObjectAtIndex_Handle, (UIntPtr) i);
return Messaging.IntPtr_objc_msgSend_UIntPtr (handle, selObjectAtIndex_XHandle, (UIntPtr) i);
#else
return Messaging.IntPtr_objc_msgSend_UIntPtr (handle, Selector.GetHandle ("objectAtIndex:"), (UIntPtr) i);
#endif
Expand Down
2 changes: 1 addition & 1 deletion src/Foundation/NSDictionary.cs
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ public IEnumerator<KeyValuePair<NSObject, NSObject>> GetEnumerator ()
public IntPtr LowlevelObjectForKey (IntPtr key)
{
#if MONOMAC
return ObjCRuntime.Messaging.IntPtr_objc_msgSend_IntPtr (this.Handle, selObjectForKey_Handle, key);
return ObjCRuntime.Messaging.IntPtr_objc_msgSend_IntPtr (this.Handle, selObjectForKey_XHandle, key);
#else
return ObjCRuntime.Messaging.IntPtr_objc_msgSend_IntPtr (this.Handle, Selector.GetHandle ("objectForKey:"), key);
#endif
Expand Down
4 changes: 2 additions & 2 deletions src/Foundation/NSMutableDictionary.cs
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ public IEnumerator<KeyValuePair<NSObject, NSObject>> GetEnumerator ()
public static NSMutableDictionary LowlevelFromObjectAndKey (IntPtr obj, IntPtr key)
{
#if MONOMAC
return (NSMutableDictionary) Runtime.GetNSObject (ObjCRuntime.Messaging.IntPtr_objc_msgSend_IntPtr_IntPtr (class_ptr, selDictionaryWithObject_ForKey_Handle, obj, key));
return (NSMutableDictionary) Runtime.GetNSObject (ObjCRuntime.Messaging.IntPtr_objc_msgSend_IntPtr_IntPtr (class_ptr, selDictionaryWithObject_ForKey_XHandle, obj, key));
#else
return (NSMutableDictionary) Runtime.GetNSObject (ObjCRuntime.Messaging.IntPtr_objc_msgSend_IntPtr_IntPtr (class_ptr, Selector.GetHandle ("dictionaryWithObject:forKey:"), obj, key));
#endif
Expand All @@ -328,7 +328,7 @@ public static NSMutableDictionary LowlevelFromObjectAndKey (IntPtr obj, IntPtr k
public void LowlevelSetObject (IntPtr obj, IntPtr key)
{
#if MONOMAC
ObjCRuntime.Messaging.void_objc_msgSend_IntPtr_IntPtr (this.Handle, selSetObject_ForKey_Handle, obj, key);
ObjCRuntime.Messaging.void_objc_msgSend_IntPtr_IntPtr (this.Handle, selSetObject_ForKey_XHandle, obj, key);
#else
ObjCRuntime.Messaging.void_objc_msgSend_IntPtr_IntPtr (this.Handle, Selector.GetHandle ("setObject:forKey:"), obj, key);
#endif
Expand Down
4 changes: 2 additions & 2 deletions src/Foundation/NSObject2.cs
Original file line number Diff line number Diff line change
Expand Up @@ -886,9 +886,9 @@ public void SetValueForKeyPath (NativeHandle handle, NSString keyPath)
#else
#if MONOMAC
if (IsDirectBinding) {
ObjCRuntime.Messaging.void_objc_msgSend_IntPtr_IntPtr (this.Handle, selSetValue_ForKeyPath_Handle, handle, keyPath.Handle);
ObjCRuntime.Messaging.void_objc_msgSend_IntPtr_IntPtr (this.Handle, selSetValue_ForKeyPath_XHandle, handle, keyPath.Handle);
} else {
ObjCRuntime.Messaging.void_objc_msgSendSuper_IntPtr_IntPtr (this.SuperHandle, selSetValue_ForKeyPath_Handle, handle, keyPath.Handle);
ObjCRuntime.Messaging.void_objc_msgSendSuper_IntPtr_IntPtr (this.SuperHandle, selSetValue_ForKeyPath_XHandle, handle, keyPath.Handle);
}
#else
if (IsDirectBinding) {
Expand Down
4 changes: 2 additions & 2 deletions src/Foundation/NSUrlProtocol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ public NSUrlProtocol (NSUrlRequest request, NSCachedUrlResponse cachedResponse,
if (client is null)
throw new ArgumentNullException ("client");
if (IsDirectBinding) {
InitializeHandle (IntPtr_objc_msgSend_IntPtr_IntPtr_IntPtr (this.Handle, selInitWithRequest_CachedResponse_Client_Handle, request.Handle, cachedResponse is null ? IntPtr.Zero : cachedResponse.Handle, client.Handle), "initWithRequest:cachedResponse:client:");
InitializeHandle (IntPtr_objc_msgSend_IntPtr_IntPtr_IntPtr (this.Handle, selInitWithRequest_CachedResponse_Client_XHandle, request.Handle, cachedResponse is null ? IntPtr.Zero : cachedResponse.Handle, client.Handle), "initWithRequest:cachedResponse:client:");
} else {
InitializeHandle (IntPtr_objc_msgSendSuper_IntPtr_IntPtr_IntPtr (this.SuperHandle, selInitWithRequest_CachedResponse_Client_Handle, request.Handle, cachedResponse is null ? IntPtr.Zero : cachedResponse.Handle, client.Handle), "initWithRequest:cachedResponse:client:");
InitializeHandle (IntPtr_objc_msgSendSuper_IntPtr_IntPtr_IntPtr (this.SuperHandle, selInitWithRequest_CachedResponse_Client_XHandle, request.Handle, cachedResponse is null ? IntPtr.Zero : cachedResponse.Handle, client.Handle), "initWithRequest:cachedResponse:client:");
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/bgen/Generator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2669,8 +2669,8 @@ public string SelectorField (string s, bool ignore_inline_directive = false)
} else
sb.Append (c);
}
if (!InlineSelectors)
sb.Append ("Handle");
if (!InlineSelectors)
sb.Append ("XHandle");
name = sb.ToString ();
selector_names [s] = name;
return name;
Expand Down
3 changes: 3 additions & 0 deletions tests/generator/BGenTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -824,6 +824,9 @@ public void DiamondProtocol ()
[Test]
public void GHIssue9065_Sealed () => BuildFile (Profile.iOS, nowarnings: true, "ghissue9065.cs");

[Test]
public void GHIssue18645_DuplicatedFiled() => BuildFile (Profile.iOS, nowarnings: true, "ghissue18645.cs");

// looking for [BindingImpl (BindingImplOptions.Optimizable)]
bool IsOptimizable (MethodDefinition method)
{
Expand Down
44 changes: 44 additions & 0 deletions tests/generator/ghissue18645.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
using Foundation;
using ObjCRuntime;

namespace GHIssue18645{

[Protocol]
[BaseType (typeof(NSObject))]
interface ASCredentialIdentity {

[Abstract]
[Export ("user")]
string User { get; }

[Abstract]
[NullAllowed, Export ("recordIdentifier")]
string RecordIdentifier { get; }

[Abstract]
[Export ("rank")]
nint Rank { get; set; }
}

[BaseType (typeof (NSObject))]
interface ASPasskeyCredentialIdentity : ASCredentialIdentity {
[Export ("relyingPartyIdentifier")]
string RelyingPartyIdentifier { get; }

[Export ("userName")]
string UserName { get; }

[Export ("credentialID", ArgumentSemantic.Copy)]
NSData CredentialID { get; }

[Export ("userHandle", ArgumentSemantic.Copy)]
NSData UserHandle { get; }

[NullAllowed, Export ("recordIdentifier")]
new string RecordIdentifier { get; }

[Export ("rank")]
new nint Rank { get; set; }
}

}

0 comments on commit 70f6d30

Please sign in to comment.