Why Can't I Access A Protected Member From A Derived Class, Part Two: Why Can I?

Why Can't I Access A Protected Member From A Derived Class, Part Two: Why Can I?

Rate This
  • Comments 38

This is a follow-up to my 2005 post on the same subject  which I believe sets a personal record for the longest time between parts of a series. (Of course, I didn't know it was a series when I started it.) Please read the previous article in this series, as this post assumes knowledge of part one.

.......

OK, now that you've read that, it's clear why you can only access a protected member from an instance of an object known to be of a type at least as derived as the current context. You can therefore deduce the answer to the question asked to me by a (very polite) reader this morning: Why did this code compile in C# 2.0 but give an error in C# 3.0?

public abstract class Item{
  private Item _parent;
  public Item Parent {
    get { return _parent; }
    protected set { _parent = value; }
  }
}
public class Bag:Item{
  private List<Item> list = new List<Item>();
  public void Add(Item item)
  {
    item.Parent = this; // Error in C# 3.0
    list.Add(item); 
  }
}
public class Torch : Item { }

It compiled in C# 2.0 because the compiler had a bug. We forgot to enforce the semantics of "protected" access for the property setter. Though the compiler did not generate an error, it did generate code which would not pass the CLR verification check! The code would run if it happened to be fully trusted, but it was not safe to do so. We fixed the bug in C# 3.0 and took the breaking change.

This raises a more interesting point though. Now that we correctly prevent you from setting the "parent" reference in this manner, how would you implement the desired pattern? The reader wanted the following conditions to be met:

  • There are two kinds of items -- "leaf" items, and "container" items. Container items may contain either kind of item, so you could have a box containing a bag, which in turn contains a torch.
  • An item may be in a container, and if it is, its parent reference refers to that container.
  • These classes must be extensible by arbitrary third parties.
  • "Unauthorized" code must not be able to muck around with the parenting invariants. 

The reader was attempting to enforce these invariants by making the parent setter protected. But even in a world where it is legal to access a protected member from an arbitrary derived class, that does not ensure that the invariants are maintained! If you allow any derived class to muck with the parenting, then you are relying upon every third-party derived class to "play nicely" and maintain your invariant. If any of them are buggy or hostile, then who knows what can happen?

When I'm faced with this kind of problem, I try to go back to first principles. Considering each design constraint leads us to an implementation decision which implements that constraint:

  • There are two kinds of abstract items -- items and containers. Therefore, there should be two abstract base classes, not just one.
  • Containers are items. Therefore, the container class should derive from the item class.
  • The parenting invariant must be maintained across all containers. Therefore the invariant implementation must be inside the abstract container base class.
  • Every possible container requires "write" access to the parent state of every possible Item.  We want that access to be as restricted as possible. Ideally, we want it to be private. The only by-design way to get access to a private is to be inside the class.

And now a full solution becomes very straightforward:

using System;
using System.Collections.Generic;

public abstract class Item {
  public Item Parent { get; private set; }
  public abstract class Container : Item {
    private HashSet<Item> items = new HashSet<Item>();
    public void Add(Item item) {
      if (item.Parent != null)
        throw new Exception("Item has inconsistent containment.");
      item.Parent = this;
      items.Add(item);
    }
    public void Remove(Item item) {
      if (!Contains(item))
        throw new Exception("Container does not contain that item.");
      items.Remove(item);
      item.Parent = null;
    }
    public bool Contains(Item item) {
      return items.Contains(item);
    }
    public IEnumerable<Item> Items {
      // Do not just return items. Then the caller could cast it
      // to HashSet<Item> and then make modifications to your
      // internal state! Return a read-only sequence:
      get {
        foreach(Item item in items) yield return item;
      }
    }
  }
}

// These can be in third-party assemblies:

public class Bag : Item.Container { }
public class Box : Item.Container { }
public class Torch : Item { }
public class TreasureMap : Item { }

public class Program {
  public static void Main() {
    var map = new TreasureMap();
    var box = new Box();
    box.Add(map);
    var bag = new Bag();
    bag.Add(box);
    foreach(Item item in bag.Items)
      Console.WriteLine(item);
  }
}

Pretty slick eh?

A couple questions for you to ponder:

1) Suppose you were a hostile third party and you wanted to mess up the parenting invariant. Clearly, if you are sufficiently trusted, you can always use private reflection or unsafe code to muck around with the state directly, so that's not a very interesting attack. Any other bright ideas come to mind for ways that this code is vulnerable to tampering?

2) Suppose you wanted to make this hierarchy an immutable collection, where "Add" and "Remove" returned new collections rather than mutating the existing collection. How would you represent the parenting relationship?

