Return-Path: Delivered-To: apmail-harmony-commits-archive@www.apache.org Received: (qmail 15211 invoked from network); 9 Dec 2009 01:01:27 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.3) by minotaur.apache.org with SMTP; 9 Dec 2009 01:01:27 -0000 Received: (qmail 72007 invoked by uid 500); 9 Dec 2009 01:01:27 -0000 Delivered-To: apmail-harmony-commits-archive@harmony.apache.org Received: (qmail 71945 invoked by uid 500); 9 Dec 2009 01:01:27 -0000 Mailing-List: contact commits-help@harmony.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@harmony.apache.org Delivered-To: mailing list commits@harmony.apache.org Received: (qmail 71936 invoked by uid 99); 9 Dec 2009 01:01:27 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 09 Dec 2009 01:01:27 +0000 X-ASF-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 09 Dec 2009 01:01:24 +0000 Received: by eris.apache.org (Postfix, from userid 65534) id 8B24223889DF; Wed, 9 Dec 2009 01:01:04 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r888668 - in /harmony/enhanced/classlib/trunk/modules/luni/src: main/java/java/util/ test/api/common/org/apache/harmony/luni/tests/java/util/ Date: Wed, 09 Dec 2009 01:01:04 -0000 To: commits@harmony.apache.org From: jessewilson@apache.org X-Mailer: svnmailer-1.0.8 Message-Id: <20091209010104.8B24223889DF@eris.apache.org> Author: jessewilson Date: Wed Dec 9 01:01:03 2009 New Revision: 888668 URL: http://svn.apache.org/viewvc?rev=888668&view=rev Log: Fix for HARMONY-6396; Null handling bugs in collections. Modified: harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/AbstractMap.java harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/AbstractSet.java harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/Collections.java harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/TreeMap.java harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/TreeSet.java harmony/enhanced/classlib/trunk/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/util/AbstractMapTest.java Modified: harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/AbstractMap.java URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/AbstractMap.java?rev=888668&r1=888667&r2=888668&view=diff ============================================================================== --- harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/AbstractMap.java (original) +++ harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/AbstractMap.java Wed Dec 9 01:01:03 2009 @@ -133,19 +133,22 @@ return false; } - Iterator> it = entrySet().iterator(); - try { - while (it.hasNext()) { - Entry entry = it.next(); + for (Entry entry : entrySet()) { K key = entry.getKey(); - V value = entry.getValue(); - Object obj = map.get(key); - if( null != obj && (!obj.equals(value)) || null == obj && obj != value) { + V mine = entry.getValue(); + Object theirs = map.get(key); + if (mine == null) { + if (theirs != null || !map.containsKey(key)) { + return false; + } + } else if (!mine.equals(theirs)) { return false; } } - } catch (ClassCastException cce) { + } catch (NullPointerException ignored) { + return false; + } catch (ClassCastException ignored) { return false; } return true; Modified: harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/AbstractSet.java URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/AbstractSet.java?rev=888668&r1=888667&r2=888668&view=diff ============================================================================== --- harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/AbstractSet.java (original) +++ harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/AbstractSet.java Wed Dec 9 01:01:03 2009 @@ -55,7 +55,9 @@ try { return size() == s.size() && containsAll(s); - } catch (ClassCastException cce) { + } catch (NullPointerException ignored) { + return false; + } catch (ClassCastException ignored) { return false; } } Modified: harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/Collections.java URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/Collections.java?rev=888668&r1=888667&r2=888668&view=diff ============================================================================== --- harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/Collections.java (original) +++ harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/Collections.java Wed Dec 9 01:01:03 2009 @@ -19,6 +19,7 @@ import java.io.IOException; import java.io.ObjectOutputStream; +import java.io.ObjectStreamException; import java.io.Serializable; import java.lang.reflect.Array; @@ -188,8 +189,15 @@ @SuppressWarnings("unchecked") public static final Map EMPTY_MAP = new EmptyMap(); + /** + * This class is a singleton so that equals() and hashCode() work properly. + */ private static final class ReverseComparator implements Comparator, Serializable { + + private static final ReverseComparator INSTANCE + = new ReverseComparator(); + private static final long serialVersionUID = 7207038068494060240L; @SuppressWarnings("unchecked") @@ -197,6 +205,10 @@ Comparable c2 = (Comparable) o2; return c2.compareTo(o1); } + + private Object readResolve() throws ObjectStreamException { + return INSTANCE; + } } private static final class ReverseComparatorWithComparator implements @@ -213,6 +225,18 @@ public int compare(T o1, T o2) { return comparator.compare(o2, o1); } + + @Override + public boolean equals(Object o) { + return o instanceof ReverseComparatorWithComparator + && ((ReverseComparatorWithComparator) o).comparator + .equals(comparator); + } + + @Override + public int hashCode() { + return ~comparator.hashCode(); + } } private static final class SingletonSet extends AbstractSet implements @@ -352,34 +376,17 @@ } public Map.Entry next() { - if (hasNext) { - hasNext = false; - return new Map.Entry() { - @Override - public boolean equals(Object object) { - return contains(object); - } - - public K getKey() { - return k; - } - - public V getValue() { - return v; - } - - @Override - public int hashCode() { - return (k == null ? 0 : k.hashCode()) - ^ (v == null ? 0 : v.hashCode()); - } - - public V setValue(V value) { - throw new UnsupportedOperationException(); - } - }; + if (!hasNext) { + throw new NoSuchElementException(); } - throw new NoSuchElementException(); + + hasNext = false; + return new MapEntry(k, v) { + @Override + public V setValue(V value) { + throw new UnsupportedOperationException(); + } + }; } public void remove() { @@ -1684,6 +1691,12 @@ */ public static T max(Collection collection, Comparator comparator) { + if (comparator == null) { + @SuppressWarnings("unchecked") // null comparator? T is comparable + T result = (T) max((Collection) collection); + return result; + } + Iterator it = collection.iterator(); T max = it.next(); while (it.hasNext()) { @@ -1734,6 +1747,12 @@ */ public static T min(Collection collection, Comparator comparator) { + if (comparator == null) { + @SuppressWarnings("unchecked") // null comparator? T is comparable + T result = (T) min((Collection) collection); + return result; + } + Iterator it = collection.iterator(); T min = it.next(); while (it.hasNext()) { @@ -1793,8 +1812,9 @@ * @see Comparable * @see Serializable */ + @SuppressWarnings("unchecked") public static Comparator reverseOrder() { - return new ReverseComparator(); + return (Comparator) ReverseComparator.INSTANCE; } /** @@ -1815,6 +1835,9 @@ if (c == null) { return reverseOrder(); } + if (c instanceof ReverseComparatorWithComparator) { + return ((ReverseComparatorWithComparator) c).comparator; + } return new ReverseComparatorWithComparator(c); } @@ -2660,8 +2683,8 @@ * class of object that should be * @return specified object */ - static E checkType(E obj, Class type) { - if (!type.isInstance(obj)) { + static E checkType(E obj, Class type) { + if (obj != null && !type.isInstance(obj)) { // luni.05=Attempt to insert {0} element into collection with // element type {1} throw new ClassCastException(Messages.getString( @@ -2769,20 +2792,11 @@ */ @SuppressWarnings("unchecked") public boolean addAll(Collection c1) { - int size = c1.size(); - if (size == 0) { - return false; + Object[] array = c1.toArray(); + for (Object o : array) { + checkType(o, type); } - E[] arr = (E[]) new Object[size]; - Iterator it = c1.iterator(); - for (int i = 0; i < size; i++) { - arr[i] = checkType(it.next(), type); - } - boolean added = false; - for (int i = 0; i < size; i++) { - added |= c.add(arr[i]); - } - return added; + return c.addAll((List) Arrays.asList(array)); } /** @@ -2928,16 +2942,11 @@ */ @SuppressWarnings("unchecked") public boolean addAll(int index, Collection c1) { - int size = c1.size(); - if (size == 0) { - return false; - } - E[] arr = (E[]) new Object[size]; - Iterator it = c1.iterator(); - for (int i = 0; i < size; i++) { - arr[i] = checkType(it.next(), type); + Object[] array = c1.toArray(); + for (Object o : array) { + checkType(o, type); } - return l.addAll(index, Arrays.asList(arr)); + return l.addAll(index, (List) Arrays.asList(array)); } /** Modified: harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/TreeMap.java URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/TreeMap.java?rev=888668&r1=888667&r2=888668&view=diff ============================================================================== --- harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/TreeMap.java (original) +++ harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/TreeMap.java Wed Dec 9 01:01:03 2009 @@ -160,6 +160,9 @@ @SuppressWarnings("unchecked") private static Comparable toComparable(T obj) { + if (obj == null) { + throw new NullPointerException(); + } return (Comparable) obj; } Modified: harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/TreeSet.java URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/TreeSet.java?rev=888668&r1=888667&r2=888668&view=diff ============================================================================== --- harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/TreeSet.java (original) +++ harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/TreeSet.java Wed Dec 9 01:01:03 2009 @@ -35,9 +35,10 @@ private static final long serialVersionUID = -2479143000061671589L; - private transient SortedMap backingMap; + /** Keys are this set's elements. Values are always Boolean.TRUE */ + private transient SortedMap backingMap; - private TreeSet(SortedMap map) { + private TreeSet(SortedMap map) { this.backingMap = map; } @@ -46,7 +47,7 @@ * ordering. */ public TreeSet() { - backingMap = new TreeMap(); + backingMap = new TreeMap(); } /** @@ -73,7 +74,7 @@ * the comparator to use. */ public TreeSet(Comparator comparator) { - backingMap = new TreeMap(comparator); + backingMap = new TreeMap(comparator); } /** @@ -107,7 +108,7 @@ */ @Override public boolean add(E object) { - return backingMap.put(object, object) == null; + return backingMap.put(object, Boolean.TRUE) == null; } /** @@ -153,10 +154,10 @@ try { TreeSet clone = (TreeSet) super.clone(); if (backingMap instanceof TreeMap) { - clone.backingMap = (SortedMap) ((TreeMap) backingMap) + clone.backingMap = (SortedMap) ((TreeMap) backingMap) .clone(); } else { - clone.backingMap = new TreeMap(backingMap); + clone.backingMap = new TreeMap(backingMap); } return clone; } catch (CloneNotSupportedException e) { @@ -373,14 +374,14 @@ private void readObject(ObjectInputStream stream) throws IOException, ClassNotFoundException { stream.defaultReadObject(); - TreeMap map = new TreeMap((Comparator) stream - .readObject()); + TreeMap map = new TreeMap( + (Comparator) stream.readObject()); int size = stream.readInt(); if (size > 0) { - TreeMap.Node lastNode = null; + TreeMap.Node lastNode = null; for(int i=0; i a = new HashMap(); + a.put("a", null); + a.put("b", null); + + Map b = new HashMap(); + a.put("c", "cat"); + a.put("d", "dog"); + + assertFalse(a.equals(b)); + assertFalse(b.equals(a)); + } + + public void testNullsOnViews() { + Map nullHostile = new Hashtable(); + + nullHostile.put("a", "apple"); + testNullsOnView(nullHostile.entrySet()); + + nullHostile.put("a", "apple"); + testNullsOnView(nullHostile.keySet()); + + nullHostile.put("a", "apple"); + testNullsOnView(nullHostile.values()); + } + + private void testNullsOnView(Collection view) { + try { + assertFalse(view.contains(null)); + } catch (NullPointerException optional) { + } + + try { + assertFalse(view.remove(null)); + } catch (NullPointerException optional) { + } + + Set setOfNull = Collections.singleton(null); + assertFalse(view.equals(setOfNull)); + + try { + assertFalse(view.removeAll(setOfNull)); + } catch (NullPointerException optional) { + } + + try { + assertTrue(view.retainAll(setOfNull)); // destructive + } catch (NullPointerException optional) { + } + } + protected void setUp() { }