Skip to content

Commit 983d0c7

Browse files
committed
A serialized Fraction can't store a bad cached hashCode.
1 parent 23f7b31 commit 983d0c7

5 files changed

Lines changed: 105 additions & 7 deletions

File tree

src/main/java/org/apache/commons/lang3/Range.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -535,11 +535,13 @@ public boolean isStartedBy(final T element) {
535535
}
536536

537537
/**
538-
* See {@link Serializable}.
538+
* Validates the cached hashCode after deserialization. Throws a {@link InvalidObjectException} when the stored hashCode does not match the canonical hash
539+
* of the deserialized minimum/maximum.
539540
*
540541
* @param in See {@link Serializable}.
541-
* @throws IOException See {@link Serializable}.
542+
* @throws IOException See {@link Serializable}.
542543
* @throws ClassNotFoundException See {@link Serializable}.
544+
* @throws InvalidObjectException If the hashCode doesn't match the minimum and maximum.
543545
*/
544546
private void readObject(final ObjectInputStream in) throws IOException, ClassNotFoundException {
545547
in.defaultReadObject();

src/main/java/org/apache/commons/lang3/math/Fraction.java

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616
*/
1717
package org.apache.commons.lang3.math;
1818

19+
import java.io.IOException;
20+
import java.io.InvalidObjectException;
21+
import java.io.ObjectInputStream;
1922
import java.io.Serializable;
2023
import java.math.BigInteger;
2124
import java.util.Objects;
@@ -402,6 +405,10 @@ private static int greatestCommonDivisor(int u, int v) {
402405
return -u * (1 << k); // gcd is u*2^k
403406
}
404407

408+
private static int hash(final int value1, final int value2) {
409+
return Objects.hash(value1, value2);
410+
}
411+
405412
/**
406413
* Multiplies two integers, checking for overflow.
407414
*
@@ -489,7 +496,7 @@ private static int subAndCheck(final int x, final int y) {
489496
private Fraction(final int numerator, final int denominator) {
490497
this.numerator = numerator;
491498
this.denominator = denominator;
492-
this.hashCode = Objects.hash(denominator, numerator);
499+
this.hashCode = hash(denominator, numerator);
493500
}
494501

495502
/**
@@ -837,6 +844,22 @@ public Fraction pow(final int power) {
837844
return f.pow(power / 2).multiplyBy(this);
838845
}
839846

847+
/**
848+
* Validates the cached hashCode after deserialization. Throws a {@link InvalidObjectException} when the stored hashCode does not match the canonical hash
849+
* of the deserialized numerator/denominator.
850+
*
851+
* @param in See {@link Serializable}.
852+
* @throws IOException See {@link Serializable}.
853+
* @throws ClassNotFoundException See {@link Serializable}.
854+
* @throws InvalidObjectException If the hashCode doesn't match the denominator and numerator.
855+
*/
856+
private void readObject(final ObjectInputStream in) throws IOException, ClassNotFoundException {
857+
in.defaultReadObject();
858+
if (hashCode != hash(denominator, numerator)) {
859+
throw new InvalidObjectException("Fraction hashCode does not match numerator/denominator.");
860+
}
861+
}
862+
840863
/**
841864
* Reduce the fraction to the smallest values for the numerator and denominator, returning the result.
842865
* <p>

src/test/java/org/apache/commons/lang3/RangeReadObjectTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
import org.junit.jupiter.api.Test;
2828

2929
/**
30-
* Tests that a serialized Range can't store a bad cached hashCode.
30+
* Tests that a serialized {@link Range} can't store a bad cached hashCode.
3131
*/
3232
class RangeReadObjectTest {
3333

src/test/java/org/apache/commons/lang3/SerializationUtilsTest.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,15 +61,17 @@ interface SerializableSupplier<T> extends Supplier<T>, Serializable {
6161
/**
6262
* Tests {@link SerializationUtils}.
6363
*/
64-
class SerializationUtilsTest extends AbstractLangTest {
64+
public class SerializationUtilsTest extends AbstractLangTest {
6565

6666
static final String CLASS_NOT_FOUND_MESSAGE = "ClassNotFoundSerialization.readObject fake exception";
67+
6768
protected static final String SERIALIZE_IO_EXCEPTION_MESSAGE = "Anonymous OutputStream I/O exception";
6869

69-
static byte[] intToBytes(final int v) {
70+
public static byte[] intToBytes(final int v) {
7071
return new byte[] { (byte) (v >>> 24), (byte) (v >>> 16), (byte) (v >>> 8), (byte) v };
7172
}
72-
static byte[] replaceLastInt(final byte[] src, final int from, final int to) {
73+
74+
public static byte[] replaceLastInt(final byte[] src, final int from, final int to) {
7375
final byte[] fromB = intToBytes(from);
7476
final byte[] toB = intToBytes(to);
7577
final byte[] out = src.clone();
@@ -85,6 +87,7 @@ static byte[] replaceLastInt(final byte[] src, final int from, final int to) {
8587
fail("No legitimate int in stream, serialization must keep hashCode in default field set");
8688
return null;
8789
}
90+
8891
private String iString;
8992

9093
private Integer iInteger;
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* https://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package org.apache.commons.lang3.math;
19+
20+
import static org.junit.jupiter.api.Assertions.assertEquals;
21+
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
22+
import static org.junit.jupiter.api.Assertions.assertThrows;
23+
24+
import java.io.InvalidObjectException;
25+
import java.util.HashMap;
26+
import java.util.Map;
27+
28+
import org.apache.commons.lang3.SerializationException;
29+
import org.apache.commons.lang3.SerializationUtils;
30+
import org.apache.commons.lang3.SerializationUtilsTest;
31+
import org.apache.commons.lang3.reflect.FieldUtils;
32+
import org.junit.jupiter.api.Test;
33+
34+
/**
35+
* Tests that a serialized {@link Fraction} can't store a bad cached hashCode.
36+
*/
37+
public class FractionReadObjectTest {
38+
39+
@Test
40+
public void testBadHashCodeStreamIsRejected() throws Exception {
41+
final Fraction fraction = Fraction.getFraction(3, 7);
42+
final byte[] bytes = SerializationUtils.serialize(fraction);
43+
final int hashCode = (Integer) FieldUtils.readDeclaredField(fraction, "hashCode", true);
44+
final byte[] edited = SerializationUtilsTest.replaceLastInt(bytes, hashCode, 0xCAFEBABE);
45+
final SerializationException ex = assertThrows(SerializationException.class, () -> SerializationUtils.deserialize(edited),
46+
"Bad hashCode in stream must be rejected with InvalidObjectException");
47+
assertInstanceOf(InvalidObjectException.class, ex.getCause());
48+
assertEquals("java.io.InvalidObjectException: Fraction hashCode does not match numerator/denominator.", ex.getMessage());
49+
50+
}
51+
52+
@Test
53+
public void testHashMapLookupAfterRoundTrip() throws Exception {
54+
final Fraction fraction = Fraction.getFraction(1, 4);
55+
final byte[] bytes = SerializationUtils.serialize(fraction);
56+
final Fraction deserialized = SerializationUtils.deserialize(bytes);
57+
final Map<Fraction, String> map = new HashMap<>();
58+
map.put(fraction, "quarter");
59+
assertEquals("quarter", map.get(deserialized), "HashMap lookup must work after deserialization");
60+
}
61+
62+
@Test
63+
public void testRoundTripPreservesHashCode() throws Exception {
64+
final Fraction fraction = Fraction.getFraction(1, 4);
65+
final Fraction roundtrip = SerializationUtils.roundtrip(fraction);
66+
assertEquals(fraction.hashCode(), roundtrip.hashCode(), "Round-trip serialization must preserve the correct hashCode");
67+
assertEquals(fraction, roundtrip);
68+
69+
}
70+
}

0 commit comments

Comments
 (0)