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-requ...@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.fu...@oracle.com>
To: Peter Levart <peter.lev...@gmail.com>,        core-libs-dev
        <core-libs-dev@openjdk.java.net>
Subject: Re: RFR: JDK-8068427 Hashtable deserialization reconstitutes
        table   with wrong capacity
Message-ID: <54aaa547.8070...@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



Reply via email to