Skip to content

Commit f2b8f32

Browse files
atsushienoatsushieno
committed
[api-xml-adjuster] method overrides cannot expose non-public types.
Fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=44531 When a Java method is overriden, its parameter type or return type can be variant, which possibly result in exposure of non-public types (returning private class object which is derived from public type is totally valid). It is because we blindly emit the parsed type from class-parse output as the API XML adjuster output. It needs to be examined and adjusted to be public (or protected) type. jar2xml didn't have this problem because Java reflection API is clever enough to cover that issue. The fixes are... not cosmetic. Basically now we use TypeReference.ToString() which used to not be precise and ready for API XML output. So there are couple of fixes for that. (We also want to track from which method base a parameter or return type is bound, so we pass that to the .ctor now). We hide extraneous public TypeParameter.ctor() now to reduce chances for bugs. Tests need to be modified to reflect the changes. Lastly, Mono.Android API XML will be updated to reflect this change. There will be some removal of method overrides from api-*.xml.in but there was no regression reported from api-diff.
1 parent c51469e commit f2b8f32

10 files changed

+98
-20
lines changed

‎src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApi.XmlModel.cs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,12 @@ public override string ToString ()
253253

254254
public partial class JavaParameter
255255
{
256+
public JavaParameter (JavaMethodBase parent)
257+
{
258+
Parent = parent;
259+
}
260+
261+
public JavaMethodBase Parent { get; set; }
256262
public string Name { get; set; }
257263
public string Type { get; set; }
258264

@@ -309,7 +315,7 @@ public JavaTypeParameter (JavaTypeParameters parent)
309315

310316
public override string ToString ()
311317
{
312-
return Name + (GenericConstraints == null ? null : GenericConstraints.ToString ());
318+
return Name;
313319
}
314320
}
315321

@@ -326,7 +332,10 @@ public JavaGenericConstraints ()
326332

327333
public override string ToString ()
328334
{
329-
return " " + (BoundsType ?? "extends") + " " + string.Join (" & ", GenericConstraints);
335+
string csts = string.Join (" & ", GenericConstraints);
336+
if (csts == "java.lang.Object")
337+
return string.Empty;
338+
return " " + (BoundsType ?? "extends") + " " + csts;
330339
}
331340
}
332341

‎src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApiFixVisibilityExtensions.cs

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
using System;
2+
using System.Linq;
3+
4+
namespace Xamarin.Android.Tools.ApiXmlAdjuster
5+
{
6+
public static class JavaApiFixVisibilityExtensions
7+
{
8+
public static string GetVisibleTypeName (this JavaParameter parameter)
9+
{
10+
var r = GetVisibleNonSpecialType (parameter);
11+
return r != null ? r.ToString () : parameter.Type;
12+
}
13+
14+
public static string GetVisibleReturnTypeName (this JavaMethod method)
15+
{
16+
var r = GetVisibleNonSpecialReturnType (method);
17+
return r != null ? r.ToString () : method.Return;
18+
}
19+
20+
public static JavaTypeReference GetVisibleNonSpecialType (this JavaParameter parameter)
21+
{
22+
return GetVisibleNonSpecialType (parameter.Parent, parameter.ResolvedType);
23+
}
24+
25+
public static JavaTypeReference GetVisibleNonSpecialReturnType (this JavaMethod method)
26+
{
27+
return GetVisibleNonSpecialType (method, method.ResolvedReturnType);
28+
}
29+
30+
static JavaTypeReference GetVisibleNonSpecialType (this JavaMethodBase method, JavaTypeReference r)
31+
{
32+
if (r == null || r.SpecialName != null || r.ReferencedTypeParameter != null || r.ArrayPart != null)
33+
return null;
34+
var requiredVisibility = method.Visibility == "public" && method.Parent.Visibility == "public" ? "public" : method.Visibility;
35+
for (var t = r; t != null; t = (t.ReferencedType as JavaClass)?.ResolvedExtends) {
36+
if (t.ReferencedType == null)
37+
break;
38+
if (IsAcceptableVisibility (required: requiredVisibility, actual: t.ReferencedType.Visibility))
39+
return t;
40+
}
41+
return null;
42+
}
43+
44+
static bool IsAcceptableVisibility (string required, string actual)
45+
{
46+
if (required == "public")
47+
return actual == "public";
48+
else
49+
return true;
50+
}
51+
}
52+
}

