Well spotted Peter. The change looks good though I wonder if it should
be:
int length = (int)((elements + elements / 20) / loadFactor) + 3;
FYI, regarding Daniel's suggestion: When similar invariant checks were
added to the HashMap deserialization method we found code which relied
upon the illegal values. In some cases the serialized HashMaps were
created by alternative serialization implementations which included
illegal, but until the checks were added, "harmless" values.
The invariant checks should still be added though. It might even be
worth adding checks that the other deserialized values are in valid
ranges.
Mike
On 2015-01-05 07:48, [email protected] wrote:
Today's Topics:
2. Re: RFR: JDK-8068427 Hashtable deserialization reconstitutes
table with wrong capacity (Daniel Fuchs)
Message: 2
Date: Mon, 05 Jan 2015 15:52:55 +0100
From: Daniel Fuchs <[email protected]>
To: Peter Levart <[email protected]>, core-libs-dev
<[email protected]>
Subject: Re: RFR: JDK-8068427 Hashtable deserialization reconstitutes
table with wrong capacity
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
On 04/01/15 18:58, Peter Levart wrote:
Hi,
While investigating the following issue:
https://bugs.openjdk.java.net/browse/JDK-8029891
I noticed there's a bug in deserialization code of java.util.Hashtable
(from day one probably):
https://bugs.openjdk.java.net/browse/JDK-8068427
The fix is a trivial one-character replacement: '*' -> '/', but I also
corrected some untruthful comments in the neighbourhood (which might
have been true from day one, but are not any more):
http://cr.openjdk.java.net/~plevart/jdk9-dev/Hashtable.8068427/webrev.01/
Hi Peter,
I wonder whether there should be a guard against loadFactor being
zero/negative/NaN after line 1173, like in the constructor e.g. as
in lines
188 if (loadFactor <= 0 || Float.isNaN(loadFactor))
189 throw new IllegalArgumentException("Illegal Load:
"+loadFactor);
if only to avoid division by zero.
best regards,
-- daniel
Regards, Peter
