Skip to content

Commit 0f6da44

Browse files
author
Gary Gregory
committed
Prevent infinite loop with AbstractCharacterFilterReader if EOF is
filtered out #226. - Based on PR #226 by Rob Spoor, with an additional (missing) test, and clean ups. - Add CharacterSetFilterReader.CharacterSetFilterReader(Reader, Integer...).
1 parent f1a143b commit 0f6da44

5 files changed

Lines changed: 82 additions & 24 deletions

File tree

‎src/changes/changes.xml‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,9 @@ The <action> type attribute can be add,update,fix,remove.
123123
<action issue="IO-720" dev="ggregory" type="fix" due-to="XenoAmess">
124124
Fix error about usage of DirectBuffer in JRE 16/17 #205.
125125
</action>
126+
<action dev="ggregory" type="fix" due-to="Rob Spoor, Gary Gregory">
127+
Prevent infinite loop with AbstractCharacterFilterReader if EOF is filtered out #226.
128+
</action>
126129
<!-- ADD -->
127130
<action dev="ggregory" type="add" due-to="Gary Gregory">
128131
Add FileSystemProviders class.
@@ -193,9 +196,12 @@ The <action> type attribute can be add,update,fix,remove.
193196
<action dev="ggregory" type="add" due-to="Gary Gregory">
194197
Add copy(URL, OutputStream).
195198
</action>
196-
<action issue="IO-651" dev="ggregory" type="add" due-to="jmark109, Gary Gregory">
199+
<action issue="IO-651" dev="ggregory" type="add" due-to="jmark109, Gary Gregory">
197200
Add DeferredFileOutputStream.toInputStream() #206.
198201
</action>
202+
<action dev="ggregory" type="add" due-to="Gary Gregory">
203+
Add CharacterSetFilterReader.CharacterSetFilterReader(Reader, Integer...).
204+
</action>
199205
<!-- UPDATES -->
200206
<action dev="ggregory" type="update" due-to="Dependabot">
201207
Update junit-jupiter from 5.6.2 to 5.7.0 #153.

