Metadata patching in Shell modifies IContent instances in cache

Found in

Release 7

Fixed in

EPiServer.CMS.UI.Core 7.8.0

Created

Mar 24 2014

Updated

Apr 30 2014

Area

Falcon/CMS/Edit UI/Editing

State

Closed, Fixed and Tested


Steps to reproduce

In class ContentMetaDataPropertiesInjector there is a method 

public virtual PropertyDataCollection Inject(IContentData content)

this method will in some cases modify the passed in content instance (which is probably readonly and in cache which makes the modifications visible for others as well).
The code that causes the problem is 

var properties = new PropertyDataCollection();
....
          //Add standard properties
            PageData pageData = content as PageData;
            foreach (string propertyName in content.Property.Keys)
            {
                var property = content.Property.Get(propertyName);
                if (pageData == null || !MetaDataProperties.IsHandledByInterface(property))
                {
                    properties.Add(property);
                }
            }

It is not obvious but Add method for property collection modifies the inparameter (bad design...) like:
 public void Add(string name, PropertyData value)
        {
            InternalApproveObject(name, value);
            if (BaseGet(name) != null)
            {
                throw new ArgumentException("PropertyData object with name \"" + name + "\" already exists", "name");
            }
            value.Parent = this;
            BaseAdd(name, value);
        }

So the value Parent on the passed in property (that is rooted in the readonly content in cache) will be changed and pointing to the unrooted PropertyCollection created in the above method.

I have reported a bug on Cms.Core that Parent should throw if it is readonly (see related). However fixing that will break earlier version of CMS.UI which has above code.
The UI code should be changed to e.g. 

properties.Add(property.CreateWritableClone());