LMS-1767 Escape values before putting them into the cache.

SVN: 18627

openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/server/StringHtmlEscapingPointcutAdvisor.java

@@ -16,9 +16,7 @@
 
 package ch.systemsx.cisd.openbis.generic.client.web.server;
 
-import java.lang.ref.WeakReference;
 import java.lang.reflect.Method;
-import java.util.WeakHashMap;
 
 import org.aopalliance.intercept.MethodInterceptor;
 import org.aopalliance.intercept.MethodInvocation;
@@ -31,6 +29,8 @@ import ch.systemsx.cisd.common.logging.LogCategory;
 import ch.systemsx.cisd.common.logging.LogFactory;
 import ch.systemsx.cisd.common.utilities.ReflectingStringEscaper;
 import ch.systemsx.cisd.openbis.generic.client.web.client.ICommonClientService;
+import ch.systemsx.cisd.openbis.generic.client.web.client.dto.ResultSet;
+import ch.systemsx.cisd.openbis.generic.client.web.client.dto.TypedTableResultSet;
 
 /**
  * The advisor for automatically escaping HTML strings in the values returned by the
@@ -80,13 +80,31 @@ public class StringHtmlEscapingPointcutAdvisor extends DefaultPointcutAdvisor
                 return false;
             }
 
-            if (method.getName().equals("getApplicationInfo"))
+            if (method.getReturnType() == Void.TYPE)
             {
-                escapeLog.info("Asked to handle getApplicationInfo");
                 return false;
             }
 
-            if (method.getReturnType() == Void.TYPE)
+            // This is handled in the cache manager
+            if (method.getReturnType() == TypedTableResultSet.class)
+            {
+                return false;
+            }
+
+            // This is handled in the cache manager
+            if (method.getReturnType() == ResultSet.class)
+            {
+                return false;
+            }
+
+            // Don't need to escape this method
+            if ("getLastModificationState".equals(method.getName()))
+            {
+                return false;
+            }
+
+            // This is handled in the cache manager
+            if (method.getReturnType() == ch.systemsx.cisd.openbis.generic.client.web.client.dto.ResultSetWithEntityTypes.class)
             {
                 return false;
             }
@@ -103,9 +121,6 @@ public class StringHtmlEscapingPointcutAdvisor extends DefaultPointcutAdvisor
     public static class StringHtmlEscapingPointcutAdvisorMethodInterceptor implements
             MethodInterceptor
     {
-        private final WeakHashMap<Object, WeakReference<Object>> alreadyEscapedObjects =
-                new WeakHashMap<Object, WeakReference<Object>>();
-
         /**
          * Get the session token and any guarded parameters and invoke the guards on those
          * parameters.
@@ -114,32 +129,34 @@ public class StringHtmlEscapingPointcutAdvisor extends DefaultPointcutAdvisor
         {
             Object result = methodInvocation.proceed();
 
-            // check if the object has already been escaped
-            synchronized (this)
-            {
-                WeakReference<Object> alreadyEscaped = alreadyEscapedObjects.get(result);
-                if (alreadyEscaped != null)
-                {
-                    escapeObject(methodInvocation, result);
-                    alreadyEscapedObjects.put(result, new WeakReference<Object>(result));
-                }
-            }
+            result = escapeObject(methodInvocation, result);
 
             return result;
         }
 
-        private void escapeObject(MethodInvocation methodInvocation, Object result)
+        private Object escapeObject(MethodInvocation methodInvocation, Object unescapedResult)
         {
-            escapeLog.info(methodInvocation.getMethod().getName() + " converting   " + result);
-            if (result instanceof String)
+            Object result = unescapedResult;
+            escapeLog.info(methodInvocation.getMethod().getName() + " converting   "
+                    + unescapedResult);
+            if (unescapedResult instanceof String)
             {
-                StringEscapeUtils.escapeHtml((String) result);
+                if ("getExportTable".equals(methodInvocation.getMethod().getName()))
+                {
+                    result = StringEscapeUtils.unescapeHtml((String) unescapedResult);
+                } else
+                {
+                    // Do we need to escape strings in general?
+                    // StringEscapeUtils.escapeHtml((String) unescapedResult);
+                }
             } else
             {
                 // Escape the result objects
-                ReflectingStringEscaper.escapeDeep(result);
+                ReflectingStringEscaper.escapeDeep(unescapedResult);
             }
-            escapeLog.info(methodInvocation.getMethod().getName() + " converted to " + result);
+            escapeLog.info(methodInvocation.getMethod().getName() + " converted to "
+                    + unescapedResult);
+            return result;
         }
     }
 }

openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/server/resultset/CachedResultSetManager.java

@@ -35,6 +35,7 @@ import ch.rinn.restrictions.Private;
 import ch.systemsx.cisd.common.logging.LogCategory;
 import ch.systemsx.cisd.common.logging.LogFactory;
 import ch.systemsx.cisd.common.shared.basic.AlternativesStringFilter;
+import ch.systemsx.cisd.common.utilities.ReflectingStringEscaper;
 import ch.systemsx.cisd.openbis.generic.client.web.client.dto.ColumnDistinctValues;
 import ch.systemsx.cisd.openbis.generic.client.web.client.dto.CustomFilterInfo;
 import ch.systemsx.cisd.openbis.generic.client.web.client.dto.GridColumnFilterInfo;
@@ -70,16 +71,17 @@ public final class CachedResultSetManager<K> implements IResultSetManager<K>, Se
     @Private
     static final int MAX_DISTINCT_COLUMN_VALUES_SIZE = 50;
 
-    private static final Logger operationLog =
-            LogFactory.getLogger(LogCategory.OPERATION, CachedResultSetManager.class);
+    private static final Logger operationLog = LogFactory.getLogger(LogCategory.OPERATION,
+            CachedResultSetManager.class);
 
     private static final GridCustomColumnInfo translate(GridCustomColumn columnDefinition)
     {
         return new GridCustomColumnInfo(columnDefinition.getCode(), columnDefinition.getName(),
                 columnDefinition.getDescription(), columnDefinition.getDataType());
     }
-    
-    private static <T> String getOriginalValue(IColumnDefinition<T> definition, final GridRowModel<T> row)
+
+    private static <T> String getOriginalValue(IColumnDefinition<T> definition,
+            final GridRowModel<T> row)
     {
         Comparable<?> value = definition.tryGetComparableValue(row);
         return value == null ? "" : value.toString();
@@ -88,16 +90,17 @@ public final class CachedResultSetManager<K> implements IResultSetManager<K>, Se
     static interface IColumnCalculator
     {
         /**
-         * Calculates the values of the specified custom column definition by using specified data and
-         * specified column definitions. In case of an error the column value is an error message.
+         * Calculates the values of the specified custom column definition by using specified data
+         * and specified column definitions. In case of an error the column value is an error
+         * message.
          * 
          * @param errorMessagesAreLong if <code>true</code> a long error message will be created.
          */
         public <T> List<PrimitiveValue> evalCustomColumn(List<T> data,
                 GridCustomColumn customColumn, Set<IColumnDefinition<T>> availableColumns,
-                boolean errorMessagesAreLong);   
+                boolean errorMessagesAreLong);
     }