‎src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApiTypeResolverExtensions.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,11 @@ static JavaTypeReference JavaTypeNameToReference (this JavaApi api, JavaTypeName
2424
var tp = contextTypeParameters.Where (tps => tps != null).SelectMany (tps => tps.TypeParameters).FirstOrDefault (_ => _.Name == tn.FullNameNonGeneric);
2525
if (tp != null)
2626
return new JavaTypeReference (tp, tn.ArrayPart);
27+
if (tn.FullNameNonGeneric == JavaTypeReference.GenericWildcard.SpecialName)
28+
return new JavaTypeReference (tn.BoundsType, tn.GenericConstraints?.Select (gc => JavaTypeNameToReference (api, gc, contextTypeParameters)), tn.ArrayPart);
2729
var primitive = JavaTypeReference.GetSpecialType (tn.FullNameNonGeneric);
2830
if (primitive != null)
29-
return tn.ArrayPart == null ? primitive : new JavaTypeReference (primitive, tn.ArrayPart);
31+
return tn.ArrayPart == null && tn.GenericConstraints == null ? primitive : new JavaTypeReference (primitive, tn.ArrayPart, tn.BoundsType, tn.GenericConstraints?.Select (gc => JavaTypeNameToReference (api, gc, contextTypeParameters)));
3032
var type = api.FindNonGenericType (tn.FullNameNonGeneric);
3133
return new JavaTypeReference (type,
3234
tn.GenericArguments != null ? tn.GenericArguments.Select (_ => api.JavaTypeNameToReference (_, contextTypeParameters)).ToArray () : null,
@@ -111,7 +113,7 @@ static void ResolveMethodBase (this JavaMethodBase m, JavaTypeParameters methodT
111113
public static void Resolve (this JavaMethod m)
112114
{
113115
if (m.TypeParameters != null)
114-
m.TypeParameters.Resolve (m.GetApi (), m.Parent.TypeParameters);
116+
m.TypeParameters.Resolve (m.GetApi (), m.TypeParameters);
115117
m.ResolveMethodBase (m.TypeParameters);
116118
m.ResolvedReturnType = m.GetApi ().Parse (m.Return, m.Parent.TypeParameters, m.TypeParameters);
117119
}

‎src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApiXmlGeneratorExtensions.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ static void Save (this JavaMethod method, XmlWriter writer)
176176
SaveCommon (method, writer, "method",
177177
XmlConvert.ToString (method.Abstract),
178178
XmlConvert.ToString (method.Native),
179-
method.Return,
179+
method.GetVisibleReturnTypeName (),
180180
XmlConvert.ToString (method.Synchronized),
181181
null,
182182
null,
@@ -232,8 +232,8 @@ static void SaveCommon (this JavaMember m, XmlWriter writer, string elementName,
232232
foreach (var p in parameters) {
233233
writer.WriteStartElement ("parameter");
234234
writer.WriteAttributeString ("name", p.Name);
235-
writer.WriteAttributeString ("type", p.Type);
236-
writer.WriteString ("\n ");
235+
writer.WriteAttributeString ("type", p.GetVisibleTypeName ());
236+
writer.WriteString ("\n ");
237237
writer.WriteFullEndElement ();
238238
}
239239
}

‎src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApiXmlLoaderExtensions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ static void LoadMethodBase (this JavaMethodBase methodBase, string elementName,
242242
tp.Load (reader);
243243
method.TypeParameters = tp;
244244
} else if (reader.LocalName == "parameter") {
245-
var p = new JavaParameter ();
245+
var p = new JavaParameter (methodBase);
246246
p.Load (reader);
247247
methodBase.Parameters.Add (p);
248248
} else if (reader.LocalName == "exception") {

‎src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaTypeName.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ public static JavaTypeName Parse (string dottedFullName)
4040
int gccidx = gcidx < 0 ? -1 : dottedFullName.IndexOf (',', 0, gcidx);
4141
if (gcidx > 0 && gcgidx < 0 && gccidx < 0) {
4242
string args = dottedFullName.Substring (gcidx + label.Length).Trim ();
43+
ret.BoundsType = label;
4344
ret.GenericConstraints = ParseCommaSeparatedTypeNames (args).Select (s => Parse (s)).ToArray ();
4445
dottedFullName = dottedFullName.Substring (0, gcidx).Trim ();
4546
}
@@ -104,6 +105,7 @@ static IEnumerable<string> ParseCommaSeparatedTypeNames (string args)
104105
}
105106

106107
public string FullNameNonGeneric { get; set; }
108+
public string BoundsType { get; set; } // " extends " / " super "
107109
public IList<JavaTypeName> GenericConstraints { get; private set; }
108110
public IList<JavaTypeName> GenericArguments { get; private set; }
109111
public string ArrayPart { get; set; }

‎src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaTypeReference.cs

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,17 +48,28 @@ static JavaTypeReference ()
4848
GenericWildcard = new JavaTypeReference ("?");
4949
}
5050

51-
public JavaTypeReference (string specialName)
51+
JavaTypeReference (string specialName)
5252
{
5353
SpecialName = specialName;
5454
}
5555

56-
public JavaTypeReference (JavaTypeReference referencedType, string arrayPart)
56+
public JavaTypeReference (string constraintLabel, IEnumerable<JavaTypeReference> wildcardConstraints, string arrayPart)
57+
{
58+
SpecialName = GenericWildcard.SpecialName;
59+
ArrayPart = arrayPart;
60+
WildcardBoundsType = constraintLabel;
61+
WildcardConstraints = wildcardConstraints != null && wildcardConstraints.Any () ? wildcardConstraints.ToList () : null;
62+
}
63+
64+
public JavaTypeReference (JavaTypeReference referencedType, string arrayPart, string wildcardBoundsType, IEnumerable<JavaTypeReference> wildcardConstraints)
5765
{
5866
if (referencedType == null)
5967
throw new ArgumentNullException ("referencedType");
6068
SpecialName = referencedType.SpecialName;
69+
WildcardBoundsType = wildcardBoundsType;
70+
WildcardConstraints = wildcardConstraints?.ToList ();
6171
ReferencedType = referencedType.ReferencedType;
72+
ReferencedTypeParameter = referencedType.ReferencedTypeParameter;
6273
TypeParameters = referencedType.TypeParameters;
6374
ArrayPart = arrayPart;
6475
}
@@ -81,17 +92,23 @@ public JavaTypeReference (JavaType referencedType, IList<JavaTypeReference> type
8192
}
8293

8394
public string SpecialName { get; private set; }
95+
96+
public string WildcardBoundsType { get; private set; }
97+
public IList<JavaTypeReference> WildcardConstraints { get; private set; }
98+
8499
public JavaType ReferencedType { get; private set; }
85100
public JavaTypeParameter ReferencedTypeParameter { get; private set; }
86101
public IList<JavaTypeReference> TypeParameters { get; private set; }
87102
public string ArrayPart { get; private set; }
88103

89104
public override string ToString ()
90105
{
91-
if (SpecialName != null)
92-
return SpecialName;
106+
if (SpecialName == GenericWildcard.SpecialName && WildcardConstraints != null)
107+
return SpecialName + WildcardBoundsType + string.Join (" & ", WildcardConstraints);
108+
else if (SpecialName != null)
109+
return SpecialName + ArrayPart;
93110
else if (ReferencedTypeParameter != null)
94-
return ReferencedTypeParameter.Name;
111+
return ReferencedTypeParameter.ToString () + ArrayPart;
95112
else
96113
return string.Format ("{0}{1}{2}{3}{4}",
97114
ReferencedType.Parent.Name,

‎src/Xamarin.Android.Tools.ApiXmlAdjuster/Tests/TypeResolverTest.cs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,19 +18,14 @@ public void SetupFixture ()
1818
[Test]
1919
public void TypeReferenceEquals ()
2020
{
21-
var intRef = new JavaTypeReference ("int");
22-
Assert.AreEqual (intRef, new JavaTypeReference ("int"), "primitive types");
21+
var intRef = JavaTypeReference.Int;
2322
Assert.AreEqual (JavaTypeReference.Int, intRef, "primitive types 2");
24-
Assert.AreNotEqual (new JavaTypeReference ("void"), intRef, "primitive types 3");
25-
Assert.AreNotEqual (intRef, new JavaTypeReference (intRef, "[]"), "primitive types: array vs. non-array");
26-
Assert.AreEqual (new JavaTypeReference (intRef, "[]"), new JavaTypeReference (intRef, "[]"), "primitive types: array vs. array");
2723

2824
var dummyType = new JavaClass (new JavaPackage (api) { Name = string.Empty }) { Name = "Dummy" };
2925
var tps = new JavaTypeParameters (dummyType);
3026
var gt = new JavaTypeParameter (tps) { Name = "T" };
3127
Assert.AreEqual (new JavaTypeReference (gt, null), new JavaTypeReference (new JavaTypeParameter (tps) { Name = "T"}, null), "type parameters");
3228
Assert.AreNotEqual (new JavaTypeReference (gt, null), new JavaTypeReference (new JavaTypeParameter (tps) { Name = "U"}, null), "type parameters 2");
33-
Assert.AreNotEqual (new JavaTypeReference (gt, null), new JavaTypeReference ("T"), "primitive vs. type parameters");
3429
Assert.AreNotEqual (new JavaTypeReference (gt, null), new JavaTypeReference (gt, "[]"), "type parameters: array vs. non-array");
3530
Assert.AreEqual (new JavaTypeReference (gt, "[]"), new JavaTypeReference (gt, "[]"), "type parameters: array vs. array");
3631

‎src/Xamarin.Android.Tools.ApiXmlAdjuster/Xamarin.Android.Tools.ApiXmlAdjuster.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
<Compile Include="JavaApiXmlGeneratorExtensions.cs" />
4949
<Compile Include="JavaApiDefectFinderExtensions.cs" />
5050
<Compile Include="JavaTypeResolutionUtil.cs" />
51+
<Compile Include="JavaApiFixVisibilityExtensions.cs" />
5152
</ItemGroup>
5253
<Import Project="$(MSBuildBinPath)\Microsoft.CSharp.targets" />
5354
</Project>

‎tools/generator/JavaApiDllLoaderExtensions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ static void Load (this JavaMethodBase method, MethodBase gm)
119119
{
120120
method.Deprecated = gm.Deprecated;
121121
method.Visibility = gm.Visibility;
122-
method.Parameters = gm.Parameters.Select (_ => new JavaParameter () {
122+
method.Parameters = gm.Parameters.Select (_ => new JavaParameter (method) {
123123
Name = _.JavaName,
124124
Type = _.RawNativeType,
125125
}).ToArray ();

0 commit comments

Comments
 (0)