RFR: JDK-8068427 Hashtable deserialization reconstitutes table with wrong capacity

[Mike Duigou]

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, core-libs-dev-request at openjdk.java.net 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 <daniel.fuchs at oracle.com>
> To: Peter Levart <peter.levart at gmail.com>,	core-libs-dev
> 	<core-libs-dev at openjdk.java.net>
> Subject: Re: RFR: JDK-8068427 Hashtable deserialization reconstitutes
> 	table	with wrong capacity
> Message-ID: <54AAA547.8070804 at oracle.com>
> 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
>> 
>