Merged HEAD-BUG-FIX (5.1/Cloud) to HEAD (5.1/Cloud)

102238: Merged 5.0.N (5.0.2) to HEAD-BUG-FIX (5.1/Cloud)
      102178: Merged V4.2-BUG-FIX (4.2.5) to 5.0.N (5.0.2)
         102050: Merged DEV to V4.2-BUG-FIX (4.2.5)
            101782: MNT-13751: MNT-12196 may corrupt node property values when auditing pre-call data 
               - Changes provided by customer and unit test.
            102011: MNT-13751: MNT-12196 may corrupt node property values when auditing pre-call data
               - Corrected unit test.


git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@102285 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

source/java/org/alfresco/repo/audit/AuditComponentImpl.java

@@ -25,6 +25,7 @@ import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
+import java.util.ArrayList;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
@@ -491,93 +492,132 @@ public class AuditComponentImpl implements AuditComponent
         return recordAuditValuesWithUserFilter(rootPath, values, true);
     }
     
-    private void trimStringsIfNecessary(Object values)
+    protected <T> T trimStringsIfNecessary (T values)
     {
-        if (values instanceof Map<?, ?>)
+        T processed;
+        
+        if (values instanceof MLText)
         {
-            // trim string audited value
+            // need to treat MLText first because it is actually a HashMap
-            Map<String, Object> map = ((Map<String, Object>) values);
+            Map<Locale, String> localizedStrings = trimStringsIfNecessary((MLText)values);
-            for (Map.Entry<String, Object> entry : map.entrySet())
+            if (localizedStrings != values)
             {
-                Object auditValue = entry.getValue();
+                // processed so far is only defensive copy of a Map, not a MLText
-                // Trim strings
+                processed = (T)new MLText();
-                if (auditValue == null)
+                ((MLText)processed).putAll(localizedStrings);
-                {
-                    // nothing to do
-                }
-                else if (auditValue instanceof String)
-                {
-                    entry.setValue(SchemaBootstrap.trimStringForTextFields((String) auditValue));
-                }
-                else if (auditValue instanceof MLText)
-                {
-                    MLText mltext = (MLText) auditValue;
-                    Set<Locale> locales = mltext.getLocales();
-                    for (Locale locale : locales)
-                    {
-                        mltext.put(locale, SchemaBootstrap.trimStringForTextFields(mltext.getValue(locale)));
-                    }
-                    entry.setValue(mltext);
-                }
-                else if ((auditValue instanceof Map<?, ?>) || (auditValue instanceof Collection<?>))
-                {
-                    trimStringsIfNecessary(auditValue);
-                }
             }
+            else
+            {
+                // no changes
+                processed = values;
+            }
+        }
+        else if (values instanceof Map<?, ?>)
+        {
+            processed = (T)trimStringsIfNecessary((Map<?, ?>)values);
+        }
+        else if (values instanceof List<?>)
+        {
+            // need to treat list specially to preserve order
+            processed = (T)trimStringsIfNecessary((List<?>)values);
         }
         else if (values instanceof Collection<?>)
         {
-            Collection<Object> collection = (Collection<Object>) values;
+            // any other collection treated as unordered with no guarantee processed data will be in same order
-            Iterator<Object> iterator = collection.iterator();
+            processed = (T)trimStringsIfNecessary((Collection<?>)values);
-            Set<String> strings = new HashSet<String>();
+        }
-            while (iterator.hasNext())
+        else if (values instanceof String)
+        {
+            processed = (T)SchemaBootstrap.trimStringForTextFields((String) values);
+        }
+        else 
+        {
+            // don't know how to process
+            processed = values;
+        }
+        
+        return processed;
+    }
+    
+    private <V> List<V> trimStringsIfNecessary (List<V> values)
+    {
+        List<V> processed = values;
+        
+        int idx = 0;
+        for (V auditValue : values)
+        {
+            if (auditValue != null )
             {
-                Object auditValue = iterator.next();
+                V processedAuditValue = trimStringsIfNecessary(auditValue);
-                // Trim strings
+                
-                if (auditValue == null)
+                if (processedAuditValue != auditValue && !auditValue.equals(processedAuditValue))
                 {
-                    // nothing to do
+                    if (processed == values)
-                }
-                else if (auditValue instanceof String)
-                {
-                    String trimmed = SchemaBootstrap.trimStringForTextFields((String) auditValue);
-                    if (!trimmed.equals(auditValue))
                     {
-                        strings.add(trimmed);
+                        // defensive copy
-                        try
+                        processed = new ArrayList<V>(values);
-                        {
-                            iterator.remove();
-                        }
-                        catch (UnsupportedOperationException e)
-                        {
-                            // nothing to do in the case of unmodifiable collection
-                        }
                     }
-                }
+                    processed.set(idx, processedAuditValue);
-                else if (auditValue instanceof MLText)
-                {
-                    MLText mltext = (MLText) auditValue;
-                    Set<Locale> locales = mltext.getLocales();
-                    for (Locale locale : locales)
-                    {
-                        mltext.put(locale, SchemaBootstrap.trimStringForTextFields(mltext.getValue(locale)));
-                    }
-                }
-                else if ((auditValue instanceof Map<?, ?>) || (auditValue instanceof Collection<?>))
-                {
-                    trimStringsIfNecessary(auditValue);
                 }
             }
             
-            try
+            idx++;
+        }
+        
+        return processed;
+    }
+    
+    private <V> Collection<V> trimStringsIfNecessary (Collection<V> values)
+    {
+        Collection<V> processed = values;
+        
+        for (V auditValue : values)
+        {
+            if (auditValue != null )
             {
-                collection.addAll(strings);
+                V processedAuditValue = trimStringsIfNecessary(auditValue);
-            }
+                
-            catch (UnsupportedOperationException e)
+                if (processedAuditValue != auditValue && !auditValue.equals(processedAuditValue))
-            {
+                {
-                // nothing to do in the case of unmodifiable collection
+                    if (processed == values)
+                    {
+                        // defensive copy
+                        processed = new HashSet<V>(values);
+                    }
+                    processed.remove(auditValue);
+                    processed.add(processedAuditValue);
+                }
             }
         }
+        
+        return processed;
+    }
+    
+    private <K, V> Map<K, V> trimStringsIfNecessary (Map<K, V> values)
+    {
+        Map<K, V> processed = values;
+        
+        for (Map.Entry<K, V> entry : values.entrySet())
+        {
+            V auditValue = entry.getValue();
+            
+            if (auditValue != null )
+            {
+                V processedAuditValue = trimStringsIfNecessary(auditValue);
+                
+                if (processedAuditValue != auditValue && !auditValue.equals(processedAuditValue))
+                {
+                    if (processed == values)
+                    {
+                        // defensive copy
+                        processed = new HashMap<K, V>(values);
+                    }
+                    processed.put(entry.getKey(), processedAuditValue);
+                }
+            }
+        }
+        
+        return processed;
     }
     
     @Override
