Skip to content

Commit 4444d7f

Browse files
hazzikhazzik
committed
Treat all non-virtual methods of System.Object as not proxiable
1 parent 25be55c commit 4444d7f

File tree

4 files changed

+130
-102
lines changed

4 files changed

+130
-102
lines changed

‎src/NHibernate.Test/NHSpecificTest/ProxyValidator/Fixture.cs

Lines changed: 48 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
using System;
22
using NHibernate.Proxy;
33
using NUnit.Framework;
4-
using System.Collections.Generic;
54

65
namespace NHibernate.Test.NHSpecificTest.ProxyValidator
76
{
@@ -10,15 +9,6 @@ public class Fixture
109
{
1110
private readonly IProxyValidator pv = new DynProxyTypeValidator();
1211

13-
private void Validate(System.Type type)
14-
{
15-
ICollection<string> errors = pv.ValidateType(type);
16-
if (errors != null)
17-
{
18-
throw new InvalidProxyTypeException(errors);
19-
}
20-
}
21-
2212
public class ValidClass
2313
{
2414
private int privateField;
@@ -64,12 +54,35 @@ protected int NonVirtualPrivateProperty
6454
#pragma warning restore 67
6555
}
6656

57+
[Test]
58+
public void ObjectIsValid()
59+
{
60+
var errors = pv.ValidateType(typeof(object));
61+
Assert.That(errors, Is.Null);
62+
}
63+
6764
[Test]
6865
public void ValidClassTest()
6966
{
70-
Validate(typeof(ValidClass));
67+
var errors = pv.ValidateType(typeof(ValidClass));
68+
Assert.That(errors, Is.Null);
7169
}
7270

71+
public class InvalidSealedToString : ValidClass
72+
{
73+
public sealed override string ToString()
74+
{
75+
return base.ToString();
76+
}
77+
}
78+
79+
[Test]
80+
public void SealedObjectOverride()
81+
{
82+
var errors = pv.ValidateType(typeof(InvalidSealedToString));
83+
Assert.That(errors, Is.Not.Null);
84+
}
85+
7386
public class InvalidPrivateConstructor : ValidClass
7487
{
7588
private InvalidPrivateConstructor()
@@ -80,7 +93,8 @@ private InvalidPrivateConstructor()
8093
[Test]
8194
public void PrivateConstructor()
8295
{
83-
Assert.Throws<InvalidProxyTypeException>(() => Validate(typeof(InvalidPrivateConstructor)));
96+
var errors = pv.ValidateType(typeof(InvalidPrivateConstructor));
97+
Assert.That(errors, Is.Not.Null);
8498
}
8599

86100
public class InvalidNonVirtualProperty : ValidClass
@@ -95,7 +109,8 @@ public int NonVirtualProperty
95109
[Test]
96110
public void NonVirtualProperty()
97111
{
98-
Assert.Throws<InvalidProxyTypeException>(() => Validate(typeof(InvalidNonVirtualProperty)));
112+
var errors = pv.ValidateType(typeof(InvalidNonVirtualProperty));
113+
Assert.That(errors, Is.Not.Null);
99114
}
100115

101116
public class InvalidPublicField : ValidClass
@@ -106,7 +121,8 @@ public class InvalidPublicField : ValidClass
106121
[Test]
107122
public void PublicField()
108123
{
109-
Assert.Throws<InvalidProxyTypeException>(() => Validate(typeof(InvalidPublicField)));
124+
var errors = pv.ValidateType(typeof(InvalidPublicField));
125+
Assert.That(errors, Is.Not.Null);
110126
}
111127

112128
public class InvalidNonVirtualEvent : ValidClass
@@ -119,7 +135,8 @@ public class InvalidNonVirtualEvent : ValidClass
119135
[Test]
120136
public void NonVirtualEvent()
121137
{
122-
Assert.Throws<InvalidProxyTypeException>(() => Validate(typeof(InvalidNonVirtualEvent)));
138+
var errors = pv.ValidateType(typeof(InvalidNonVirtualEvent));
139+
Assert.That(errors, Is.Not.Null);
123140
}
124141

125142
public interface ValidInterface
@@ -129,7 +146,8 @@ public interface ValidInterface
129146
[Test]
130147
public void Interface()
131148
{
132-
Validate(typeof(ValidInterface));
149+
var errors = pv.ValidateType(typeof(ValidInterface));
150+
Assert.That(errors, Is.Null);
133151
}
134152

135153
public class MultipleErrors
@@ -153,15 +171,8 @@ public int NonVirtualProperty
153171
[Test]
154172
public void MultipleErrorsReported()
155173
{
156-
try
157-
{
158-
Validate(typeof(MultipleErrors));
159-
Assert.Fail("Should have failed validation");
160-
}
161-
catch (InvalidProxyTypeException e)
162-
{
163-
Assert.IsTrue(e.Errors.Count > 1);
164-
}
174+
var errors = pv.ValidateType(typeof(MultipleErrors));
175+
Assert.That(errors, Has.Count.GreaterThan(1));
165176
}
166177

167178
public class InvalidNonVirtualInternalProperty : ValidClass
@@ -183,13 +194,15 @@ public class InvalidInternalField : ValidClass
183194
[Test]
184195
public void NonVirtualInternal()
185196
{
186-
Assert.Throws<InvalidProxyTypeException>(() => Validate(typeof(InvalidNonVirtualInternalProperty)));
197+
var errors = pv.ValidateType(typeof(InvalidNonVirtualInternalProperty));
198+
Assert.That(errors, Is.Not.Null);
187199
}
188200

189201
[Test]
190202
public void InternalField()
191203
{
192-
Assert.Throws<InvalidProxyTypeException>(() => Validate(typeof(InvalidInternalField)));
204+
var errors = pv.ValidateType(typeof(InvalidInternalField));
205+
Assert.That(errors, Is.Not.Null);
193206
}
194207

195208
public class InvalidNonVirtualProtectedProperty : ValidClass
@@ -204,8 +217,8 @@ protected int NonVirtualProperty
204217
[Test]
205218
public void NonVirtualProtected()
206219
{
207-
Validate(typeof(InvalidNonVirtualProtectedProperty));
208-
Assert.IsTrue(true, "Always should pass, protected members do not need to be virtual.");
220+
var errors = pv.ValidateType(typeof(InvalidNonVirtualProtectedProperty));
221+
Assert.That(errors, Is.Null);
209222
}
210223

211224
public class InvalidNonVirtualProtectedInternalProperty : ValidClass
@@ -220,7 +233,8 @@ protected internal int NonVirtualProperty
220233
[Test]
221234
public void NonVirtualProtectedInternal()
222235
{
223-
Assert.Throws<InvalidProxyTypeException>(() => Validate(typeof(InvalidNonVirtualProtectedInternalProperty)));
236+
var errors = pv.ValidateType(typeof(InvalidNonVirtualProtectedInternalProperty));
237+
Assert.That(errors, Is.Not.Null);
224238
}
225239

226240
interface INonVirtualPublicImplementsInterface
@@ -239,7 +253,8 @@ public int NonVirtualMethodImplementsInterface
239253
[Test]
240254
public void VirtualPublicImplementsInterface()
241255
{
242-
Assert.Throws<InvalidProxyTypeException>(() => Validate(typeof(NonVirtualPublicImplementsInterface)));
256+
var errors = pv.ValidateType(typeof(NonVirtualPublicImplementsInterface));
257+
Assert.That(errors, Is.Not.Null);
243258
}
244259

245260
public class InvalidVirtualPrivateAutoProperty : ValidClass
@@ -254,7 +269,8 @@ public virtual int NonVirtualSetterProperty
254269
[Test]
255270
public void PrivateSetterOnVirtualPropertyShouldThrows()
256271
{
257-
Assert.Throws<InvalidProxyTypeException>(() => Validate(typeof(InvalidVirtualPrivateAutoProperty)));
272+
var errors = pv.ValidateType(typeof(InvalidVirtualPrivateAutoProperty));
273+
Assert.That(errors, Is.Not.Null);
258274
}
259275
}
260276
}

‎src/NHibernate.Test/NHSpecificTest/ProxyValidator/ShouldBeProxiableTests.cs

Lines changed: 72 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -46,37 +46,78 @@ protected internal void AProtectedInternal() { }
4646
}
4747

