Skip to content

Commit 3b077b8

Browse files
Alexei Voitylovgnu-andrew
Alexei Voitylov
authored and
gnu-andrew
committed
8331446: Improve deserialization support
Reviewed-by: mbalao, andrew Backport-of: 8e4a392832f83e16d521024505b52c96d0a993f2
1 parent 1650221 commit 3b077b8

File tree

3 files changed

+243
-10
lines changed

3 files changed

+243
-10
lines changed

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

Lines changed: 49 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;
@@ -960,6 +961,8 @@ public Object[] parse(String source, ParsePosition pos) {
960961
maximumArgumentNumber = argumentNumbers[i];
961962
}
962963
}
964+
965+
// Constructors/applyPattern ensure that resultArray.length < MAX_ARGUMENT_INDEX
963966
Object[] resultArray = new Object[maximumArgumentNumber + 1];
964967

965968
int patternOffset = 0;
@@ -1210,6 +1213,9 @@ protected Object readResolve() throws InvalidObjectException {
12101213
* @serial
12111214
*/
12121215
private int[] argumentNumbers = new int[INITIAL_FORMATS];
1216+
// Implementation limit for ArgumentIndex pattern element. Valid indices must
1217+
// be less than this value
1218+
private static final int MAX_ARGUMENT_INDEX = 10000;
12131219

12141220
/**
12151221
* One less than the number of entries in <code>offsets</code>. Can also be thought of
@@ -1434,6 +1440,11 @@ private void makeFormat(int position, int offsetNumber,
14341440
+ argumentNumber);
14351441
}
14361442

1443+
if (argumentNumber >= MAX_ARGUMENT_INDEX) {
1444+
throw new IllegalArgumentException(
1445+
argumentNumber + " exceeds the ArgumentIndex implementation limit");
1446+
}
1447+
14371448
// resize format information arrays if necessary
14381449
if (offsetNumber >= formats.length) {
14391450
int newLength = formats.length * 2;
@@ -1580,24 +1591,52 @@ private static final void copyAndFixQuotes(String source, int start, int end,
15801591
* @throws InvalidObjectException if the objects read from the stream is invalid.
15811592
*/
15821593
private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException {
1583-
in.defaultReadObject();
1584-
boolean isValid = maxOffset >= -1
1585-
&& formats.length > maxOffset
1586-
&& offsets.length > maxOffset
1587-
&& argumentNumbers.length > maxOffset;
1594+
ObjectInputStream.GetField fields = in.readFields();
1595+
if (fields.defaulted("argumentNumbers") || fields.defaulted("offsets")
1596+
|| fields.defaulted("formats") || fields.defaulted("locale")
1597+
|| fields.defaulted("pattern") || fields.defaulted("maxOffset")){
1598+
throw new InvalidObjectException("Stream has missing data");
1599+
}
1600+
1601+
locale = (Locale) fields.get("locale", null);
1602+
String patt = (String) fields.get("pattern", null);
1603+
int maxOff = fields.get("maxOffset", -2);
1604+
int[] argNums = ((int[]) fields.get("argumentNumbers", null)).clone();
1605+
int[] offs = ((int[]) fields.get("offsets", null)).clone();
1606+
Format[] fmts = ((Format[]) fields.get("formats", null)).clone();
1607+
1608+
// Check arrays/maxOffset have correct value/length
1609+
boolean isValid = maxOff >= -1 && argNums.length > maxOff
1610+
&& offs.length > maxOff && fmts.length > maxOff;
1611+
1612+
// Check the correctness of arguments and offsets
15881613
if (isValid) {
1589-
int lastOffset = pattern.length() + 1;
1590-
for (int i = maxOffset; i >= 0; --i) {
1591-
if ((offsets[i] < 0) || (offsets[i] > lastOffset)) {
1614+
int lastOffset = patt.length() + 1;
1615+
for (int i = maxOff; i >= 0; --i) {
1616+
if (argNums[i] < 0 || argNums[i] >= MAX_ARGUMENT_INDEX
1617+
|| offs[i] < 0 || offs[i] > lastOffset) {
15921618
isValid = false;
15931619
break;
15941620
} else {
1595-
lastOffset = offsets[i];
1621+
lastOffset = offs[i];
15961622
}
15971623
}
15981624
}
1625+
15991626
if (!isValid) {
1600-
throw new InvalidObjectException("Could not reconstruct MessageFormat from corrupt stream.");
1627+
throw new InvalidObjectException("Stream has invalid data");
16011628
}
1629+
maxOffset = maxOff;
1630+
pattern = patt;
1631+
offsets = offs;
1632+
formats = fmts;
1633+
argumentNumbers = argNums;
1634+
}
1635+
1636+
/**
1637+
* Serialization without data not supported for this class.
1638+
*/
1639+
private void readObjectNoData() throws ObjectStreamException {
1640+
throw new InvalidObjectException("Deserialized MessageFormat objects need data");
16021641
}
16031642
}

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

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
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 testng MaxArgumentIndexTest
32+
*/
33+
34+
import org.testng.annotations.Test;
35+
36+
import java.text.MessageFormat;
37+
import java.util.Locale;
38+
import java.util.stream.Stream;
39+
40+
import static org.testng.Assert.assertThrows;
41+
42+
public class MaxArgumentIndexTest {
43+
44+
// A MessageFormat pattern that contains an ArgumentIndex value
45+
// which violates this implementation's limit: MAX_ARGUMENT_INDEX(10,000)
46+
// As this check is exclusive, 10,000 will violate the limit
47+
private static final String VIOLATES_MAX_ARGUMENT_INDEX = "{10000}";
48+
49+
// Check String constructor enforces the limit
50+
@Test
51+
public void constructorTest() {
52+
assertThrows(IllegalArgumentException.class,
53+
() -> new MessageFormat(VIOLATES_MAX_ARGUMENT_INDEX));
54+
}
55+
56+
// Check String, Locale constructor enforces the limit
57+
public static void constructorWithLocaleTest(Locale locale) {
58+
assertThrows(IllegalArgumentException.class,
59+
() -> new MessageFormat(VIOLATES_MAX_ARGUMENT_INDEX, locale));
60+
}
61+
62+
// Provide some basic common locale values
63+
@Test
64+
public static void constructorWithLocaleTest() {
65+
Stream.of(null, Locale.US, Locale.ROOT).forEach(MaxArgumentIndexTest::constructorWithLocaleTest);
66+
}
67+
68+
// Edge case: Test a locale dependent subformat (with null locale) with a
69+
// violating ArgumentIndex. In this instance, the violating ArgumentIndex
70+
// will be caught and IAE thrown instead of the NPE
71+
@Test
72+
public void localeDependentSubFormatTest() {
73+
assertThrows(IllegalArgumentException.class,
74+
() -> new MessageFormat("{10000,number,short}", null));
75+
// For reference
76+
assertThrows(NullPointerException.class,
77+
() -> new MessageFormat("{999,number,short}", null));
78+
}
79+
80+
// Check that the static format method enforces the limit
81+
@Test
82+
public void staticFormatTest() {
83+
assertThrows(IllegalArgumentException.class,
84+
() -> MessageFormat.format(VIOLATES_MAX_ARGUMENT_INDEX, new Object[]{1}));
85+
}
86+
87+
// Check that applyPattern(String) enforces the limit
88+
@Test
89+
public void applyPatternTest() {
90+
MessageFormat mf = new MessageFormat("");
91+
assertThrows(IllegalArgumentException.class,
92+
() -> mf.applyPattern(VIOLATES_MAX_ARGUMENT_INDEX));
93+
}
94+
}

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

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
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 testng SerializationTest
29+
*/
30+
31+
import org.testng.annotations.Test;
32+
33+
import java.io.ByteArrayInputStream;
34+
import java.io.ByteArrayOutputStream;
35+
import java.io.IOException;
36+
import java.io.ObjectInputStream;
37+
import java.io.ObjectOutputStream;
38+
import java.text.MessageFormat;
39+
import java.util.Locale;
40+
import java.util.stream.Stream;
41+
42+
import static org.testng.Assert.assertEquals;
43+
44+
public class SerializationTest {
45+
46+
// Ensure basic correctness of serialization round trip
47+
public void serializationRoundTrip(MessageFormat expectedMf)
48+
throws IOException, ClassNotFoundException {
49+
byte[] bytes = ser(expectedMf);
50+
MessageFormat actualMf = (MessageFormat) deSer(bytes);
51+
assertEquals(expectedMf, actualMf);
52+
}
53+
54+
// Various valid MessageFormats
55+
@Test
56+
public void serializationRoundTrip() {
57+
Stream.of(
58+
// basic pattern
59+
new MessageFormat("{0} foo"),
60+
// Multiple arguments
61+
new MessageFormat("{0} {1} foo"),
62+
// duplicate arguments
63+
new MessageFormat("{0} {0} {1} foo"),
64+
// Non-ascending arguments
65+
new MessageFormat("{1} {0} foo"),
66+
// With locale
67+
new MessageFormat("{1} {0} foo", Locale.UK),
68+
// With null locale. (NPE not thrown, if no format defined)
69+
new MessageFormat("{1} {0} foo", null),
70+
// With formats
71+
new MessageFormat("{0,number,short} {0} {1,date,long} foo")
72+
).forEach(format -> {
73+
try {
74+
serializationRoundTrip(format);
75+
} catch (IOException | ClassNotFoundException e) {
76+
throw new RuntimeException(e);
77+
}
78+
});
79+
}
80+
81+
// Utility method to serialize
82+
private static byte[] ser(Object obj) throws IOException {
83+
ByteArrayOutputStream byteArrayOutputStream = new
84+
ByteArrayOutputStream();
85+
ObjectOutputStream oos = new
86+
ObjectOutputStream(byteArrayOutputStream);
87+
oos.writeObject(obj);
88+
return byteArrayOutputStream.toByteArray();
89+
}
90+
91+
// Utility method to deserialize
92+
private static Object deSer(byte[] bytes) throws
93+
IOException, ClassNotFoundException {
94+
ByteArrayInputStream byteArrayInputStream = new
95+
ByteArrayInputStream(bytes);
96+
ObjectInputStream ois = new
97+
ObjectInputStream(byteArrayInputStream);
98+
return ois.readObject();
99+
}
100+
}

0 commit comments

Comments
 (0)