Next time, another oddity involving "protected" semantics. Have a good weekend!

 

  • I noticed no one really addressed Eric's 2nd question about how to "make this hierarchy an immutable collection"  I could not really think of any good way to make the hierarchy immutable and have items know their parents without having to rebuild the entire hierarchy every time something changes.

    The only thing I could come up with to represent this type of immutible hierarchy was to have the container hold its child items in a binary tree structure.

  • For the immutable question, how about introducing some sort of "view" classes? Items don't actually store upward links to their containers, but a container can create "views" of its children, which are immutable wrappers around the items that also have an upward link back to (a view of?) the container. Of course, it could get nasty making all the view classes.

    For that matter, what should be the return value of an immutable container's add or remove method? Suppose we have a bag inside a box, and we do "bag.Add(torch);", what's the return value? Do we get a bag containing a torch? Do we get a bag that's inside a box, and which contains a torch? Do we get a box containing a bag containing a torch? Are we even allowed to do this?

    How do we call the remove method? If we're copying the hierarchy in order to make new versions, can we sensibly use a reference to identify the item we want to remove, or do we need to give our objects value semantics?

  • Holy goodness, I've been busy. The MVP Summit was fabulous for us; thanks to all who attended and gave

  • In Part Two I asked a couple of follow-up questions, the first of which was: Suppose you were a hostile

  • I wonder if this problem (maintaining bilateral references) can be solved the way you described if you add a third type: containers that aren't items. For example, a room can contain a box and a torch, a box can contain a sack and a a map, but nothing can can contain a room.

    Without multiple inheritance you have to extract the IContainer and IContainable interfaces and implement them in the abstract Item, Container and ContainerItem classes. Okay, you can put the Container class declaration into the Item class declaration, but what if Container is involved in another bilateral relationship?

    Can you user partial class declarations to grant access to specific internal members to a specific type?

  • I would take the easy route and give each item a Bulk property and each container a MaxCapacity property to serve as a validation tool for determining which items fit inside which containers.  Since containers are items, they too follow this "bulk" policy — as such, a room won't fit inside a sack, but it could fit inside a house or a forest, if you were so inclined to use that level of abstraction.  (It also has the lovely side effect of providing an indicator for when a container is "full.")

    You might also consider a component design approach, wherein items have components, and components (rather than the class hierarchy) define the item's functionality.  But I won't go into that.  If you're interested, here's a good link to get you started:  http://www.enchantedage.com/node/45.  The title says XNA, but the concept is very general.

  • I see potential threat of abusing this cross sibling calls to protected methods of the base class, but there's also similar risk of malicious calls from malicious classes that derive from one of the siblings. I mention calls to methods but it also covers accessing other things like properties.

    Here's my example - some software development company structure. Where only people of development background are allowed to give work to developers. Others have to go to the manager to get it vetted.

    public class Employee {}

    public class Developer : Employee()

    {

      protected void virtual Develop(Requirements data) { // develop }

    }

    public class CSharpDeveloper : Developer

    {

      protected  void DoSomeCSharpStuff() {}

      override void Develop(Requirements data) {

          // implement all code changes, remove from 'data' and delegate rest to other devs }

    }

    public class DbDeveloper : Developer

    {

      protected void DoSomeDbStuff() {}

      override void Develop(Requirements data) {

         // implement all db changes, remove from 'data' and delegate rest to other devs }

    }

    public class DevelopmentManager : Developer, IAmManager

    {

      DbDeveloper [] dbDevs;

      CSharpDeveloper[] cDevs;

      override void Develop(Requirements data) { // are you kidding me?! find relevant developer and delegate work }

      IAmManager.RequestWork(Requirements data) { // assess whether relevant to the devs and allocate or bin it }

    }

    public class Tester : Employee {}

    So if sibling calls were allowed then DevelopmentManager would be able to call Develop() on devs in his Team. Developers would be protected from any calls from Testers, they will have to go through proper channels :-)

    Now someone could derive malicious class from Developer class and send malicious calls to Develop() method, which relies on access modifier and doesn't validate input as much.

    But  preventing cross sibling calls doesn't stop malicious class deriving from CSharpDeveloper class and also invoking calls.

    'Internal protected' access modifier doesn't solve the problem because it would limit this model to 1 assembly.

  • I see potential threat of abusing this cross sibling calls to protected methods of the base class, but there's also similar risk of malicious calls from malicious classes that derive from one of the siblings. I mention calls to methods but it also covers accessing other things like properties.

    Here's my example - some software development company structure. Where only people of development background are allowed to give work to developers. Others have to go to the manager to get it vetted.

    public class Employee {}

    public class Developer : Employee()

    {

      protected void virtual Develop(Requirements data) { // develop }

    }

    public class CSharpDeveloper : Developer

    {

      protected  void DoSomeCSharpStuff() {}

      override void Develop(Requirements data) {

          // implement all code changes, remove from 'data' and delegate rest to other devs }

    }

    public class DbDeveloper : Developer

    {

      protected void DoSomeDbStuff() {}

      override void Develop(Requirements data) {

         // implement all db changes, remove from 'data' and delegate rest to other devs }

    }

    public class DevelopmentManager : Developer, IAmManager

    {

      DbDeveloper [] dbDevs;

      CSharpDeveloper[] cDevs;

      override void Develop(Requirements data) { // are you kidding me?! find relevant developer and delegate work }

      IAmManager.RequestWork(Requirements data) { // assess whether relevant to the devs and allocate or bin it }

    }

    public class Tester : Employee {}

    So if sibling calls were allowed then DevelopmentManager would be able to call Develop() on devs in his Team. Developers would be protected from any calls from Testers, they will have to go through proper channels :-)

    Now someone could derive malicious class from Developer class and send malicious calls to Develop() method, which relies on access modifier and doesn't validate input as much.

    But  preventing cross sibling calls doesn't stop malicious class deriving from CSharpDeveloper class and also invoking calls.

    'Internal protected' access modifier doesn't solve the problem because it would limit this model to 1 assembly.

Page 3 of 3 (38 items) 123