diff --git a/src/java/org/apache/poi/poifs/property/DirectoryProperty.java b/src/java/org/apache/poi/poifs/property/DirectoryProperty.java index 5681ad5f0..e0d817cf0 100644 --- a/src/java/org/apache/poi/poifs/property/DirectoryProperty.java +++ b/src/java/org/apache/poi/poifs/property/DirectoryProperty.java @@ -127,7 +127,7 @@ public class DirectoryProperty return result; } - private class PropertyComparator + public static class PropertyComparator implements Comparator { @@ -162,13 +162,40 @@ public class DirectoryProperty public int compare(Object o1, Object o2) { + String VBA_PROJECT = "_VBA_PROJECT"; String name1 = (( Property ) o1).getName(); String name2 = (( Property ) o2).getName(); - int result = name1.length() - name2.length(); + int result = name1.length() - name2.length(); if (result == 0) { - result = name1.compareTo(name2); + // _VBA_PROJECT, it seems, will always come last + if (name1.compareTo(VBA_PROJECT) == 0) + result = 1; + else if (name2.compareTo(VBA_PROJECT) == 0) + result = -1; + else + { + if (name1.startsWith("__") && name2.startsWith("__")) + { + // Betweeen __SRP_0 and __SRP_1 just sort as normal + result = name1.compareToIgnoreCase(name2); + } + else if (name1.startsWith("__")) + { + // If only name1 is __XXX then this will be placed after name2 + result = 1; + } + else if (name2.startsWith("__")) + { + // If only name2 is __XXX then this will be placed after name1 + result = -1; + } + else + // result = name1.compareTo(name2); + // The default case is to sort names ignoring case + result = name1.compareToIgnoreCase(name2); + } } return result; } diff --git a/src/testcases/org/apache/poi/hssf/data/39234.xls b/src/testcases/org/apache/poi/hssf/data/39234.xls new file mode 100644 index 000000000..20fc9a001 Binary files /dev/null and b/src/testcases/org/apache/poi/hssf/data/39234.xls differ diff --git a/src/testcases/org/apache/poi/poifs/filesystem/TestPropertySorter.java b/src/testcases/org/apache/poi/poifs/filesystem/TestPropertySorter.java new file mode 100644 index 000000000..7ddd2cca9 --- /dev/null +++ b/src/testcases/org/apache/poi/poifs/filesystem/TestPropertySorter.java @@ -0,0 +1,152 @@ + +/* ==================================================================== + Copyright 2002-2004 Apache Software Foundation + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +==================================================================== */ + +package org.apache.poi.poifs.filesystem; + +import junit.framework.TestCase; +import junit.framework.ComparisonFailure; + +import java.io.*; +import java.util.*; + +import org.apache.poi.poifs.property.DirectoryProperty; +import org.apache.poi.poifs.property.Property; + +/** + * Verify the order of entries DirectoryProperty . + *

+ * In particular it is important to serialize ROOT._VBA_PROJECT_CUR.VBA node. + * See bug 39234 in bugzilla. Thanks to Bill Seddon for providing the solution. + *

+ * + * @author Yegor Kozlov + */ +public class TestPropertySorter extends TestCase { + + //the correct order of entries in the test file + protected static final String[] _entries = { + "dir", "JML", "UTIL", "Loader", "Sheet1", "Sheet2", "Sheet3", + "__SRP_0", "__SRP_1", "__SRP_2", "__SRP_3", "__SRP_4", "__SRP_5", + "ThisWorkbook", "_VBA_PROJECT", + }; + + protected File testFile; + + public void setUp(){ + String home = System.getProperty("HSSF.testdata.path"); + testFile = new File(home + "/39234.xls"); + } + + /** + * Test sorting of properties in DirectoryProperty + */ + public void testSortProperties() throws IOException { + InputStream is = new FileInputStream(testFile); + POIFSFileSystem fs = new POIFSFileSystem(is); + is.close(); + Property[] props = getVBAProperties(fs); + + assertEquals(_entries.length, props.length); + + // (1). See that there is a problem with the old case-sensitive property comparartor + Arrays.sort(props, new CaseSensitivePropertyComparator()); + try { + for (int i = 0; i < props.length; i++) { + assertEquals(_entries[i], props[i].getName()); + } + fail("case-sensitive property comparator returns properties in wrong order"); + } catch (ComparisonFailure e){ + ; // as expected + } + + // (2) Verify that the fixed proeprty comparator works right + Arrays.sort(props, new DirectoryProperty.PropertyComparator()); + for (int i = 0; i < props.length; i++) { + assertEquals(_entries[i], props[i].getName()); + } + } + + /** + * Serialize file system and verify that the order of properties is the same as in the original file. + */ + public void testSerialization() throws IOException { + InputStream is = new FileInputStream(testFile); + POIFSFileSystem fs = new POIFSFileSystem(is); + is.close(); + + ByteArrayOutputStream out = new ByteArrayOutputStream(); + fs.writeFilesystem(out); + out.close(); + is = new ByteArrayInputStream(out.toByteArray()); + fs = new POIFSFileSystem(is); + is.close(); + Property[] props = getVBAProperties(fs); + Arrays.sort(props, new DirectoryProperty.PropertyComparator()); + + assertEquals(_entries.length, props.length); + for (int i = 0; i < props.length; i++) { + assertEquals(_entries[i], props[i].getName()); + } + } + + /** + * @return array of properties read from ROOT._VBA_PROJECT_CUR.VBA node + */ + protected Property[] getVBAProperties(POIFSFileSystem fs) throws IOException { + String _VBA_PROJECT_CUR = "_VBA_PROJECT_CUR"; + String VBA = "VBA"; + + DirectoryEntry root = fs.getRoot(); + DirectoryEntry vba_project = (DirectoryEntry)root.getEntry(_VBA_PROJECT_CUR); + + DirectoryNode vba = (DirectoryNode)vba_project.getEntry(VBA); + DirectoryProperty p = (DirectoryProperty)vba.getProperty(); + + ArrayList lst = new ArrayList(); + for (Iterator it = p.getChildren(); it.hasNext();){ + Property ch = (Property)it.next(); + lst.add(ch); + } + return (Property [])lst.toArray(new Property[ 0 ]); + } + + /** + * Old version of case-sensitive PropertyComparator to demonstrate the problem + */ + private class CaseSensitivePropertyComparator implements Comparator + { + + public boolean equals(Object o) + { + return this == o; + } + + public int compare(Object o1, Object o2) + { + String name1 = (( Property ) o1).getName(); + String name2 = (( Property ) o2).getName(); + int result = name1.length() - name2.length(); + + if (result == 0) + { + result = name1.compareTo(name2); + } + return result; + } + } + +}