From 8225326b568a9ddf79978c63c7a10dbeae1626b2 Mon Sep 17 00:00:00 2001
From: Greg Woolsey <gwoolsey@apache.org>
Date: Sat, 16 Mar 2019 00:27:13 +0000
Subject: [PATCH] #63264 Conditional Format rule evaluation calculates relative
 references incorrectly

Fixing the offset calculation and passing the top-left-most region rather than the region matching the current cell fixed these cases.  I've augmented unit tests to check for them to avoid future regressions.

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1855619 13f79535-47bb-0310-9956-ffa450edef68
---
 .../EvaluationConditionalFormatRule.java      | 17 +++++++++++---
 .../poi/ss/formula/WorkbookEvaluator.java     | 16 +++++--------
 .../ConditionalFormattingEvalTest.java        | 23 +++++++++++++++++++
 3 files changed, 43 insertions(+), 13 deletions(-)

diff --git a/src/java/org/apache/poi/ss/formula/EvaluationConditionalFormatRule.java b/src/java/org/apache/poi/ss/formula/EvaluationConditionalFormatRule.java
index 3ca5e71d76..a487b36ec4 100644
--- a/src/java/org/apache/poi/ss/formula/EvaluationConditionalFormatRule.java
+++ b/src/java/org/apache/poi/ss/formula/EvaluationConditionalFormatRule.java
@@ -69,6 +69,9 @@ public class EvaluationConditionalFormatRule implements Comparable<EvaluationCon
     
     /* cached values */
     private final CellRangeAddress[] regions;
