Apache
/
poi
mirror of https://github.com/apache/poi.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Do not use a WeakReference for the parent-link in HWPF-Ranges 

Some unit-tests show sporadic failures and it seems functionality is actually
broken if the WeakReference is garbage-collected. Not sure why a
WeakReference was used here anyway.

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1866054 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Dominik Stadler 2019-08-29 05:15:57 +00:00
parent d81b74ba67
commit 734d6911d8
2 changed files with 12 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
src/scratchpad/src/org/apache/poi/hwpf/usermodel/Range.java
View File

@ -17,7 +17,6 @@
package org.apache.poi.hwpf.usermodel; package org.apache.poi.hwpf.usermodel;
import java.lang.ref.WeakReference;
import java.util.List; import java.util.List;
import org.apache.poi.hwpf.HWPFDocument; import org.apache.poi.hwpf.HWPFDocument;
@ -98,7 +97,7 @@ public class Range { // TODO -instantiable superclass
public static final int TYPE_UNDEFINED = 6; public static final int TYPE_UNDEFINED = 6;
/** Needed so inserts and deletes will ripple up through containing Ranges */ /** Needed so inserts and deletes will ripple up through containing Ranges */
private final WeakReference<Range> _parent; private final Range _parent;
/** The starting character offset of this range. */ /** The starting character offset of this range. */
protected final int _start; protected final int _start;
@ -167,7 +166,7 @@ public class Range { // TODO -instantiable superclass
_paragraphs = _doc.getParagraphTable().getParagraphs(); _paragraphs = _doc.getParagraphTable().getParagraphs();
_characters = _doc.getCharacterTable().getTextRuns(); _characters = _doc.getCharacterTable().getTextRuns();
_text = _doc.getText(); _text = _doc.getText();
_parent = new WeakReference<>(null); _parent = null;
sanityCheckStartEnd(); sanityCheckStartEnd();
} }
@ -190,7 +189,7 @@ public class Range { // TODO -instantiable superclass
_paragraphs = parent._paragraphs; _paragraphs = parent._paragraphs;
_characters = parent._characters; _characters = parent._characters;
_text = parent._text; _text = parent._text;
_parent = new WeakReference<>(parent); _parent = parent;
sanityCheckStartEnd(); sanityCheckStartEnd();
sanityCheck(); sanityCheck();
@ -558,7 +557,7 @@ public class Range { // TODO -instantiable superclass
} }
_text.delete( _start, _end ); _text.delete( _start, _end );
Range parent = _parent.get(); Range parent = _parent;
if ( parent != null ) if ( parent != null )
{ {
parent.adjustForInsert( -( _end - _start ) ); parent.adjustForInsert( -( _end - _start ) );
@ -776,7 +775,7 @@ public class Range { // TODO -instantiable superclass
} }
Range r = paragraph; Range r = paragraph;
if (r._parent.get() != this) { if (r._parent != this) {
throw new IllegalArgumentException("This paragraph is not a child of this range instance"); throw new IllegalArgumentException("This paragraph is not a child of this range instance");
} }
@ -1119,7 +1118,7 @@ public class Range { // TODO -instantiable superclass
_end += length; _end += length;
reset(); reset();
Range parent = _parent.get(); Range parent = _parent;
if (parent != null) { if (parent != null) {
parent.adjustForInsert(length); parent.adjustForInsert(length);
} }

9
src/scratchpad/testcases/org/apache/poi/hwpf/usermodel/TestBug47563.java
View File

@ -18,10 +18,12 @@ package org.apache.poi.hwpf.usermodel;
import org.apache.poi.hwpf.HWPFDocument; import org.apache.poi.hwpf.HWPFDocument;
import org.apache.poi.hwpf.HWPFTestDataSamples; import org.apache.poi.hwpf.HWPFTestDataSamples;
import org.apache.poi.util.HexDump;
import org.junit.Test; import org.junit.Test;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
import org.junit.runners.Parameterized; import org.junit.runners.Parameterized;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collection; import java.util.Collection;
import java.util.List; import java.util.List;
@ -29,7 +31,7 @@ import java.util.List;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
/** /**
* Bug 47563 - Exception when working with table * Bug 47563 - Exception when working with table
*/ */
@RunWith(Parameterized.class) @RunWith(Parameterized.class)
public class TestBug47563 { public class TestBug47563 {
@ -94,11 +96,12 @@ public class TestBug47563 {
} }
String text = range.text(); String text = range.text();
String textBytes = HexDump.toHex(text.getBytes(StandardCharsets.UTF_8));
int mustBeAfter = 0; int mustBeAfter = 0;
for (int i = 0; i < rows * columns; i++) { for (int i = 0; i < rows * columns; i++) {
int next = text.indexOf(Integer.toString(i), mustBeAfter); int next = text.indexOf(Integer.toString(i), mustBeAfter);
assertTrue("Test with " + rows + "/" + columns + ": Should not find " + i + assertTrue("Test with " + rows + "/" + columns + ": Should find " + i +
" but found it at " + next + " with " + mustBeAfter + " in " + text + "\n" + " but did not find it (" + next + ") with " + mustBeAfter + " in " + textBytes + "\n" +
text.indexOf(Integer.toString(i), mustBeAfter), text.indexOf(Integer.toString(i), mustBeAfter),
next != -1); next != -1);
mustBeAfter = next; mustBeAfter = next;