4848
[Test]
49-
public void GetTypeNotBeProxiable()
49+
public void GetTypeShouldNotBeProxiable()
5050
{
51-
var method = typeof(object).GetMethod("GetType");
52-
Assert.That(method.ShouldBeProxiable(), Is.False);
51+
var method = typeof(object).GetMethod(nameof(GetType));
52+
Assert.Multiple(
53+
() =>
54+
{
55+
Assert.That(method.ShouldBeProxiable(), Is.False);
56+
Assert.That(method.IsProxiable(), Is.False);
57+
});
5358
}
5459

5560
[Test]
56-
public void DisposeNotBeProxiable()
61+
public void ToStringShouldBeProxiable()
5762
{
58-
var method = typeof(MyClass).GetMethod("Dispose");
59-
Assert.That(method.ShouldBeProxiable(), Is.False);
63+
var method = typeof(object).GetMethod(nameof(ToString));
64+
Assert.Multiple(
65+
() =>
66+
{
67+
Assert.That(method.ShouldBeProxiable(), Is.True);
68+
Assert.That(method.IsProxiable(), Is.True);
69+
});
6070
}
6171

6272
[Test]
63-
public void ObjectDestructorShouldNotBeProxiable()
73+
public void GetHashCodeShouldBeProxiable()
6474
{
65-
var method = typeof(object).GetMethod(
66-
"Finalize",
67-
BindingFlags.NonPublic | BindingFlags.Instance);
75+
var method = typeof(object).GetMethod(nameof(GetHashCode));
76+
Assert.Multiple(
77+
() =>
78+
{
79+
Assert.That(method.ShouldBeProxiable(), Is.True);
80+
Assert.That(method.IsProxiable(), Is.True);
81+
});
82+
}
6883

