Merge pull request #1598 from antony-liu/poi/v3.17-patch14

Bug 61431 - Conditional formatting evaluation ignores undefined cells

Author: tonyqus

main/SS/Formula/ConditionalFormattingEvaluator.cs

@@ -2,16 +2,53 @@
 using NPOI.SS.Util;
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using System.Text;
 
 namespace NPOI.SS.Formula
 {
+    /// <summary>
+    /// <para>
+    /// Evaluates Conditional Formatting constraints.
+    /// </para>
+    /// <para>
+    /// For performance reasons, this class keeps a cache of all previously evaluated rules and cells.
+    /// Be sure to call <see cref="clearAllCachedFormats()" /> if any conditional formats are modified, added, or deleted,
+    /// and <see cref="clearAllCachedValues()" /> whenever cell values change.
+    /// </para>
+    /// </summary>
     public class ConditionalFormattingEvaluator
     {
         private WorkbookEvaluator workbookEvaluator;
         private IWorkbook workbook;
-
+        /// <summary>
+        /// <para>
+        /// All the underlying structures, for both HSSF and XSSF, repeatedly go to the raw bytes/XML for the
+        /// different pieces used in the ConditionalFormatting* structures.  That's highly inefficient,
+        /// and can cause significant lag when checking formats for large workbooks.
+        /// </para>
+        /// <para>
+        /// Instead we need a cached version that is discarded when definitions change.
+        /// </para>
+        /// <para>
+        /// Sheets don't implement equals, and since its an interface,
+        /// there's no guarantee instances won't be recreated on the fly by some implementation.
+        /// So we use sheet name.
+        /// </para>
+        /// </summary>
         private Dictionary<String, List<EvaluationConditionalFormatRule>> formats = new Dictionary<string, List<EvaluationConditionalFormatRule>>();
+        /// <summary>
+    /// <para>
+    /// Evaluating rules for cells in their region(s) is expensive, so we want to cache them,
+    /// and empty/reevaluate the cache when values change.
+    /// </para>
+    /// <para>
+    /// Rule lists are in priority order, as evaluated by Excel (smallest priority # for XSSF, definition order for HSSF)
+    /// </para>
+    /// <para>
+    /// CellReference : equals().
+    /// </para>
+    /// </summary>
         private SortedDictionary<CellReference, List<EvaluationConditionalFormatRule>> values = new SortedDictionary<CellReference, List<EvaluationConditionalFormatRule>>();
 
         public ConditionalFormattingEvaluator(IWorkbook wb, IWorkbookEvaluatorProvider provider)
@@ -26,6 +63,28 @@ protected WorkbookEvaluator WorkbookEvaluator
                 return workbookEvaluator;
             }
         }
