Fix bug #56450 - Avoid a NPE if a comment has no associated NoteRecord (but we still don't know where it belongs)

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1589770 13f79535-47bb-0310-9956-ffa450edef68

4 files changed, 52 insertions, 4 deletions

diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFComment.java b/src/java/org/apache/poi/hssf/usermodel/HSSFComment.java
index 4e37e41648..8b4350893c 100644
--- a/src/java/org/apache/poi/hssf/usermodel/HSSFComment.java
+++ b/src/java/org/apache/poi/hssf/usermodel/HSSFComment.java
@@ -16,8 +16,18 @@
 ==================================================================== */
 package org.apache.poi.hssf.usermodel;
 
-import org.apache.poi.ddf.*;
-import org.apache.poi.hssf.record.*;
+import org.apache.poi.ddf.DefaultEscherRecordFactory;
+import org.apache.poi.ddf.EscherBSERecord;
+import org.apache.poi.ddf.EscherContainerRecord;
+import org.apache.poi.ddf.EscherOptRecord;
+import org.apache.poi.ddf.EscherProperties;
+import org.apache.poi.ddf.EscherSimpleProperty;
+import org.apache.poi.hssf.record.CommonObjectDataSubRecord;
+import org.apache.poi.hssf.record.EndSubRecord;
+import org.apache.poi.hssf.record.NoteRecord;
+import org.apache.poi.hssf.record.NoteStructureSubRecord;
+import org.apache.poi.hssf.record.ObjRecord;
+import org.apache.poi.hssf.record.TextObjectRecord;
 import org.apache.poi.ss.usermodel.Comment;
 
 /**
@@ -211,6 +221,15 @@ public class HSSFComment extends HSSFTextbox implements Comment {
     protected NoteRecord getNoteRecord() {
         return _note;
     }
+    
+    /**
+     * Do we know which cell this comment belongs to?
+     */
+    public boolean hasPosition() {
+        if (_note == null) return false;
+        if (getColumn() < 0 || getRow() < 0) return false;
+        return true;
+    }
 
     @Override
     public void setShapeType(int shapeType) {
diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java b/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java
index b57588ebfb..170c015ac5 100644
--- a/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java
+++ b/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java
@@ -28,7 +28,20 @@ import org.apache.poi.hssf.model.DrawingManager2;
 import org.apache.poi.hssf.model.HSSFFormulaParser;
 import org.apache.poi.hssf.model.InternalSheet;
 import org.apache.poi.hssf.model.InternalWorkbook;
-import org.apache.poi.hssf.record.*;
+import org.apache.poi.hssf.record.AutoFilterInfoRecord;
+import org.apache.poi.hssf.record.CellValueRecordInterface;
+import org.apache.poi.hssf.record.DVRecord;
+import org.apache.poi.hssf.record.DimensionsRecord;
+import org.apache.poi.hssf.record.DrawingRecord;
+import org.apache.poi.hssf.record.EscherAggregate;
+import org.apache.poi.hssf.record.ExtendedFormatRecord;
+import org.apache.poi.hssf.record.NameRecord;
+import org.apache.poi.hssf.record.NoteRecord;
+import org.apache.poi.hssf.record.Record;
+import org.apache.poi.hssf.record.RowRecord;
+import org.apache.poi.hssf.record.SCLRecord;
+import org.apache.poi.hssf.record.WSBoolRecord;
+import org.apache.poi.hssf.record.WindowTwoRecord;
 import org.apache.poi.hssf.record.aggregates.DataValidityTable;
 import org.apache.poi.hssf.record.aggregates.FormulaRecordAggregate;
 import org.apache.poi.hssf.record.aggregates.WorksheetProtectionBlock;
@@ -2115,7 +2128,7 @@ public final class HSSFSheet implements org.apache.poi.ss.usermodel.Sheet {
             }
             if (shape instanceof HSSFComment) {
                 HSSFComment comment = (HSSFComment) shape;
-                if (comment.getColumn() == column && comment.getRow() == row) {
+                if (comment.hasPosition() && comment.getColumn() == column && comment.getRow() == row) {
                     return comment;
                 }
             }
diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java
index 6df39bbe29..16d8e208b6 100644
--- a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java
+++ b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java
@@ -2549,4 +2549,20 @@ public final class TestBugs extends BaseTestBugzillaIssues {
         assertAlmostEquals(1950, s.getColumnWidth(10), fontAccuracy);
         assertAlmostEquals(2225, s.getColumnWidth(11), fontAccuracy);
     }
+    
+    @Test
+    public void bug56450() {
+        HSSFWorkbook wb = openSample("56450.xls");
+        HSSFSheet sheet = wb.getSheetAt(0);
+        int comments = 0;
+        for (Row r : sheet) {
+            for (Cell c : r) {
+                if (c.getCellComment() != null) {
+                    assertNotNull(c.getCellComment().getString().getString());
+                    comments++;
+                }
+            }
+        }
+        assertEquals(0, comments);
+    }
 }
diff --git a/test-data/spreadsheet/56450.xls b/test-data/spreadsheet/56450.xls
new file mode 100644
index 0000000000..66798a8b49
--- /dev/null
+++ b/test-data/spreadsheet/56450.xls