69-
Assert.That(method.ShouldBeProxiable(), Is.False);
84+
[Test]
85+
public void EqualsShouldBeProxiable()
86+
{
87+
var method = typeof(object).GetMethod(nameof(Equals), BindingFlags.Public | BindingFlags.Instance);
88+
Assert.Multiple(
89+
() =>
90+
{
91+
Assert.That(method.ShouldBeProxiable(), Is.True);
92+
Assert.That(method.IsProxiable(), Is.True);
93+
});
94+
}
95+
96+
[Test]
97+
public void DisposeNotBeProxiable()
98+
{
99+
var method = typeof(MyClass).GetMethod("Dispose");
100+
Assert.Multiple(
101+
() =>
102+
{
103+
Assert.That(method.ShouldBeProxiable(), Is.False);
104+
Assert.That(method.IsProxiable(), Is.False);
105+
});
70106
}
71107

72108
[Test]
73-
public void ObjectDestructorIsNotProxiable()
109+
public void ObjectDestructorShouldNotBeProxiable()
74110
{
75111
var method = typeof(object).GetMethod(
76112
"Finalize",
77113
BindingFlags.NonPublic | BindingFlags.Instance);
78114

79-
Assert.That(method.IsProxiable(), Is.False);
115+
Assert.Multiple(
116+
() =>
117+
{
118+
Assert.That(method.ShouldBeProxiable(), Is.False);
119+
Assert.That(method.IsProxiable(), Is.False);
120+
});
80121
}
81122