+
+        /// <summary>
+        /// <para>
+        /// This checks all applicable <see cref="ConditionalFormattingRule"/>s for the cell's sheet,
+        /// in defined "priority" order, returning the matches if any.  This is a property currently
+        /// not exposed from <c>CTCfRule</c> in <c>XSSFConditionalFormattingRule</c>.
+        /// </para>
+        /// <para>
+        /// Most cells will have zero or one applied rule, but it is possible to define multiple rules
+        /// that apply at the same time to the same cell, thus the List result.
+        /// </para>
+        /// <para>
+        /// Note that to properly apply conditional rules, care must be taken to offset the base
+        /// formula by the relative position of the current cell, or the wrong value is checked.
+        /// This is handled by <see cref="WorkbookEvaluator.Evaluate(String, CellReference, CellRangeAddressBase)" />.
+        /// </para>
+        /// </summary>
+        /// <param name="cellRef">NOTE: if no sheet name is specified, this uses the workbook active sheet</param>
+        /// <returns>Unmodifiable List of <see cref="EvaluationConditionalFormatRule"/>s that apply to the current cell value,
+        /// in priority order, as evaluated by Excel (smallest priority # for XSSF, definition order for HSSF),
+        /// or null if none apply
+        /// </returns>
         public List<EvaluationConditionalFormatRule> GetConditionalFormattingForCell(CellReference cellRef)
         {
             List<EvaluationConditionalFormatRule> rules = values.TryGetValue(cellRef, out List<EvaluationConditionalFormatRule> value) ? value : null;
@@ -71,6 +130,28 @@ public List<EvaluationConditionalFormatRule> GetConditionalFormattingForCell(Cel
 
             return rules;
         }
+
+        /// <summary>
+        /// <para>
+        /// This checks all applicable <see cref="ConditionalFormattingRule"/>s for the cell's sheet,
+        /// in defined "priority" order, returning the matches if any.  This is a property currently
+        /// not exposed from <c>CTCfRule</c> in <c>XSSFConditionalFormattingRule</c>.
+        /// </para>
+        /// <para>
+        /// Most cells will have zero or one applied rule, but it is possible to define multiple rules
+        /// that apply at the same time to the same cell, thus the List result.
+        /// </para>
+        /// <para>
+        /// Note that to properly apply conditional rules, care must be taken to offset the base
+        /// formula by the relative position of the current cell, or the wrong value is checked.
+        /// This is handled by <see cref="WorkbookEvaluator.Evaluate(String, CellReference, CellRangeAddressBase)" />.
+        /// </para>
+        /// </summary>
+        /// <param name="cell"></param>
+        /// <returns>Unmodifiable List of <see cref="EvaluationConditionalFormatRule"/>s that apply to the current cell value,
+        /// in priority order, as evaluated by Excel (smallest priority # for XSSF, definition order for HSSF),
+        /// or null if none apply
+        /// </returns>
         public List<EvaluationConditionalFormatRule> GetConditionalFormattingForCell(ICell cell)
         {
             return GetConditionalFormattingForCell(GetRef(cell));
@@ -80,6 +161,11 @@ public static CellReference GetRef(ICell cell)
             return new CellReference(cell.Sheet.SheetName, cell.RowIndex, cell.ColumnIndex, false, false);
         }
 
+        /// <summary>
+        /// lazy load by sheet since reading can be expensive
+        /// </summary>
+        /// <param name="sheet"></param>
+        /// <returns>unmodifiable list of rules</returns>
         protected List<EvaluationConditionalFormatRule> GetRules(ISheet sheet)
         {
             String sheetName = sheet.SheetName;
@@ -110,13 +196,95 @@ protected List<EvaluationConditionalFormatRule> GetRules(ISheet sheet)
             }
             return rules;
         }
+        /// <summary>
+        /// Call this whenever rules are added, reordered, or removed, or a rule formula is changed
+        /// (not the formula inputs but the formula expression itself)
+        /// </summary>
         public void ClearAllCachedFormats()
         {
             formats.Clear();
         }
+        /// <summary>
+        /// <para>
+        /// Call this whenever cell values change in the workbook, so condional formats are re-evaluated
+        /// for all cells.
+        /// </para>
+        /// <para>
+        /// TODO: eventually this should work like <see cref="EvaluationCache.notifyUpdateCell(int, int, EvaluationCell)" />
+        /// and only clear values that need recalculation based on the formula dependency tree.
+        /// </para>
+        /// </summary>
         public void ClearAllCachedValues()
         {
             values.Clear();
         }
