RFR: JDK-8068427 Hashtable deserialization reconstitutes table with wrong capacity
Peter Levart
peter.levart at gmail.com
Wed Jan 21 13:04:31 UTC 2015
Hi Martin and others,
I have also modified the test per Martin's suggestion to factor out the
serialClone() method. Here's the latest webrev:
http://cr.openjdk.java.net/~plevart/jdk9-dev/Hashtable.8068427/webrev.03/
Is this ok now?
Regards, Peter
On 01/09/2015 11:16 AM, Peter Levart wrote:
> On 01/05/2015 05:52 PM, Mike Duigou wrote:
>> 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
>
> Hi Mike and others,
>
> Yes, your suggested length computation is more in "spirit" with the
> comment above that states: "Compute new length with a bit of room 5%
> to grow...", since it takes loadFactor into account for that
> additional 5% too. I changed it to your suggested expression.
>
> Here's new webrev:
>
> http://cr.openjdk.java.net/~plevart/jdk9-dev/Hashtable.8068427/webrev.02/
>
> In addition to valid loadFactor, # of elements is checked to be
> non-negative. The original length is just "repaired" if it appears to
> be less than the enforced auto-growth invariant of Hashtable.
>
> I also expanded the test to exercise more combinations of # of
> elements and loadFactor. Here's what gets printed with current
> Hashtable implementation:
>
> ser. deser.
> size load lentgh length valid range ok?
> ------- ----- ------- ------- ----------------- ------
> 10 0.15 127 4 67... 134 NOT-OK
> 10 0.50 31 8 21... 42 NOT-OK
> 10 0.75 15 10 14... 28 NOT-OK
> 10 1.00 15 13 11... 22 OK
> 10 2.50 7 7 5... 10 OK
> 50 0.15 511 12 334... 668 NOT-OK
> 50 0.50 127 30 101... 202 NOT-OK
> 50 0.75 127 42 67... 134 NOT-OK
> 50 1.00 63 55 51... 102 OK
> 50 2.50 31 31 21... 42 OK
> 500 0.15 4095 103 3334... 6668 NOT-OK
> 500 0.50 1023 278 1001... 2002 NOT-OK
> 500 0.75 1023 403 667... 1334 NOT-OK
> 500 1.00 511 511 501... 1002 OK
> 500 2.50 255 255 201... 402 OK
> 5000 0.15 65535 1003 33334... 66668 NOT-OK
> 5000 0.50 16383 2753 10001... 20002 NOT-OK
> 5000 0.75 8191 4003 6667... 13334 NOT-OK
> 5000 1.00 8191 5253 5001... 10002 OK
> 5000 2.50 2047 2047 2001... 4002 OK
>
>
> With patched Hashtable, the test passes:
>
> ser. deser.
> size load lentgh length valid range ok?
> ------- ----- ------- ------- ----------------- ------
> 10 0.15 127 69 67... 134 OK
> 10 0.50 31 23 21... 42 OK
> 10 0.75 15 15 14... 28 OK
> 10 1.00 15 13 11... 22 OK
> 10 2.50 7 7 5... 10 OK
> 50 0.15 511 349 334... 668 OK
> 50 0.50 127 107 101... 202 OK
> 50 0.75 127 71 67... 134 OK
> 50 1.00 63 55 51... 102 OK
> 50 2.50 31 23 21... 42 OK
> 500 0.15 4095 3501 3334... 6668 OK
> 500 0.50 1023 1023 1001... 2002 OK
> 500 0.75 1023 703 667... 1334 OK
> 500 1.00 511 511 501... 1002 OK
> 500 2.50 255 213 201... 402 OK
> 5000 0.15 65535 35003 33334... 66668 OK
> 5000 0.50 16383 10503 10001... 20002 OK
> 5000 0.75 8191 7003 6667... 13334 OK
> 5000 1.00 8191 5253 5001... 10002 OK
> 5000 2.50 2047 2047 2001... 4002 OK
>
>
>
> Regards, Peter
>
>>
>> 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
>>>>
>>>
>>
>
More information about the core-libs-dev
mailing list