DO NOT REPLY [Bug 17139] New: - StaticBucketMap.remove(Object) incorrectly handles key comparison

[bu...@apache.org]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://nagoya.apache.org/bugzilla/show_bug.cgi?id=17139>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://nagoya.apache.org/bugzilla/show_bug.cgi?id=17139

StaticBucketMap.remove(Object) incorrectly handles key comparison

           Summary: StaticBucketMap.remove(Object) incorrectly handles key
                    comparison
           Product: Commons
           Version: 2.1 Final
          Platform: Other
        OS/Version: Other
            Status: NEW
          Severity: Normal
          Priority: Other
         Component: Collections
        AssignedTo: commons-dev@jakarta.apache.org
        ReportedBy: leo.sutic@inspireinfrastructure.com


I've found a bug in the StaticBucketMap class, the remove(Object)
method:

    /**
     *  Implements {@link Map#remove(Object)}.
     */
    public Object remove( Object key )
    {
        int hash = getHash( key );

        synchronized( m_locks[ hash ] )
        {
            Node n = m_buckets[ hash ];
            Node prev = null;

            while( n != null )
            {
HERE>>>>>>>>>   if( n.key == null || ( n.key != null && n.key.equals(
key ) ) )  <<<<<<<<<<<<<<<<<<
                {
                    // Remove this node from the linked list of nodes.
                    if( null == prev )
                    {
                        // This node was the head, set the next node to
be the new head.
                        m_buckets[ hash ] = n.next;
                    }
                    else
                    {
                        // Set the next node of the previous node to be
the node after this one.
                        prev.next = n.next;
                    }
                    m_locks[hash].size--;
                    return n.value;
                }

                prev = n;
                n = n.next;
            }
        }

        return null;
    }

The test is:

                if( n.key == null || ( n.key != null && n.key.equals(
key ) ) )

should be:

                if( n.key == key || ( n.key != null && n.key.equals( key
) ) )

which is how it works in get(Object), containsKey(Object) etc. and which
is correct. We have a match if the keys match using == OR if they are
equal according to equals().

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org