@@ -594,7 +634,7 @@ public class AuditComponentImpl implements AuditComponent
         }
         
         // MNT-12196
-        trimStringsIfNecessary(values);
+        values = trimStringsIfNecessary(values);
         
         // Log inbound values
         if (loggerInbound.isDebugEnabled())

source/test-java/org/alfresco/repo/audit/AuditComponentImplTest.java

@@ -0,0 +1,167 @@
+/*
+ * Copyright (C) 2005-2015 Alfresco Software Limited.
+ *
+ * This file is part of Alfresco
+ *
+ * Alfresco is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Alfresco is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.alfresco.repo.audit;
+
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Locale;
+import java.util.Map;
+
+import org.alfresco.repo.domain.schema.SchemaBootstrap;
+import org.alfresco.service.cmr.repository.MLText;
+
+import junit.framework.TestCase;
+
+public class AuditComponentImplTest extends TestCase
+{
+    public void testTrimStringsIfNecessary()
+    {
+        final int OVERLIMIT_SIZE = 1500;
+        AuditComponentImpl auditComponent = new AuditComponentImpl();
+
+        StringBuilder sb = new StringBuilder();
+        for (int i = 0; i < OVERLIMIT_SIZE; i++)
+        {
+            sb.append("a");
+        }
+
+        // Test map input
+        HashMap<String, Serializable> map = new HashMap<String, Serializable>();
+        String nullValue = null;
+        String oversizeString = sb.toString();
+        MLText mlTextValue = new MLText();
+        mlTextValue.put(Locale.ENGLISH, oversizeString);
+
+        HashMap<String, Serializable> mapEntry = new HashMap<String, Serializable>();
+        MLText mlTextMap = new MLText();
+        mlTextMap.put(Locale.ENGLISH, oversizeString);
+        mapEntry.put("StringMapEntry", oversizeString);
+        mapEntry.put("MLText", mlTextMap);
+
+        ArrayList<Serializable> list = new ArrayList<Serializable>();
+        MLText mlTextList = new MLText();
+        mlTextList.put(Locale.ENGLISH, oversizeString);
+        list.add(oversizeString);
+        list.add(mlTextList);
+
+        ArrayList<Serializable> listEntry = new ArrayList<Serializable>();
+        MLText mlTextListEntry = new MLText();
+        mlTextListEntry.put(Locale.ENGLISH, oversizeString);
+        HashMap<String, Serializable> mapListEntry = new HashMap<String, Serializable>();
+        mapListEntry.put("StringMapListEntry", oversizeString);
+        listEntry.add(nullValue);
+        listEntry.add(oversizeString);
+        listEntry.add(mlTextListEntry);
+        listEntry.add(mapListEntry);
+        listEntry.add(list);
+
+        ArrayList<Serializable> listForUnmd = new ArrayList<Serializable>();
+        listForUnmd.add(oversizeString);
+        Collection<Serializable> unmdCollection = Collections.unmodifiableCollection(listForUnmd);
+
+        map.put("nullValue", nullValue);
+        map.put("StringMap", oversizeString);
+        map.put("MLText", mlTextValue);
+        map.put("mapEntry", mapEntry);
+        map.put("listEntry", listEntry);
+        map.put("unmodifiableCollection", (Serializable) unmdCollection);
+
+        // Test method
+        Map<String, Serializable> processed = auditComponent.trimStringsIfNecessary(map);
+
+        // Check that nothing changed with null
+        assertNull(processed.get("nullValue"));
+
+        // Check StringMap
+        String stringMap = (String) processed.get("StringMap");
+        assertNotSame(stringMap, oversizeString);
+        assertEquals(SchemaBootstrap.DEFAULT_MAX_STRING_LENGTH, stringMap.length());
+
+        // Check MLText
+        MLText mlTextProc = (MLText) processed.get("MLText");
+        assertNotSame(mlTextProc, mlTextValue);
+        String stringMLText = mlTextProc.get(Locale.ENGLISH);
+        assertNotSame(stringMLText, oversizeString);
+        assertEquals(SchemaBootstrap.DEFAULT_MAX_STRING_LENGTH, stringMLText.length());
+
+        // Check mapEntry
+        HashMap<String, Serializable> mapEntryProc = (HashMap<String, Serializable>) processed.get("mapEntry");
+        assertNotSame(mapEntryProc, mapEntry);
+
+        String stringMapEntry = (String) mapEntryProc.get("StringMapEntry");
+        assertNotSame(stringMapEntry, oversizeString);
+        assertEquals(SchemaBootstrap.DEFAULT_MAX_STRING_LENGTH, stringMapEntry.length());
+
+        MLText mlTextMapProc = (MLText) mapEntryProc.get("MLText");
+        assertNotSame(mlTextMapProc, mlTextMap);
+        String stringMLTextMap = mlTextMapProc.get(Locale.ENGLISH);
+        assertNotSame(stringMLTextMap, oversizeString);
+        assertEquals(SchemaBootstrap.DEFAULT_MAX_STRING_LENGTH, stringMLTextMap.length());
+
+        // Check listEntry
+        ArrayList<Serializable> listEntryProc = (ArrayList<Serializable>) processed.get("listEntry");
+        assertNotSame(listEntryProc, listEntry);
+
+        assertNull(listEntryProc.get(0));
+
+        String stringListEntry = (String) listEntryProc.get(1);
+        assertNotSame(stringListEntry, oversizeString);
+        assertEquals(SchemaBootstrap.DEFAULT_MAX_STRING_LENGTH, stringListEntry.length());
+
+        MLText mlTextListEntryProc = (MLText) listEntryProc.get(2);
+        assertNotSame(mlTextListEntryProc, mlTextListEntry);
+        String stringMLTextListEntry = mlTextListEntryProc.get(Locale.ENGLISH);
+        assertNotSame(stringMLTextListEntry, oversizeString);
+        assertEquals(SchemaBootstrap.DEFAULT_MAX_STRING_LENGTH, stringMLTextListEntry.length());
+
+        HashMap<String, Serializable> mapListEntryProc = (HashMap<String, Serializable>) listEntryProc.get(3);
+        assertNotSame(mapListEntryProc, mapListEntry);
+
+        String stringMapListEntry = (String) mapListEntryProc.get("StringMapListEntry");
+        assertNotSame(stringMapListEntry, oversizeString);
+        assertEquals(SchemaBootstrap.DEFAULT_MAX_STRING_LENGTH, stringMapListEntry.length());
+
+        ArrayList<Serializable> listProc = (ArrayList<Serializable>) listEntryProc.get(4);
+        assertNotSame(listProc, list);
+
+        String stringList = (String) listProc.get(0);
+        assertNotSame(stringList, oversizeString);
+        assertEquals(SchemaBootstrap.DEFAULT_MAX_STRING_LENGTH, stringList.length());
+
+        MLText mlTextListProc = (MLText) listProc.get(1);
+        assertNotSame(mlTextListProc, mlTextList);
+        String stringMLTextList = mlTextListProc.get(Locale.ENGLISH);
+        assertEquals(SchemaBootstrap.DEFAULT_MAX_STRING_LENGTH, stringMLTextList.length());
+
+        // Check unmodifiableCollection.
+        Collection<Serializable> unmdCollectionProc = (Collection<Serializable>) processed.get("unmodifiableCollection");
+        assertNotSame(unmdCollectionProc, unmdCollection);
+        Object[] array = unmdCollectionProc.toArray();
+        String stringUNMDCollection = (String) array[0];
+        assertNotSame(stringUNMDCollection, oversizeString);
+        assertEquals(SchemaBootstrap.DEFAULT_MAX_STRING_LENGTH, stringUNMDCollection.length());
+
+        // Check that initial string have not been changed
+        assertEquals(OVERLIMIT_SIZE, oversizeString.length());
+    }
+}