-    
+
     private static final class Column
     {
         private final GridCustomColumn columnDefinition;
@@ -139,13 +142,14 @@ public final class CachedResultSetManager<K> implements IResultSetManager<K>, Se
         private final IColumnCalculator columnCalculator;
 
         private final List<TableModelColumnHeader> headers;
-        
-        private final List<IColumnDefinition<T>> headerColumnDefinitions = new ArrayList<IColumnDefinition<T>>();
-        
+
+        private final List<IColumnDefinition<T>> headerColumnDefinitions =
+                new ArrayList<IColumnDefinition<T>>();
+
         private Map<String, Column> calculatedColumns = new HashMap<String, Column>();
 
-        TableData(List<T> originalData, List<TableModelColumnHeader> headers, ICustomColumnsProvider customColumnsProvider,
-                IColumnCalculator columnCalculator)
+        TableData(List<T> originalData, List<TableModelColumnHeader> headers,
+                ICustomColumnsProvider customColumnsProvider, IColumnCalculator columnCalculator)
         {
             this.originalData = originalData;
             this.headers = headers;
@@ -422,7 +426,7 @@ public final class CachedResultSetManager<K> implements IResultSetManager<K>, Se
 
     // all cache access should be doen in a monitor (synchronized clause)
     private final Map<K, TableData<?>> cache = new HashMap<K, TableData<?>>();
-    
+
     private final XMLPropertyTransformer xmlPropertyTransformer = new XMLPropertyTransformer();
 
     private final IColumnCalculator columnCalculator;
@@ -512,7 +516,8 @@ public final class CachedResultSetManager<K> implements IResultSetManager<K>, Se
             ITableDataProvider dataProvider =
                     TableDataProviderFactory.createDataProvider(rows,
                             new ArrayList<IColumnDefinition<T>>(availableColumns));
-            List<Integer> indices = GridExpressionUtils.applyCustomFilter(dataProvider, customFilterInfo);
+            List<Integer> indices =
+                    GridExpressionUtils.applyCustomFilter(dataProvider, customFilterInfo);
             filteredRows = new ArrayList<GridRowModel<T>>();
             for (Integer index : indices)
             {
@@ -630,6 +635,19 @@ public final class CachedResultSetManager<K> implements IResultSetManager<K>, Se
 
     public final <T> IResultSet<K, T> getResultSet(final String sessionToken,
             final IResultSetConfig<K, T> resultConfig, final IOriginalDataProvider<T> dataProvider)
+    {
+        // Get the raw result set
+        IResultSet<K, T> cachedResultSet =
+                getRawResultSet(sessionToken, resultConfig, dataProvider);
+
+        return cachedResultSet;
+    }
+
+    /**
+     * Raw handling of retrieving the result set from the cache -- no HTML escaping is applied.
+     */
+    private final <T> IResultSet<K, T> getRawResultSet(final String sessionToken,
+            final IResultSetConfig<K, T> resultConfig, final IOriginalDataProvider<T> dataProvider)
     {
         assert resultConfig != null : "Unspecified result configuration";
         assert dataProvider != null : "Unspecified data retriever";
@@ -665,11 +683,19 @@ public final class CachedResultSetManager<K> implements IResultSetManager<K>, Se
         List<T> rows = dataProvider.getOriginalData();
         List<TableModelColumnHeader> headers = dataProvider.getHeaders();
         xmlPropertyTransformer.transformXMLProperties(rows);
-        TableData<T> tableData = new TableData<T>(rows, headers, customColumnsProvider, columnCalculator);
+        TableData<T> tableData =
+                new TableData<T>(rows, headers, customColumnsProvider, columnCalculator);
+
+        // Escape the values
+        for (T row : rows)
+        {
+            ReflectingStringEscaper.escapeDeep(row);
+        }
+
         addToCache(dataKey, tableData);
         return calculateSortAndFilterResult(sessionToken, tableData, resultConfig, dataKey);
     }
-    
+
     private synchronized <T> void addToCache(K dataKey, TableData<T> tableData)
     {
         cache.put(dataKey, tableData);