+
+        /// <summary>
+        /// </summary>
+        /// <param name="sheetName"></param>
+        /// <returns>unmodifiable list of all Conditional format rules for the given sheet, if any</returns>
+        public List<EvaluationConditionalFormatRule> GetFormatRulesForSheet(string sheetName) {
+            return GetFormatRulesForSheet(workbook.GetSheet(sheetName));
+        }
+    
+        /// <summary>
+        /// </summary>
+        /// <param name="sheet"></param>
+        /// <returns>unmodifiable list of all Conditional format rules for the given sheet, if any</returns>
+        public List<EvaluationConditionalFormatRule> GetFormatRulesForSheet(ISheet sheet) {
+            return GetRules(sheet);
+        }
+    
+        /// <summary>
+        /// <para>
+        /// Conditional formatting rules can apply only to cells in the sheet to which they are attached.
+        /// The POI data model does not have a back-reference to the owning sheet, so it must be passed in separately.
+        /// </para>
+        /// <para>
+        /// We could overload this with convenience methods taking a sheet name and sheet index as well.
+        /// </para>
+        /// </summary>
+        /// <param name="sheet">containing the rule</param>
+        /// <param name="conditionalFormattingIndex">of the <see cref="ConditionalFormatting"/> instance in the sheet's array</param>
+        /// <param name="ruleIndex">of the <see cref="ConditionalFormattingRule"/> instance within the <see cref="ConditionalFormatting"/></param>
+        /// <returns>unmodifiable List of all cells in the rule's region matching the rule's condition</returns>
+        public List<ICell> GetMatchingCells(ISheet sheet, int conditionalFormattingIndex, int ruleIndex) {
+            foreach (EvaluationConditionalFormatRule rule in GetRules(sheet)) {
+                if (rule.Sheet.Equals(sheet) && rule.FormattingIndex == conditionalFormattingIndex && rule.RuleIndex == ruleIndex) {
+                    return GetMatchingCells(rule);
+                }
+            }
+            return [];
+        }
+    
+        /// <summary>
+        /// </summary>
+        /// <param name="rule"></param>
+        /// <returns>unmodifiable List of all cells in the rule's region matching the rule's condition</returns>
+        public List<ICell> GetMatchingCells(EvaluationConditionalFormatRule rule) {
+            List<ICell> cells = new List<ICell>();
+            ISheet sheet = rule.Sheet;
+        
+            foreach (CellRangeAddress region in rule.Regions) {
+                for (int r = region.FirstRow; r <= region.LastRow; r++) {
+                    IRow row = sheet.GetRow(r);
+                    if (row == null) {
+                        continue; // no cells to check
+                    }
+                    for (int c = region.FirstColumn; c <= region.LastColumn; c++) {
+                        ICell cell = row.GetCell(c);
+                        if (cell == null) {
+                            continue;
+                        }
+                    
+                        List<EvaluationConditionalFormatRule> cellRules = GetConditionalFormattingForCell(cell);
+                        if (cellRules.Contains(rule)) {
+                            cells.Add(cell);
+                        }
+                    }
+                }
+            }
+            return cells;
+        }
     }
 }

main/SS/Formula/EvaluationConditionalFormatRule.cs