+    
+    private CellRangeAddress topLeftRegion;
+    
     /**
      * Depending on the rule type, it may want to know about certain values in the region when evaluating {@link #matches(CellReference)},
      * such as top 10, unique, duplicate, average, etc.  This collection stores those if needed so they are not repeatedly calculated
@@ -114,6 +117,14 @@ public class EvaluationConditionalFormatRule implements Comparable<EvaluationCon
         this.priority = rule.getPriority();
         
         this.regions = regions;
+        
+        for (CellRangeAddress region : regions) {
+            if (topLeftRegion == null) topLeftRegion = region;
+            else if (region.getFirstColumn() < topLeftRegion.getFirstColumn()
+                    || region.getFirstRow() < topLeftRegion.getFirstRow()) {
+                topLeftRegion = region;
+            }
+        }
         formula1 = rule.getFormula1();
         formula2 = rule.getFormula2();
         
@@ -316,13 +327,13 @@ public class EvaluationConditionalFormatRule implements Comparable<EvaluationCon
         if (ruleType.equals(ConditionType.CELL_VALUE_IS)) {
             // undefined cells never match a VALUE_IS condition
             if (cell == null) return false;
-            return checkValue(cell, region);
+            return checkValue(cell, topLeftRegion);
         }
         if (ruleType.equals(ConditionType.FORMULA)) {
-            return checkFormula(ref, region);
+            return checkFormula(ref, topLeftRegion);
         }
         if (ruleType.equals(ConditionType.FILTER)) {
-            return checkFilter(cell, ref, region);
+            return checkFilter(cell, ref, topLeftRegion);
         }
         
         // TODO: anything else, we don't handle yet, such as top 10
diff --git a/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java b/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java
index 7fed473011..2116456f34 100644
--- a/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java
+++ b/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java
@@ -880,20 +880,16 @@ public final class WorkbookEvaluator {
      * <p><pre>formula ref + current cell ref</pre></p>
      * @param ptgs
      * @param target cell within the region to use.
-     * @param region containing the cell
+     * @param region containing the cell, OR, for conditional format rules with multiple ranges, the region with the top-left-most cell
      * @return true if any Ptg references were shifted
      * @throws IndexOutOfBoundsException if the resulting shifted row/column indexes are over the document format limits
      * @throws IllegalArgumentException if target is not within region.
      */
     protected boolean adjustRegionRelativeReference(Ptg[] ptgs, CellReference target, CellRangeAddressBase region) {
-        if (! region.isInRange(target)) {
-            throw new IllegalArgumentException(target + " is not within " + region);
-        }
+        // region may not be the one that contains the target, if a conditional formatting rule applies to multiple regions
         
-        //return adjustRegionRelativeReference(ptgs, target.getRow() - region.getFirstRow(), target.getCol() - region.getFirstColumn());
-        
-        int deltaRow = target.getRow();
-        int deltaColumn = target.getCol();
+        int deltaRow = target.getRow() - region.getFirstRow();
+        int deltaColumn = target.getCol() - region.getFirstColumn();
         
         boolean shifted = false;
         for (Ptg ptg : ptgs) {
@@ -902,7 +898,7 @@ public final class WorkbookEvaluator {
                 RefPtgBase ref = (RefPtgBase) ptg;
                 // re-calculate cell references
                 final SpreadsheetVersion version = _workbook.getSpreadsheetVersion();
-                if (ref.isRowRelative()) {
+                if (ref.isRowRelative() && deltaRow > 0) {
                     final int rowIndex = ref.getRow() + deltaRow;
                     if (rowIndex > version.getMaxRows()) {
                         throw new IndexOutOfBoundsException(version.name() + " files can only have " + version.getMaxRows() + " rows, but row " + rowIndex + " was requested.");
@@ -910,7 +906,7 @@ public final class WorkbookEvaluator {
                     ref.setRow(rowIndex);
                     shifted = true;
                 }
-                if (ref.isColRelative()) {
+                if (ref.isColRelative() && deltaColumn > 0) {
                     final int colIndex = ref.getColumn() + deltaColumn;
                     if (colIndex > version.getMaxColumns()) {
                         throw new IndexOutOfBoundsException(version.name() + " files can only have " + version.getMaxColumns() + " columns, but column " + colIndex + " was requested.");
diff --git a/src/ooxml/testcases/org/apache/poi/ss/usermodel/ConditionalFormattingEvalTest.java b/src/ooxml/testcases/org/apache/poi/ss/usermodel/ConditionalFormattingEvalTest.java
index 3b1155fe73..9d83635215 100644
--- a/src/ooxml/testcases/org/apache/poi/ss/usermodel/ConditionalFormattingEvalTest.java
+++ b/src/ooxml/testcases/org/apache/poi/ss/usermodel/ConditionalFormattingEvalTest.java
@@ -19,6 +19,7 @@ package org.apache.poi.ss.usermodel;
 
 
 import java.io.IOException;
+import java.util.Date;
 import java.util.List;
 
 import org.apache.poi.ss.formula.ConditionalFormattingEvaluator;
@@ -117,6 +118,28 @@ public class ConditionalFormattingEvalTest {
         getRulesFor(8,2);
         assertEquals("wrong # of rules for " + ref, 1, rules.size());
         
+        sheet = wb.getSheet("Compare to totals");
+        getRulesFor(3, 2);
+        assertEquals("wrong # of rules for " + ref, 1, rules.size());
+        assertEquals("wrong fg color for " + ref, "FFFF0000", getColor(rules.get(0).getRule().getFontFormatting().getFontColor()));
+        getRulesFor(3, 3);
+        assertEquals("wrong # of rules for " + ref, 0, rules.size());
+        getRulesFor(15, 4);
+        assertEquals("wrong # of rules for " + ref, 0, rules.size());
+        getRulesFor(16, 1);
+        assertEquals("wrong # of rules for " + ref, 1, rules.size());
+        assertEquals("wrong fg color for " + ref, "FFFF0000", getColor(rules.get(0).getRule().getFontFormatting().getFontColor()));
+        
+        sheet = wb.getSheet("Products3");
+        sheet.getRow(8).getCell(0).setCellValue(new Date());
+        getRulesFor(8, 0);
+        assertEquals("wrong # of rules for " + ref, 1, rules.size());
+        getRulesFor(8, 3);
+        assertEquals("wrong # of rules for " + ref, 1, rules.size());
+
+        sheet = wb.getSheet("Customers2");
+        getRulesFor(3, 0);
+        assertEquals("wrong # of rules for " + ref, 0, rules.size());
     }
 
     @Test