‎src/main/java/org/apache/commons/io/input/AbstractCharacterFilterReader.java‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,7 @@ public abstract class AbstractCharacterFilterReader extends FilterReader {
3030
/**
3131
* Constructs a new reader.
3232
*
33-
* @param reader
34-
* the reader to filter
33+
* @param reader the reader to filter
3534
*/
3635
protected AbstractCharacterFilterReader(final Reader reader) {
3736
super(reader);
@@ -42,15 +41,14 @@ public int read() throws IOException {
4241
int ch;
4342
do {
4443
ch = in.read();
45-
} while (filter(ch));
44+
} while (ch != EOF && filter(ch));
4645
return ch;
4746
}
4847

4948
/**
5049
* Returns true if the given character should be filtered out, false to keep the character.
5150
*
52-
* @param ch
53-
* the character to test.
51+
* @param ch the character to test.
5452
* @return true if the given character should be filtered out, false to keep the character.
5553
*/
5654
protected abstract boolean filter(int ch);

‎src/main/java/org/apache/commons/io/input/CharacterSetFilterReader.java‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,17 @@
1717
package org.apache.commons.io.input;
1818

1919
import java.io.Reader;
20+
import java.util.Arrays;
2021
import java.util.Collections;
22+
import java.util.HashSet;
2123
import java.util.Set;
2224

2325
/**
24-
* A filter reader that removes a given set of characters represented as {@code int} code points, handy to remove
25-
* known junk characters from CSV files for example.
26+
* A filter reader that removes a given set of characters represented as {@code int} code points, handy to remove known
27+
* junk characters from CSV files for example.
2628
* <p>
27-
* This class must convert each {@code int} read to an {@code Integer}. You can increase the Integer cache
28-
* with a system property, see {@link Integer}.
29+
* This class must convert each {@code int} read to an {@code Integer}. You can increase the Integer cache with a system
30+
* property, see {@link Integer}.
2931
* </p>
3032
*/
3133
public class CharacterSetFilterReader extends AbstractCharacterFilterReader {
@@ -36,10 +38,19 @@ public class CharacterSetFilterReader extends AbstractCharacterFilterReader {
3638
/**
3739
* Constructs a new reader.
3840
*
39-
* @param reader
40-
* the reader to filter.
41-
* @param skip
42-
* the set of characters to filter out.
41+
* @param reader the reader to filter.
42+
* @param skip the set of characters to filter out.
43+
* @since 2.9.0
44+
*/
45+
public CharacterSetFilterReader(final Reader reader, final Integer... skip) {
46+
this(reader, new HashSet<>(Arrays.asList(skip)));
47+
}
48+
49+
/**
50+
* Constructs a new reader.
51+
*
52+
* @param reader the reader to filter.
53+
* @param skip the set of characters to filter out.
4354
*/
4455
public CharacterSetFilterReader(final Reader reader, final Set<Integer> skip) {
4556
super(reader);

‎src/test/java/org/apache/commons/io/input/CharacterFilterReaderTest.java‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,13 @@
1616
*/
1717
package org.apache.commons.io.input;
1818

19+
import static org.apache.commons.io.IOUtils.EOF;
1920
import static org.junit.jupiter.api.Assertions.assertEquals;
21+
import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
2022

2123
import java.io.IOException;
2224
import java.io.StringReader;
25+
import java.time.Duration;
2326
import java.util.HashSet;
2427

2528
import org.apache.commons.io.IOUtils;
@@ -28,6 +31,8 @@
2831

2932
public class CharacterFilterReaderTest {
3033

34+
private static final String STRING_FIXTURE = "ababcabcd";
35+
3136
@Test
3237
public void testInputSize0FilterSize1() throws IOException {
3338
final StringReader input = new StringReader("");
@@ -41,7 +46,7 @@ public void testInputSize0FilterSize1() throws IOException {
4146
@Test
4247
public void testInputSize1FilterSize1() throws IOException {
4348
try (StringReader input = new StringReader("a");
44-
CharacterFilterReader reader = new CharacterFilterReader(input, 'a')) {
49+
CharacterFilterReader reader = new CharacterFilterReader(input, 'a')) {
4550
assertEquals(-1, reader.read());
4651
}
4752
}
@@ -72,9 +77,24 @@ public void testInputSize2FilterSize1FilterLast() throws IOException {
7277
}
7378
}
7479

80+
@Test
81+
public void testReadFilteringEOF() throws IOException {
82+
final StringReader input = new StringReader(STRING_FIXTURE);
83+
assertTimeoutPreemptively(Duration.ofMillis(500), () -> {
84+
try (StringBuilderWriter output = new StringBuilderWriter();
85+
CharacterFilterReader reader = new CharacterFilterReader(input, EOF)) {
86+
int c;
87+
while ((c = reader.read()) != EOF) {
88+
output.write(c);
89+
}
90+
assertEquals(STRING_FIXTURE, output.toString());
91+
}
92+
});
93+
}
94+
7595
@Test
7696
public void testReadIntoBuffer() throws IOException {
77-
final StringReader input = new StringReader("ababcabcd");
97+
final StringReader input = new StringReader(STRING_FIXTURE);
7898
try (CharacterFilterReader reader = new CharacterFilterReader(input, 'b')) {
7999
final char[] buff = new char[9];
80100
final int charCount = reader.read(buff);
@@ -85,8 +105,9 @@ public void testReadIntoBuffer() throws IOException {
85105

86106
@Test
87107
public void testReadUsingReader() throws IOException {
88-
final StringReader input = new StringReader("ababcabcd");
89-
try (StringBuilderWriter output = new StringBuilderWriter(); CharacterFilterReader reader = new CharacterFilterReader(input, 'b')) {
108+
final StringReader input = new StringReader(STRING_FIXTURE);
109+
try (StringBuilderWriter output = new StringBuilderWriter();
110+
CharacterFilterReader reader = new CharacterFilterReader(input, 'b')) {
90111
IOUtils.copy(reader, output);
91112
assertEquals("aacacd", output.toString());
92113
}

‎src/test/java/org/apache/commons/io/input/CharacterSetFilterReaderTest.java‎

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,23 @@
1616
*/
1717
package org.apache.commons.io.input;
1818

19+
import static org.apache.commons.io.IOUtils.EOF;
1920
import static org.junit.jupiter.api.Assertions.assertEquals;
21+
import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
2022

2123
import java.io.IOException;
2224
import java.io.StringReader;
25+
import java.time.Duration;
2326
import java.util.HashSet;
27+
import java.util.Set;
2428

29+
import org.apache.commons.io.output.StringBuilderWriter;
2530
import org.junit.jupiter.api.Test;
2631

2732
public class CharacterSetFilterReaderTest {
2833

34+
private static final String STRING_FIXTURE = "ab";
35+
2936
@Test
3037
public void testInputSize0FilterSize0() throws IOException {
3138
final StringReader input = new StringReader("");
@@ -47,7 +54,7 @@ public void testInputSize0FilterSize1() throws IOException {
4754
@Test
4855
public void testInputSize0NullFilter() throws IOException {
4956
final StringReader input = new StringReader("");
50-
try (CharacterSetFilterReader reader = new CharacterSetFilterReader(input, null)) {
57+
try (CharacterSetFilterReader reader = new CharacterSetFilterReader(input, (Set<Integer>) null)) {
5158
assertEquals(-1, reader.read());
5259
}
5360
}
@@ -75,7 +82,7 @@ public void testInputSize2FilterSize1FilterAll() throws IOException {
7582

7683
@Test
7784
public void testInputSize2FilterSize1FilterFirst() throws IOException {
78-
final StringReader input = new StringReader("ab");
85+
final StringReader input = new StringReader(STRING_FIXTURE);
7986
final HashSet<Integer> codePoints = new HashSet<>();
8087
codePoints.add(Integer.valueOf('a'));
8188
try (CharacterSetFilterReader reader = new CharacterSetFilterReader(input, codePoints)) {
@@ -86,7 +93,7 @@ public void testInputSize2FilterSize1FilterFirst() throws IOException {
8693

8794
@Test
8895
public void testInputSize2FilterSize1FilterLast() throws IOException {
89-
final StringReader input = new StringReader("ab");
96+
final StringReader input = new StringReader(STRING_FIXTURE);
9097
final HashSet<Integer> codePoints = new HashSet<>();
9198
codePoints.add(Integer.valueOf('b'));
9299
try (CharacterSetFilterReader reader = new CharacterSetFilterReader(input, codePoints)) {
@@ -97,7 +104,7 @@ public void testInputSize2FilterSize1FilterLast() throws IOException {
97104

98105
@Test
99106
public void testInputSize2FilterSize2FilterFirst() throws IOException {
100-
final StringReader input = new StringReader("ab");
107+
final StringReader input = new StringReader(STRING_FIXTURE);
101108
final HashSet<Integer> codePoints = new HashSet<>();
102109
codePoints.add(Integer.valueOf('a'));
103110
codePoints.add(Integer.valueOf('y'));
@@ -109,7 +116,7 @@ public void testInputSize2FilterSize2FilterFirst() throws IOException {
109116

110117
@Test
111118
public void testInputSize2FilterSize2FilterLast() throws IOException {
112-
final StringReader input = new StringReader("ab");
119+
final StringReader input = new StringReader(STRING_FIXTURE);
113120
final HashSet<Integer> codePoints = new HashSet<>();
114121
codePoints.add(Integer.valueOf('x'));
115122
codePoints.add(Integer.valueOf('b'));
@@ -121,7 +128,7 @@ public void testInputSize2FilterSize2FilterLast() throws IOException {
121128

122129
@Test
123130
public void testInputSize2FilterSize2FilterNone() throws IOException {
124-
final StringReader input = new StringReader("ab");
131+
final StringReader input = new StringReader(STRING_FIXTURE);
125132
final HashSet<Integer> codePoints = new HashSet<>();
126133
codePoints.add(Integer.valueOf('x'));
127134
codePoints.add(Integer.valueOf('y'));
@@ -130,4 +137,19 @@ public void testInputSize2FilterSize2FilterNone() throws IOException {
130137
assertEquals('b', reader.read());
131138
}
132139
}
140+
141+
@Test
142+
public void testReadFilteringEOF() throws IOException {
143+
final StringReader input = new StringReader(STRING_FIXTURE);
144+
assertTimeoutPreemptively(Duration.ofMillis(500), () -> {
145+
try (StringBuilderWriter output = new StringBuilderWriter();
146+
CharacterSetFilterReader reader = new CharacterSetFilterReader(input, EOF)) {
147+
int c;
148+
while ((c = reader.read()) != EOF) {
149+
output.write(c);
150+
}
151+
assertEquals(STRING_FIXTURE, output.toString());
152+
}
153+
});
154+
}
133155
}

0 commit comments

Comments
 (0)