@@ -562,7 +562,10 @@ private bool CheckFilter(ICell cell, CellReference reference, CellRangeAddress r
                 case ConditionFilterType.TOP_10:
                     // from testing, Excel only operates on numbers and dates (which are stored as numbers) in the range.
                     // numbers stored as text are ignored, but numbers formatted as text are treated as numbers.
-
+                    if (! cv.IsNumber)
+                    {
+                        return false;
+                    }
                     return GetMeaningfulValues(region, true,  (allValues)=> {
                         IConditionFilterData fc = rule.FilterConfiguration;
 
@@ -696,16 +699,20 @@ private bool CheckFilter(ICell cell, CellReference reference, CellRangeAddress r
                     return OperatorEnumHelper.IsValid(op, val, comp, null);
                 case ConditionFilterType.CONTAINS_TEXT:
                     // implemented both by a cfRule "text" attribute and a formula.  Use the text.
-                    return text == null ? false : cv.ToString().ToLowerInvariant().Contains(lowerText);
+                    //return text == null ? false : cv.ToString().ToLowerInvariant().Contains(lowerText);
+                    return CheckFormula(reference, region);
                 case ConditionFilterType.NOT_CONTAINS_TEXT:
                     // implemented both by a cfRule "text" attribute and a formula.  Use the text.
-                    return text == null ? true : !cv.ToString().ToLowerInvariant().Contains(lowerText);
+                    //return text == null ? true : !cv.ToString().ToLowerInvariant().Contains(lowerText);
+                    return CheckFormula(reference, region);
                 case ConditionFilterType.BEGINS_WITH:
                     // implemented both by a cfRule "text" attribute and a formula.  Use the text.
-                    return cv.ToString().ToLowerInvariant().StartsWith(lowerText);
+                    //return cv.ToString().ToLowerInvariant().StartsWith(lowerText);
+                    return CheckFormula(reference, region);
                 case ConditionFilterType.ENDS_WITH:
                     // implemented both by a cfRule "text" attribute and a formula.  Use the text.
-                    return cv.ToString().ToLowerInvariant().EndsWith(lowerText);
+                    //return cv.ToString().ToLowerInvariant().EndsWith(lowerText);
+                    return CheckFormula(reference, region);
                 case ConditionFilterType.CONTAINS_BLANKS:
                     try
                     {

main/SS/Formula/WorkbookEvaluator.cs

@@ -327,17 +327,40 @@ private ValueEval Evaluate(String formula, CellReference target, CellRangeAddres
                 formulaType.GetAttributes().Get<SingleValueAttribute>().IsSingleValue);
             return EvaluateNameFormula(ptgs, ec);
         }
+        /// <summary>
+        /// Adjust formula relative references by the offset between the start of the given region and the given target cell.
+        /// </summary>
+        /// <param name="ptgs"></param>
+        /// <param name="target">cell within the region to use.</param>
+        /// <param name="region">containing the cell</param>
+        /// <returns>true if any Ptg references were shifted</returns>
+        /// <exception cref="IndexOutOfRangeException">if the resulting shifted row/column indexes are over the document format limits</exception>
+        /// <exception cref="ArgumentException">if target is not within region.</exception>
         protected bool AdjustRegionRelativeReference(Ptg[] ptgs, CellReference target, CellRangeAddressBase region)
         {
             if (!region.IsInRange(target))
             {
                 throw new ArgumentException(target + " is not within " + region);
             }
 
-            //return adjustRegionRelativeReference(ptgs, target.getRow() - region.getFirstRow(), target.getCol() - region.getFirstColumn());
-
-            int deltaRow = target.Row;
-            int deltaColumn = target.Col;
+            return AdjustRegionRelativeReference(ptgs, target.Row - region.FirstRow, target.Col - region.FirstColumn);
+        }
+    
+        /// <summary>
+        /// Adjust the formula relative cell references by a given delta
+        /// </summary>
+        /// <param name="ptgs"></param>
+        /// <param name="deltaRow">target row offset from the top left cell of a region</param>
+        /// <param name="deltaColumn">target column offset from the top left cell of a region</param>
+        /// <returns>true if any Ptg references were shifted</returns>
+        /// <exception cref="IndexOutOfRangeException">if the resulting shifted row/column indexes are over the document format limits</exception>
+        /// <exception cref="ArgumentException">if either of the deltas are negative, as the assumption is we are shifting formulas
+        /// relative to the top left cell of a region.
+        /// </exception>
+        protected bool AdjustRegionRelativeReference(Ptg[] ptgs, int deltaRow, int deltaColumn) 
+        {
+            if (deltaRow < 0) throw new ArgumentException("offset row must be positive");
+            if (deltaColumn < 0) throw new ArgumentException("offset column must be positive");
 
             bool shifted = false;
             foreach(Ptg ptg in ptgs)

ooxml/POIXMLTextExtractor.cs

@@ -18,7 +18,9 @@ limitations under the License.
 namespace NPOI
 {
     using System;
+    using System.Text;
     using NPOI.OpenXml4Net.OPC;
+    using NPOI.OpenXml4Net.Util;
 
     public abstract class POIXMLTextExtractor : POITextExtractor
     {
@@ -102,6 +104,23 @@ public override void Close()
             }
             base.Close();
         }
+
+        protected void CheckMaxTextSize(StringBuilder text, String string1)
+        {
+            if(string1 == null)
+            {
+                return;
+            }
+
+            int size = text.Length + string1.Length;
+            if(size > ZipSecureFile.GetMaxTextSize())
+            {
+                throw new InvalidOperationException("The text would exceed the max allowed overall size of extracted text. "
+                        + "By default this is prevented as some documents may exhaust available memory and it may indicate that the file is used to inflate memory usage and thus could pose a security risk. "
+                        + "You can adjust this limit via ZipSecureFile.setMaxTextSize() if you need to work with files which have a lot of text. "
+                        + "Size: " + size + ", limit: MAX_TEXT_SIZE: " + ZipSecureFile.GetMaxTextSize());
+            }
+        }
     }
 
 }