Skip to content

Commit 5010e3f

Browse files
Alexei VoitylovRealCLanger
Alexei Voitylov
authored and
RealCLanger
committed
8331446: Improve deserialization support
Reviewed-by: yan, mbalao, andrew Backport-of: 8e4a392832f83e16d521024505b52c96d0a993f2
1 parent feed7d8 commit 5010e3f

File tree

3 files changed

+243
-10
lines changed

3 files changed

+243
-10
lines changed

‎src/java.base/share/classes/java/text/MessageFormat.java‎

Lines changed: 50 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import java.io.InvalidObjectException;
4242
import java.io.IOException;
4343
import java.io.ObjectInputStream;
44+
import java.io.ObjectStreamException;
4445
import java.text.DecimalFormat;
4546
import java.util.ArrayList;
4647
import java.util.Arrays;
@@ -983,6 +984,8 @@ public Object[] parse(String source, ParsePosition pos) {
983984
maximumArgumentNumber = argumentNumbers[i];
984985
}
985986
}
987+
988+
// Constructors/applyPattern ensure that resultArray.length < MAX_ARGUMENT_INDEX
986989
Object[] resultArray = new Object[maximumArgumentNumber + 1];
987990

988991
int patternOffset = 0;
@@ -1235,6 +1238,9 @@ protected Object readResolve() throws InvalidObjectException {
12351238
* @serial
12361239
*/
12371240
private int[] argumentNumbers = new int[INITIAL_FORMATS];
1241+
// Implementation limit for ArgumentIndex pattern element. Valid indices must
1242+
// be less than this value
1243+
private static final int MAX_ARGUMENT_INDEX = 10000;
12381244

12391245
/**
12401246
* One less than the number of entries in {@code offsets}. Can also be thought of
@@ -1459,6 +1465,11 @@ private void makeFormat(int position, int offsetNumber,
14591465
+ argumentNumber);
14601466
}
14611467

1468+
if (argumentNumber >= MAX_ARGUMENT_INDEX) {
1469+
throw new IllegalArgumentException(
1470+
argumentNumber + " exceeds the ArgumentIndex implementation limit");
1471+
}
1472+
14621473
// resize format information arrays if necessary
14631474
if (offsetNumber >= formats.length) {
14641475
int newLength = formats.length * 2;
@@ -1606,24 +1617,53 @@ private static final void copyAndFixQuotes(String source, int start, int end,
16061617
*/
16071618
@java.io.Serial
16081619
private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException {
1609-
in.defaultReadObject();
1610-
boolean isValid = maxOffset >= -1
1611-
&& formats.length > maxOffset
1612-
&& offsets.length > maxOffset
1613-
&& argumentNumbers.length > maxOffset;
1620+
ObjectInputStream.GetField fields = in.readFields();
1621+
if (fields.defaulted("argumentNumbers") || fields.defaulted("offsets")
1622+
|| fields.defaulted("formats") || fields.defaulted("locale")
1623+
|| fields.defaulted("pattern") || fields.defaulted("maxOffset")){
1624+
throw new InvalidObjectException("Stream has missing data");
1625+
}
1626+
1627+
locale = (Locale) fields.get("locale", null);
1628+
String patt = (String) fields.get("pattern", null);
1629+
int maxOff = fields.get("maxOffset", -2);
1630+
int[] argNums = ((int[]) fields.get("argumentNumbers", null)).clone();
1631+
int[] offs = ((int[]) fields.get("offsets", null)).clone();
1632+
Format[] fmts = ((Format[]) fields.get("formats", null)).clone();
1633+
1634+
// Check arrays/maxOffset have correct value/length
1635+
boolean isValid = maxOff >= -1 && argNums.length > maxOff
1636+
&& offs.length > maxOff && fmts.length > maxOff;
1637+
1638+
// Check the correctness of arguments and offsets
16141639
if (isValid) {
1615-
int lastOffset = pattern.length() + 1;
1616-
for (int i = maxOffset; i >= 0; --i) {
1617-
if ((offsets[i] < 0) || (offsets[i] > lastOffset)) {
1640+
int lastOffset = patt.length() + 1;
1641+
for (int i = maxOff; i >= 0; --i) {
1642+
if (argNums[i] < 0 || argNums[i] >= MAX_ARGUMENT_INDEX
1643+
|| offs[i] < 0 || offs[i] > lastOffset) {
16181644
isValid = false;
16191645
break;
16201646
} else {
1621-
lastOffset = offsets[i];
1647+
lastOffset = offs[i];
16221648
}
16231649
}
16241650
}
1651+
16251652
if (!isValid) {
1626-
throw new InvalidObjectException("Could not reconstruct MessageFormat from corrupt stream.");
1653+
throw new InvalidObjectException("Stream has invalid data");
16271654
}
1655+
maxOffset = maxOff;
1656+
pattern = patt;
1657+
offsets = offs;
1658+
formats = fmts;
1659+
argumentNumbers = argNums;
1660+
}
1661+
1662+
/**
1663+
* Serialization without data not supported for this class.
1664+
*/
1665+
@java.io.Serial
1666+
private void readObjectNoData() throws ObjectStreamException {
1667+
throw new InvalidObjectException("Deserialized MessageFormat objects need data");
16281668
}
16291669
}

‎test/jdk/java/text/Format/MessageFormat/MaxArgumentIndexTest.java‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
/*
2+
* Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
/*
25+
* @test
26+
* @bug 8331446
27+
* @summary Enforce the MAX_ARGUMENT_INDEX(10,000) implementation limit for the
28+
* ArgumentIndex element in the MessageFormat pattern syntax. This
29+
* should be checked during construction/applyPattern/readObject and should effectively
30+
* prevent parse/format from being invoked with values over the limit.
31+
* @run junit MaxArgumentIndexTest
32+
*/
33+
34+
import org.junit.jupiter.api.Test;
35+
import org.junit.jupiter.params.ParameterizedTest;
36+
import org.junit.jupiter.params.provider.MethodSource;
37+
38+
import java.text.MessageFormat;
39+
import java.util.Locale;
40+
import java.util.stream.Stream;
41+
42+
import static org.junit.jupiter.api.Assertions.assertThrows;
43+
44+
public class MaxArgumentIndexTest {
45+
46+
// A MessageFormat pattern that contains an ArgumentIndex value
47+
// which violates this implementation's limit: MAX_ARGUMENT_INDEX(10,000)
48+
// As this check is exclusive, 10,000 will violate the limit
49+
private static final String VIOLATES_MAX_ARGUMENT_INDEX = "{10000}";
50+
51+
// Check String constructor enforces the limit
52+
@Test
53+
public void constructorTest() {
54+
assertThrows(IllegalArgumentException.class,
55+
() -> new MessageFormat(VIOLATES_MAX_ARGUMENT_INDEX));
56+
}
57+
58+
// Check String, Locale constructor enforces the limit
59+
@ParameterizedTest
60+
@MethodSource
61+
public void constructorWithLocaleTest(Locale locale) {
62+
assertThrows(IllegalArgumentException.class,
63+
() -> new MessageFormat(VIOLATES_MAX_ARGUMENT_INDEX, locale));
64+
}
65+
66+
// Provide some basic common locale values
67+
private static Stream<Locale> constructorWithLocaleTest() {
68+
return Stream.of(null, Locale.US, Locale.ROOT);
69+
}
70+
71+
// Edge case: Test a locale dependent subformat (with null locale) with a
72+
// violating ArgumentIndex. In this instance, the violating ArgumentIndex
73+
// will be caught and IAE thrown instead of the NPE
74+
@Test
75+
public void localeDependentSubFormatTest() {
76+
assertThrows(IllegalArgumentException.class,
77+
() -> new MessageFormat("{10000,number,short}", null));
78+
// For reference
79+
assertThrows(NullPointerException.class,
80+
() -> new MessageFormat("{999,number,short}", null));
81+
}
82+
83+
// Check that the static format method enforces the limit
84+
@Test
85+
public void staticFormatTest() {
86+
assertThrows(IllegalArgumentException.class,
87+
() -> MessageFormat.format(VIOLATES_MAX_ARGUMENT_INDEX, new Object[]{1}));
88+
}
89+
90+
// Check that applyPattern(String) enforces the limit
91+
@Test
92+
public void applyPatternTest() {
93+
MessageFormat mf = new MessageFormat("");
94+
assertThrows(IllegalArgumentException.class,
95+
() -> mf.applyPattern(VIOLATES_MAX_ARGUMENT_INDEX));
96+
}
97+
}

‎test/jdk/java/text/Format/MessageFormat/SerializationTest.java‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
/*
2+
* Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
/*
25+
* @test
26+
* @bug 8331446
27+
* @summary Check correctness of deserialization
28+
* @run junit SerializationTest
29+
*/
30+
31+
import org.junit.jupiter.params.ParameterizedTest;
32+
import org.junit.jupiter.params.provider.MethodSource;
33+
34+
import java.io.ByteArrayInputStream;
35+
import java.io.ByteArrayOutputStream;
36+
import java.io.IOException;
37+
import java.io.ObjectInputStream;
38+
import java.io.ObjectOutputStream;
39+
import java.text.MessageFormat;
40+
import java.util.Locale;
41+
import java.util.stream.Stream;
42+
43+
import static org.junit.jupiter.api.Assertions.assertEquals;
44+
45+
public class SerializationTest {
46+
47+
// Ensure basic correctness of serialization round trip
48+
@ParameterizedTest
49+
@MethodSource
50+
public void serializationRoundTrip(MessageFormat expectedMf)
51+
throws IOException, ClassNotFoundException {
52+
byte[] bytes = ser(expectedMf);
53+
MessageFormat actualMf = (MessageFormat) deSer(bytes);
54+
assertEquals(expectedMf, actualMf);
55+
}
56+
57+
// Various valid MessageFormats
58+
private static Stream<MessageFormat> serializationRoundTrip() {
59+
return Stream.of(
60+
// basic pattern
61+
new MessageFormat("{0} foo"),
62+
// Multiple arguments
63+
new MessageFormat("{0} {1} foo"),
64+
// duplicate arguments
65+
new MessageFormat("{0} {0} {1} foo"),
66+
// Non-ascending arguments
67+
new MessageFormat("{1} {0} foo"),
68+
// With locale
69+
new MessageFormat("{1} {0} foo", Locale.UK),
70+
// With null locale. (NPE not thrown, if no format defined)
71+
new MessageFormat("{1} {0} foo", null),
72+
// With formats
73+
new MessageFormat("{0,number,short} {0} {1,date,long} foo")
74+
);
75+
}
76+
77+
// Utility method to serialize
78+
private static byte[] ser(Object obj) throws IOException {
79+
ByteArrayOutputStream byteArrayOutputStream = new
80+
ByteArrayOutputStream();
81+
ObjectOutputStream oos = new
82+
ObjectOutputStream(byteArrayOutputStream);
83+
oos.writeObject(obj);
84+
return byteArrayOutputStream.toByteArray();
85+
}
86+
87+
// Utility method to deserialize
88+
private static Object deSer(byte[] bytes) throws
89+
IOException, ClassNotFoundException {
90+
ByteArrayInputStream byteArrayInputStream = new
91+
ByteArrayInputStream(bytes);
92+
ObjectInputStream ois = new
93+
ObjectInputStream(byteArrayInputStream);
94+
return ois.readObject();
95+
}
96+
}

0 commit comments

Comments
 (0)