82123
[Test]
@@ -89,20 +130,12 @@ public void MyClassDestructorShouldNotBeProxiable()
89130
System.Type.EmptyTypes,
90131
null);
91132

92-
Assert.That(method.ShouldBeProxiable(), Is.False);
93-
}
94-
95-
[Test]
96-
public void MyClassDestructorIsNotProxiable()
97-
{
98-
var method = typeof(MyClass).GetMethod(
99-
"Finalize",
100-
BindingFlags.NonPublic | BindingFlags.Instance,
101-
null,
102-
System.Type.EmptyTypes,
103-
null);
104-
105-
Assert.That(method.IsProxiable(), Is.False);
133+
Assert.Multiple(
134+
() =>
135+
{
136+
Assert.That(method.ShouldBeProxiable(), Is.False);
137+
Assert.That(method.IsProxiable(), Is.False);
138+
});
106139
}
107140

108141
[Test]
@@ -115,22 +148,14 @@ public void MyClassLowerCaseFinalizeShouldBeProxiable()
115148
System.Type.EmptyTypes,
116149
null);
117150

118-
Assert.That(method.ShouldBeProxiable(), Is.True);
151+
Assert.Multiple(
152+
() =>
153+
{
154+
Assert.That(method.ShouldBeProxiable(), Is.True);
155+
Assert.That(method.IsProxiable(), Is.True);
156+
});
119157
}
120158

121-
[Test]
122-
public void MyClassLowerCaseFinalizeIsProxiable()
123-
{
124-
var method = typeof(MyClass).GetMethod(
125-
"finalize",
126-
BindingFlags.Public | BindingFlags.Instance,
127-
null,
128-
System.Type.EmptyTypes,
129-
null);
130-
131-
Assert.That(method.IsProxiable(), Is.True);
132-
}
133-
134159
[Test]
135160
public void MyClassFinalizeWithParametersShouldBeProxiable()
136161
{
@@ -141,20 +166,12 @@ public void MyClassFinalizeWithParametersShouldBeProxiable()
141166
new[] { typeof(int) },
142167
null);
143168

144-
Assert.That(method.ShouldBeProxiable(), Is.True);
145-
}
146-
147-
[Test]
148-
public void MyClassFinalizeWithParametersIsProxiable()
149-
{
150-
var method = typeof(MyClass).GetMethod(
151-
"Finalize",
152-
BindingFlags.Public | BindingFlags.Instance,
153-
null,
154-
new[] { typeof(int) },
155-
null);
156-
157-
Assert.That(method.IsProxiable(), Is.True);
169+
Assert.Multiple(
170+
() =>
171+
{
172+
Assert.That(method.ShouldBeProxiable(), Is.True);
173+
Assert.That(method.IsProxiable(), Is.True);
174+
});
158175
}
159176

160177
[Test]

‎src/NHibernate/Proxy/DefaultDynamicProxyMethodCheckerExtensions.cs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,10 @@ public static bool IsProxiable(this MethodInfo method)
2323
public static bool ShouldBeProxiable(this MethodInfo method)
2424
{
2525
// to use only for real methods (no getter/setter)
26-
return (method.DeclaringType != typeof (MarshalByRefObject)) &&
27-
!IsFinalizeMethod(method) &&
28-
(!(method.DeclaringType == typeof (object) && "GetType".Equals(method.Name))) &&
29-
(!(method.DeclaringType == typeof (object) && "obj_address".Equals(method.Name))) && // Mono-specific method
26+
return method.DeclaringType != typeof(MarshalByRefObject) &&
27+
!(method.DeclaringType == typeof(object) && !method.IsVirtual) &&
3028
!IsDisposeMethod(method) &&
31-
(method.IsPublic || method.IsAssembly || method.IsFamilyOrAssembly);
29+
(method.IsPublic || method.IsAssembly || method.IsFamilyOrAssembly);
3230
}
3331

3432
public static bool ShouldBeProxiable(this PropertyInfo propertyInfo)

0 commit comments

Comments
 (0)