Don’t write code like this…
I’ve just finished doing a security review of my MSDN columns, and as part of that,
I needed to add some security to an add-in architecture, so the add-in architecture
won’t run antisocial code. The approach taken by .NET security is reminiscent of ACLs,
which means (at least to me) that it’s confusing and I need to read the code 5 or
6 times before I understand it. Along the way, I got a chunk of code from somebody
on the .NET security team, which I’ll reproduce here:
private NamedPermissionSet FindNamedPermissionSet(string name)
{
IEnumerator policyEnumerator = SecurityManager.PolicyHierarchy();
while (policyEnumerator.MoveNext())
{
PolicyLevel currentLevel = (PolicyLevel)policyEnumerator.Current;
if (currentLevel.Label == "Machine")
{
IList namedPermissions = currentLevel.NamedPermissionSets;
IEnumerator namedPermission = namedPermissions.GetEnumerator();
while (namedPermission.MoveNext())
{
if (((NamedPermissionSet)namedPermission.Current).Name == name)
{
return ((NamedPermissionSet)namedPermission.Current);
}
}
}
}
return null;
}
Ugly, isn’t it?
The root of the problem is that PolicyHierarchy returns an IEnumerator instead of
an object that supports IENumerable. This means that I can’t use foreach on
it, and have to write the traversal myself. This decision also means that, without
a collection returned, there’s no place to hang a useful method like “LookupLabel()”
on, so all users are stuck writing the search code themselves (or encapsulating it
off into a separate method).
PolicyLevel.NamedPermissionSets uses a different approach, that of returning an IList,
which is better than an enumerator, as I could use foreach on the IList, but the person
who wrote this sample didn’t because they didn’t know about foreach. If they had,
they would have done the right thing.
We need to not do this.
Been there, wrote that code, cursed the guy who create the PolicyHierarchy method.
Hi,
Its probably not the right place to post this, but while we’re on Enumerators, I was writing a rot13 implementation in C# the other day and realised that CharEnumerator returns a read-only char, e.g.
foreach (char alpha in "A String") {
alpha = ‘a’ ; // this isn’t allowed as alpha is read-only ???
…
What I don’t understand is that char is value type and what I have in alpha is a copy and not a reference so, how it becomes read-only ? I am not writing into the string, just assigning to a char variable.
Regards,
Richard.
Ick! Yeah, that is pretty ugly. I also find it quite interesting to hear that person who coded this "didn’t know about foreach". Scary.
Ick! Yeah, that is pretty ugly. I also find it quite interesting to hear that person who coded this "didn’t know about foreach". Scary.
Yeah, looks ugly, but IMO using Foreach instead is not going to add alot of improvement – you’ll still have to loop through, have your own lookup routine – something that should’ve been provided by an interface returned from the methods in first place.
Disagree with the comment above. It’s not a question of what but where.
Agree with Eric and others on IEnumerators vs. IEnumerable. The urge to refactor into something cleaner when you hit this is palpable.
I was going to say that you can do this in VB.NET because I remembered the spec mentions something about working with a class if it has the methods that are specified by IEnumerable even it doesn’t explicitly declare implementing that interface.
But this only happens if the object is obtained by a call to GetEnumerator, like the inner loop, and C# can do it too.
So I see this as an arbitrary limitation of both languages.
My VB workaround that highlights this:
Private Class MyPolicyHierarchy
Public Function GetEnumerator() As IEnumerator
Return SecurityManager.PolicyHierarchy
End Function
End Class
Private Function FindNamedPermissionSet(ByVal name As String)
_ As NamedPermissionSet
For Each currentLevel As PolicyLevel In New MyPolicyHierarchy()
If currentLevel.Label = "Machine" Then
For Each namedPermission As NamedPermissionSet _
In currentLevel.NamedPermissionSets
If namedPermission.Name = name Then
Return namedPermission
End If
Next
End If
Next
Return Nothing
End Function
And a DotLisp sample that doesn’t have this problem (though it needs to jump through other hoops to stop searching when the PermissionSet is found):
(for-each current-level (SecurityManager:PolicyHierarchy)
(when (== current-level.Label "Machine")
(for-each named-permission current-level.NamedPermissionSets
(when (== named-permission.Name "LocalIntranet")
(set intranet-ps named-permission)))))
Helpful For MBA Fans.