Adjust trimming sheetnames > 31 chars

Define Constant MAX_SENSITIVE_SHEET_NAME_LEN = 31
Warn about trimmed sheet names
Implement sheetname trimming for HSSFWorkbook
Add use case tests

Closes #273

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

3 files changed, 38 insertions, 5 deletions

diff --git a/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java b/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java
index 9c989004b9..9fcdf6c81e 100644
--- a/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java
+++ b/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java
@@ -877,10 +877,10 @@ public final class HSSFWorkbook extends POIDocument implements Workbook {
             // Try and find the next sheet name that is unique
             String index = Integer.toString(uniqueIndex++);
             String name;
-            if (baseName.length() + index.length() + 2 < 31) {
+            if (baseName.length() + index.length() + 2 < MAX_SENSITIVE_SHEET_NAME_LEN) {
                 name = baseName + " (" + index + ")";
             } else {
-                name = baseName.substring(0, 31 - index.length() - 2) + "(" + index + ")";
+                name = baseName.substring(0, MAX_SENSITIVE_SHEET_NAME_LEN - index.length() - 2) + "(" + index + ")";
             }
 
             //If the sheet name is unique, then set it otherwise move on to the next number.
@@ -936,8 +936,22 @@ public final class HSSFWorkbook extends POIDocument implements Workbook {
             throw new IllegalArgumentException("sheetName must not be null");
         }
 
-        if (workbook.doesContainsSheetName(sheetname, _sheets.size())) {
-            throw new IllegalArgumentException("The workbook already contains a sheet named '" + sheetname + "'");
+		if (workbook.doesContainsSheetName(sheetname, _sheets.size())) {
+			throw new IllegalArgumentException("The workbook already contains a sheet named '" + sheetname + "'");
+		}
+
+		// YK: Mimic Excel and silently truncate sheet names longer than 31 characters
+        // Issue a WARNING though in order to prevent a situation, where the provided long sheet name is
+        // not accessible due to the trimming while we are not even aware of the reason and continue to use
+        // the long name in generated formulas
+        if(sheetname.length() > MAX_SENSITIVE_SHEET_NAME_LEN) {
+            String trimmedSheetname = sheetname.substring(0, MAX_SENSITIVE_SHEET_NAME_LEN);
+
+			// we still need to warn about the trimming as the original sheet name won't be available
+			// e.g. when referenced by formulas
+			LOGGER.atWarn().log("Sheet '{}' will be added with a trimmed name '{}' for MS Excel compliance.",
+					sheetname, trimmedSheetname);
+			sheetname = trimmedSheetname;
         }
 
         HSSFSheet sheet = new HSSFSheet(this);
diff --git a/poi/src/main/java/org/apache/poi/ss/usermodel/Workbook.java b/poi/src/main/java/org/apache/poi/ss/usermodel/Workbook.java
index b231382b25..eac127a873 100644
--- a/poi/src/main/java/org/apache/poi/ss/usermodel/Workbook.java
+++ b/poi/src/main/java/org/apache/poi/ss/usermodel/Workbook.java
@@ -55,6 +55,12 @@ public interface Workbook extends Closeable, Iterable<Sheet> {
     int PICTURE_TYPE_DIB = 7;
 
     /**
+     * Excel silently truncates long sheet names to 31 chars.
+     * This constant is used to ensure uniqueness in the first 31 chars
+     */
+    int MAX_SENSITIVE_SHEET_NAME_LEN = 31;
+
+    /**
      * Convenience method to get the active sheet.  The active sheet is is the sheet
      * which is currently displayed when the workbook is viewed in Excel.
      * 'Selected' sheet(s) is a distinct concept.
diff --git a/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestWorkbook.java b/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestWorkbook.java
index eba86eadf3..d41a9a9aa3 100644
--- a/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestWorkbook.java
+++ b/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestWorkbook.java
@@ -525,7 +525,7 @@ public abstract class BaseTestWorkbook {
     }
 
     /**
-     * Tests that all of the unicode capable string fields can be set, written and then read back
+     * Tests that all the unicode capable string fields can be set, written and then read back
      */
     @Test
     protected void unicodeInAll() throws IOException {
@@ -905,4 +905,17 @@ public abstract class BaseTestWorkbook {
             assertEquals(1114425, anchor.getDx2()); //HSSF: 171
         }
     }
+
+	@Test
+	void testSheetNameTrimming() throws IOException {
+		try (Workbook workbook = _testDataProvider.createWorkbook()) {
+			Sheet sheet = workbook.createSheet("MyVeryLongSheetName_9999999999999999");
+			assertNotNull(sheet);
+			assertEquals("MyVeryLongSheetName_99999999999", workbook.getSheetName(0));
+
+			assertThrows(IllegalArgumentException.class,
+					() -> workbook.createSheet("MyVeryLongSheetName_9999999999999998")
+			);
+		}
+	}
 }