From 01f4f634010e4345098f046674075bac1e2a2dbf Mon Sep 17 00:00:00 2001 From: Uwe Schindler Date: Wed, 11 Nov 2015 19:11:35 +0000 Subject: [PATCH] Fix remaining security manager problems. Forbiddenapis now passes. git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1713915 13f79535-47bb-0310-9956-ffa450edef68 --- src/java/org/apache/poi/POIDocument.java | 6 +++ .../devtools/forbidden-signatures.txt | 7 ++-- .../org/apache/poi/hwpf/HWPFDocumentCore.java | 5 +++ .../org/apache/poi/hwpf/dev/HWPFLister.java | 26 ++----------- .../apache/poi/hwpf/usermodel/Paragraph.java | 7 +++- .../poi/hwpf/usermodel/SectionProperties.java | 37 ------------------- .../hwpf/model/TestFileInformationBlock.java | 21 +++++++++-- src/testcases/org/apache/poi/POITestCase.java | 4 +- 8 files changed, 44 insertions(+), 69 deletions(-) diff --git a/src/java/org/apache/poi/POIDocument.java b/src/java/org/apache/poi/POIDocument.java index 4345d4b1ca..2131ad8570 100644 --- a/src/java/org/apache/poi/POIDocument.java +++ b/src/java/org/apache/poi/POIDocument.java @@ -35,6 +35,7 @@ import org.apache.poi.poifs.filesystem.DocumentInputStream; import org.apache.poi.poifs.filesystem.NPOIFSFileSystem; import org.apache.poi.poifs.filesystem.OPOIFSFileSystem; import org.apache.poi.poifs.filesystem.POIFSFileSystem; +import org.apache.poi.util.Internal; import org.apache.poi.util.POILogFactory; import org.apache.poi.util.POILogger; @@ -305,4 +306,9 @@ public abstract class POIDocument { * @throws IOException thrown on errors writing to the stream */ public abstract void write(OutputStream out) throws IOException; + + @Internal + public DirectoryNode getDirectory() { + return directory; + } } diff --git a/src/resources/devtools/forbidden-signatures.txt b/src/resources/devtools/forbidden-signatures.txt index 1d9dbfcc7d..75953ccc7e 100644 --- a/src/resources/devtools/forbidden-signatures.txt +++ b/src/resources/devtools/forbidden-signatures.txt @@ -26,7 +26,6 @@ java.util.Locale#setDefault(java.util.Locale) @ Do not use methods that depend o java.util.TimeZone#getDefault() @ Do not use methods that depend on the current Local, either use Locale.ROOT or let the user define the local, see class LocaleUtil for details java.util.Date#toString() @ Do not use methods that depend on the current Local, either use Locale.ROOT or let the user define the local, see class LocaleUtil for details -# disabled as there are still invocations that we could not remove easily -#java.lang.reflect.AccessibleObject#setAccessible(java.lang.reflect.AccessibleObject[], boolean) @ Reflection usage fails with SecurityManagers and likely will not work any more in Java 9 -#java.lang.reflect.AccessibleObject#setAccessible(boolean) @ Reflection usage fails with SecurityManagers and likely will not work any more in Java 9 -#java.lang.reflect.Method#invoke(java.lang.Object, java.lang.Object[]) @ Reflection usage fails with SecurityManagers and likely will not work any more in Java 9 +# Disallow reflection on private object fields/methods +java.lang.reflect.AccessibleObject#setAccessible(java.lang.reflect.AccessibleObject[], boolean) @ Reflection usage fails with SecurityManagers and likely will not work any more in Java 9 +java.lang.reflect.AccessibleObject#setAccessible(boolean) @ Reflection usage fails with SecurityManagers and likely will not work any more in Java 9 diff --git a/src/scratchpad/src/org/apache/poi/hwpf/HWPFDocumentCore.java b/src/scratchpad/src/org/apache/poi/hwpf/HWPFDocumentCore.java index 04fe38ca38..74c73d7524 100644 --- a/src/scratchpad/src/org/apache/poi/hwpf/HWPFDocumentCore.java +++ b/src/scratchpad/src/org/apache/poi/hwpf/HWPFDocumentCore.java @@ -233,4 +233,9 @@ public abstract class HWPFDocumentCore extends POIDocument } public abstract TextPieceTable getTextTable(); + + @Internal + public byte[] getMainStream() { + return _mainStream; + } } diff --git a/src/scratchpad/src/org/apache/poi/hwpf/dev/HWPFLister.java b/src/scratchpad/src/org/apache/poi/hwpf/dev/HWPFLister.java index d008ec907c..54be13f14e 100644 --- a/src/scratchpad/src/org/apache/poi/hwpf/dev/HWPFLister.java +++ b/src/scratchpad/src/org/apache/poi/hwpf/dev/HWPFLister.java @@ -23,7 +23,6 @@ import java.io.File; import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; -import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -32,7 +31,6 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import org.apache.poi.POIDocument; import org.apache.poi.hwpf.HWPFDocument; import org.apache.poi.hwpf.HWPFDocumentCore; import org.apache.poi.hwpf.HWPFOldDocument; @@ -63,7 +61,6 @@ import org.apache.poi.hwpf.usermodel.Picture; import org.apache.poi.hwpf.usermodel.Range; import org.apache.poi.poifs.common.POIFSConstants; import org.apache.poi.poifs.filesystem.DirectoryEntry; -import org.apache.poi.poifs.filesystem.DirectoryNode; import org.apache.poi.poifs.filesystem.Entry; import org.apache.poi.poifs.filesystem.POIFSFileSystem; import org.apache.poi.util.Beta; @@ -458,12 +455,7 @@ public final class HWPFLister public void dumpFileSystem() throws Exception { - java.lang.reflect.Field field = POIDocument.class - .getDeclaredField( "directory" ); - field.setAccessible( true ); - DirectoryNode directoryNode = (DirectoryNode) field.get( _doc ); - - System.out.println( dumpFileSystem( directoryNode ) ); + System.out.println( dumpFileSystem( _doc.getDirectory() ) ); } private String dumpFileSystem( DirectoryEntry directory ) @@ -531,10 +523,7 @@ public final class HWPFLister HWPFDocument doc = (HWPFDocument) _doc; - java.lang.reflect.Field fMainStream = HWPFDocumentCore.class - .getDeclaredField( "_mainStream" ); - fMainStream.setAccessible( true ); - byte[] mainStream = (byte[]) fMainStream.get( _doc ); + byte[] mainStream = _doc.getMainStream(); PlexOfCps binTable = new PlexOfCps( doc.getTableStream(), doc .getFileInformationBlock().getFcPlcfbtePapx(), doc @@ -584,12 +573,6 @@ public final class HWPFLister } } - Method newParagraph = Paragraph.class.getDeclaredMethod( - "newParagraph", Range.class, PAPX.class ); - newParagraph.setAccessible( true ); - java.lang.reflect.Field _props = Paragraph.class - .getDeclaredField( "_props" ); - _props.setAccessible( true ); for ( PAPX papx : _doc.getParagraphTable().getParagraphs() ) { @@ -597,9 +580,8 @@ public final class HWPFLister if ( withProperties ) { - Paragraph paragraph = (Paragraph) newParagraph.invoke( null, - _doc.getOverallRange(), papx ); - System.out.println( _props.get( paragraph ) ); + Paragraph paragraph = Paragraph.newParagraph( _doc.getOverallRange(), papx ); + System.out.println( paragraph.getProps() ); } if ( true ) diff --git a/src/scratchpad/src/org/apache/poi/hwpf/usermodel/Paragraph.java b/src/scratchpad/src/org/apache/poi/hwpf/usermodel/Paragraph.java index 367c3ade7d..c615e76e42 100644 --- a/src/scratchpad/src/org/apache/poi/hwpf/usermodel/Paragraph.java +++ b/src/scratchpad/src/org/apache/poi/hwpf/usermodel/Paragraph.java @@ -94,7 +94,7 @@ public class Paragraph extends Range implements Cloneable { public final static short SPRM_FADJUSTRIGHT = 0x2448; @Internal - static Paragraph newParagraph( Range parent, PAPX papx ) + public static Paragraph newParagraph( Range parent, PAPX papx ) { HWPFDocumentCore doc = parent._doc; ListTables listTables = doc.getListTables(); @@ -659,4 +659,9 @@ public class Paragraph extends Range implements Cloneable { { return "Paragraph [" + getStartOffset() + "; " + getEndOffset() + ")"; } + + @Internal + public ParagraphProperties getProps() { + return _props; + } } diff --git a/src/scratchpad/src/org/apache/poi/hwpf/usermodel/SectionProperties.java b/src/scratchpad/src/org/apache/poi/hwpf/usermodel/SectionProperties.java index 031cdb0e71..2d1ed8eefa 100644 --- a/src/scratchpad/src/org/apache/poi/hwpf/usermodel/SectionProperties.java +++ b/src/scratchpad/src/org/apache/poi/hwpf/usermodel/SectionProperties.java @@ -17,9 +17,6 @@ package org.apache.poi.hwpf.usermodel; -import java.lang.reflect.AccessibleObject; -import java.lang.reflect.Field; - import org.apache.poi.hwpf.model.types.SEPAbstractType; public final class SectionProperties extends SEPAbstractType @@ -45,39 +42,5 @@ public final class SectionProperties extends SEPAbstractType return copy; } - - @Override - public boolean equals( Object obj ) - { - Field[] fields = SectionProperties.class.getSuperclass() - .getDeclaredFields(); - AccessibleObject.setAccessible( fields, true ); - try - { - for ( int x = 0; x < fields.length; x++ ) - { - Object obj1 = fields[x].get( this ); - Object obj2 = fields[x].get( obj ); - if ( obj1 == null && obj2 == null ) - { - continue; - } - if ( obj1 == null || obj2 == null || !obj1.equals( obj2 ) ) - { - return false; - } - } - return true; - } - catch ( Exception e ) - { - return false; - } - } - @Override - public int hashCode() { - assert false : "hashCode not designed"; - return 42; // any arbitrary constant will do - } } diff --git a/src/scratchpad/testcases/org/apache/poi/hwpf/model/TestFileInformationBlock.java b/src/scratchpad/testcases/org/apache/poi/hwpf/model/TestFileInformationBlock.java index 6ca926edac..56a85c3bfb 100644 --- a/src/scratchpad/testcases/org/apache/poi/hwpf/model/TestFileInformationBlock.java +++ b/src/scratchpad/testcases/org/apache/poi/hwpf/model/TestFileInformationBlock.java @@ -19,10 +19,14 @@ package org.apache.poi.hwpf.model; import java.lang.reflect.AccessibleObject; import java.lang.reflect.Field; +import java.security.AccessController; +import java.security.PrivilegedActionException; +import java.security.PrivilegedExceptionAction; import junit.framework.TestCase; import org.apache.poi.hwpf.HWPFDocFixture; +import org.apache.poi.util.SuppressForbidden; public final class TestFileInformationBlock extends TestCase { private FileInformationBlock _fileInformationBlock = null; @@ -37,9 +41,20 @@ public final class TestFileInformationBlock extends TestCase { FileInformationBlock newFileInformationBlock = new FileInformationBlock( buf); - Field[] fields = FileInformationBlock.class.getSuperclass() - .getDeclaredFields(); - AccessibleObject.setAccessible(fields, true); + final Field[] fields; + try { + fields = AccessController.doPrivileged(new PrivilegedExceptionAction() { + @Override + @SuppressForbidden("Test only") + public Field[] run() throws Exception { + final Field[] fields = FileInformationBlock.class.getSuperclass().getDeclaredFields(); + AccessibleObject.setAccessible(fields, true); + return fields; + } + }); + } catch (PrivilegedActionException pae) { + throw pae.getException(); + } for (int x = 0; x < fields.length; x++) { assertEquals(fields[x].get(_fileInformationBlock), diff --git a/src/testcases/org/apache/poi/POITestCase.java b/src/testcases/org/apache/poi/POITestCase.java index f25a7853ec..8bd466c162 100644 --- a/src/testcases/org/apache/poi/POITestCase.java +++ b/src/testcases/org/apache/poi/POITestCase.java @@ -91,7 +91,7 @@ public class POITestCase { } }); } catch (PrivilegedActionException pae) { - throw new AssertionError("Cannot access field '" + fieldName + "' of class " + clazz); + throw new RuntimeException("Cannot access field '" + fieldName + "' of class " + clazz, pae.getException()); } } @@ -112,7 +112,7 @@ public class POITestCase { } }); } catch (PrivilegedActionException pae) { - throw new AssertionError("Cannot access method '" + methodName + "' of class " + clazz); + throw new RuntimeException("Cannot access method '" + methodName + "' of class " + clazz, pae.getException()); } } } -- 2.39.5