From 3b55b3e0a3fcdd653cba0c4c34a852b7330e42b7 Mon Sep 17 00:00:00 2001 From: ian zhang Date: Thu, 16 Jul 2026 16:04:19 +0800 Subject: [PATCH 1/3] fix: XLS BIFF8 encryption not applied due to premature password clearing The refactor in 7fe23f0 added a finally block in WorkBookUtil.createWorkBook() that clears the Biff8EncryptionKey ThreadLocal immediately after setting it. However, workbook.write() (which applies BIFF8 encryption) runs later in WriteContextImpl.finish(). At that point the password is already null, so the file content is never encrypted - only a write-protection flag is set. Fix: Remove the premature clearing. The password is correctly cleared by WriteContextImpl.clearEncrypt03() after workbook.write() completes. Tests: - Update WorkBookUtilTest to verify password remains set after createWorkBook() - Add EncryptDataTest.xlsPasswordWrite_isActuallyEncrypted to verify that reading an encrypted XLS without a password fails --- .../apache/fesod/sheet/util/WorkBookUtil.java | 8 ++--- .../sheet/readwrite/EncryptDataTest.java | 34 +++++++++++++++++++ .../fesod/sheet/util/WorkBookUtilTest.java | 5 ++- 3 files changed, 40 insertions(+), 7 deletions(-) diff --git a/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/WorkBookUtil.java b/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/WorkBookUtil.java index a8e1d076f..7802d1034 100644 --- a/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/WorkBookUtil.java +++ b/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/WorkBookUtil.java @@ -93,12 +93,8 @@ public static void createWorkBook(WriteWorkbookHolder writeWorkbookHolder) throw writeWorkbookHolder.setCachedWorkbook(hssfWorkbook); writeWorkbookHolder.setWorkbook(hssfWorkbook); if (writeWorkbookHolder.getPassword() != null) { - try { - Biff8EncryptionKey.setCurrentUserPassword(writeWorkbookHolder.getPassword()); - hssfWorkbook.writeProtectWorkbook(writeWorkbookHolder.getPassword(), StringUtils.EMPTY); - } finally { - Biff8EncryptionKey.setCurrentUserPassword(null); - } + Biff8EncryptionKey.setCurrentUserPassword(writeWorkbookHolder.getPassword()); + hssfWorkbook.writeProtectWorkbook(writeWorkbookHolder.getPassword(), StringUtils.EMPTY); } return; case CSV: diff --git a/fesod-sheet/src/test/java/org/apache/fesod/sheet/readwrite/EncryptDataTest.java b/fesod-sheet/src/test/java/org/apache/fesod/sheet/readwrite/EncryptDataTest.java index 1741d581a..057844819 100644 --- a/fesod-sheet/src/test/java/org/apache/fesod/sheet/readwrite/EncryptDataTest.java +++ b/fesod-sheet/src/test/java/org/apache/fesod/sheet/readwrite/EncryptDataTest.java @@ -41,6 +41,7 @@ import org.apache.fesod.sheet.write.builder.ExcelWriterBuilder; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; /** @@ -104,4 +105,37 @@ private void readAndWrite( Assertions.assertEquals(10, dataList.size()); Assertions.assertNotNull(dataList.get(0).getName()); } + + /** + * Verifies that an XLS file written with a password is actually encrypted at the BIFF8 record level, + * not merely flagged as write-protected. Without the correct password, reading the file content + * must fail. + */ + @Test + void xlsPasswordWrite_isActuallyEncrypted() throws Exception { + File file = createTempFile("enc-verify", ExcelFormat.XLS); + + // Write an encrypted XLS file + FesodSheet.write(file, SimpleData.class) + .excelType(ExcelTypeEnum.XLS) + .password(PASSWORD) + .sheet() + .doWrite(TestDataBuilder.simpleData(10)); + + // Reading without the password must fail because the content is BIFF8-encrypted + Assertions.assertThrows( + Exception.class, + () -> FesodSheet.read(file, SimpleData.class, new CollectingReadListener()) + .excelType(ExcelTypeEnum.XLS) + .sheet() + .doReadSync()); + + // Reading with the correct password must succeed + List dataList = FesodSheet.read(file, SimpleData.class, new CollectingReadListener()) + .excelType(ExcelTypeEnum.XLS) + .password(PASSWORD) + .sheet() + .doReadSync(); + Assertions.assertEquals(10, dataList.size()); + } } diff --git a/fesod-sheet/src/test/java/org/apache/fesod/sheet/util/WorkBookUtilTest.java b/fesod-sheet/src/test/java/org/apache/fesod/sheet/util/WorkBookUtilTest.java index 713553693..0b2db0ffc 100644 --- a/fesod-sheet/src/test/java/org/apache/fesod/sheet/util/WorkBookUtilTest.java +++ b/fesod-sheet/src/test/java/org/apache/fesod/sheet/util/WorkBookUtilTest.java @@ -231,7 +231,10 @@ void test_createWorkBook_XLS_Password() throws IOException { // Verify Mockito.verify(writeWorkbookHolder).setWorkbook(Mockito.any(HSSFWorkbook.class)); - Assertions.assertNull(Biff8EncryptionKey.getCurrentUserPassword()); + // The BIFF8 encryption password must remain set after createWorkBook() so that + // workbook.write() (called later in WriteContextImpl.finish()) can apply encryption. + // WriteContextImpl.clearEncrypt03() clears it after writing completes. + Assertions.assertEquals("123456", Biff8EncryptionKey.getCurrentUserPassword()); } @Test From b28d736728429a8863a609479b63b548c6247b34 Mon Sep 17 00:00:00 2001 From: ian zhang Date: Sun, 19 Jul 2026 10:24:11 +0800 Subject: [PATCH 2/3] style: fix spotless formatting in EncryptDataTest Merge the assertThrows lambda argument onto a single line to comply with the project Spotless formatting rules. --- .../java/org/apache/fesod/sheet/readwrite/EncryptDataTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fesod-sheet/src/test/java/org/apache/fesod/sheet/readwrite/EncryptDataTest.java b/fesod-sheet/src/test/java/org/apache/fesod/sheet/readwrite/EncryptDataTest.java index 057844819..edeeb8b80 100644 --- a/fesod-sheet/src/test/java/org/apache/fesod/sheet/readwrite/EncryptDataTest.java +++ b/fesod-sheet/src/test/java/org/apache/fesod/sheet/readwrite/EncryptDataTest.java @@ -124,8 +124,7 @@ void xlsPasswordWrite_isActuallyEncrypted() throws Exception { // Reading without the password must fail because the content is BIFF8-encrypted Assertions.assertThrows( - Exception.class, - () -> FesodSheet.read(file, SimpleData.class, new CollectingReadListener()) + Exception.class, () -> FesodSheet.read(file, SimpleData.class, new CollectingReadListener()) .excelType(ExcelTypeEnum.XLS) .sheet() .doReadSync()); From 57f16d2205de9ac8d03f0c865140130e3a4fd8a4 Mon Sep 17 00:00:00 2001 From: ian zhang Date: Sun, 19 Jul 2026 11:17:15 +0800 Subject: [PATCH 3/3] fix: address Copilot review comments on PR #959 - WorkBookUtil: use !StringUtils.isEmpty() instead of != null to prevent empty password ThreadLocal leak (clearEncrypt03 returns early for empty passwords) - EncryptDataTest: use EncryptedDocumentException instead of Exception for more precise assertion of encryption failure --- .../org/apache/fesod/sheet/util/WorkBookUtil.java | 2 +- .../apache/fesod/sheet/readwrite/EncryptDataTest.java | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/WorkBookUtil.java b/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/WorkBookUtil.java index 7802d1034..244384205 100644 --- a/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/WorkBookUtil.java +++ b/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/WorkBookUtil.java @@ -92,7 +92,7 @@ public static void createWorkBook(WriteWorkbookHolder writeWorkbookHolder) throw } writeWorkbookHolder.setCachedWorkbook(hssfWorkbook); writeWorkbookHolder.setWorkbook(hssfWorkbook); - if (writeWorkbookHolder.getPassword() != null) { + if (!StringUtils.isEmpty(writeWorkbookHolder.getPassword())) { Biff8EncryptionKey.setCurrentUserPassword(writeWorkbookHolder.getPassword()); hssfWorkbook.writeProtectWorkbook(writeWorkbookHolder.getPassword(), StringUtils.EMPTY); } diff --git a/fesod-sheet/src/test/java/org/apache/fesod/sheet/readwrite/EncryptDataTest.java b/fesod-sheet/src/test/java/org/apache/fesod/sheet/readwrite/EncryptDataTest.java index edeeb8b80..1eb375ee8 100644 --- a/fesod-sheet/src/test/java/org/apache/fesod/sheet/readwrite/EncryptDataTest.java +++ b/fesod-sheet/src/test/java/org/apache/fesod/sheet/readwrite/EncryptDataTest.java @@ -39,6 +39,7 @@ import org.apache.fesod.sheet.testkit.models.SimpleData; import org.apache.fesod.sheet.testkit.params.ExcelFormatSource; import org.apache.fesod.sheet.write.builder.ExcelWriterBuilder; +import org.apache.poi.EncryptedDocumentException; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; @@ -123,11 +124,11 @@ void xlsPasswordWrite_isActuallyEncrypted() throws Exception { .doWrite(TestDataBuilder.simpleData(10)); // Reading without the password must fail because the content is BIFF8-encrypted - Assertions.assertThrows( - Exception.class, () -> FesodSheet.read(file, SimpleData.class, new CollectingReadListener()) - .excelType(ExcelTypeEnum.XLS) - .sheet() - .doReadSync()); + Assertions.assertThrows(EncryptedDocumentException.class, () -> FesodSheet.read( + file, SimpleData.class, new CollectingReadListener()) + .excelType(ExcelTypeEnum.XLS) + .sheet() + .doReadSync()); // Reading with the correct password must succeed List dataList = FesodSheet.read(file, SimpleData.class, new CollectingReadListener())