Bug ID: JDK-5026830 (process) Improve Set returned by ProcessBuilder.environment().entrySet()

Type: Bug
Component: core-libs
Sub-Component: java.lang
Affected Version: 5.0

Priority: P2
Status: Resolved
Resolution: Fixed
OS: generic
CPU: generic

Submitted: 2004-04-05
Updated: 2004-04-16
Resolved: 2004-04-16

Other
5.0 b48Fixed

The contains(Object) and remove(Object) methods of Set implemented for
the Set<Map.Entry<String,String>> returned by the entrySet() method
of the Map<String,String> returned by ProcessBuilder.environment()
and implemented in the ProcessEnvironment.java files for Unix and Windows
have the following issues:

This code is currently accepted by Tiger's javac, but an imminent putback
will make this a compile error:

	    instanceof Map.Entry<String,String>

Further, these methods should throw the optional NullPointerException
and ClassCastExceptions as documented in Set.java.

A previous fix
 4932663 (process) Add more runtime error checking for environment map operation
ensured that these exceptions were thrown for other relevant methods, such
as ProcessEnvironment.environment().keySet().contains(Object)

This should be made consistent.

The following desirable property of Set is not satisfied by the set returned
by entrySet

For each element e of the Set s as returned by its iterator, 
it should be true that
s.contains(e)

###@###.### 2004-04-05

CONVERTED DATA BugTraq+ Release Management Values COMMIT TO FIX: tiger-beta2 FIXED IN: tiger-beta2 INTEGRATED IN: tiger-b48 tiger-beta2

14-06-2004

SUGGESTED FIX --- /u/martin/ws/tiger/src/solaris/classes/java/lang/ProcessEnvironment.java 2004-01-12 15:05:40.356959000 -0800 +++ /u/martin/ws/process/src/solaris/classes/java/lang/ProcessEnvironment.java 2004-04-04 15:14:14.873633000 -0700 @@ -317,14 +317,23 @@ public void remove() {i.remove();} }; } - public boolean contains(Object o) { - return o instanceof StringEntry - && s.contains(((StringEntry)o).e); - } - public boolean remove(Object o) { - return o instanceof StringEntry - && s.remove(((StringEntry)o).e); + private static Map.Entry<Variable,Value> vvEntry(final Object o) { + if (o instanceof StringEntry) + return ((StringEntry)o).e; + return new Map.Entry<Variable,Value>() { + public Variable getKey() { + return Variable.valueOfQueryOnly(((Map.Entry)o).getKey()); + } + public Value getValue() { + return Value.valueOfQueryOnly(((Map.Entry)o).getValue()); + } + public Value setValue(Value value) { + throw new UnsupportedOperationException(); + } + }; } + public boolean contains(Object o) { return s.contains(vvEntry(o)); } + public boolean remove(Object o) { return s.remove(vvEntry(o)); } public boolean equals(Object o) { return o instanceof StringEntrySet && s.equals(((StringEntrySet) o).s); --- /u/martin/ws/tiger/src/windows/classes/java/lang/ProcessEnvironment.java 2004-01-12 15:08:06.318151000 -0800 +++ /u/martin/ws/process/src/windows/classes/java/lang/ProcessEnvironment.java 2004-04-02 22:22:36.597796000 -0800 @@ -124,14 +124,14 @@ public void remove() { i.remove();} }; } - public boolean contains(Object o) { - return o instanceof Map.Entry<String,String> - && s.contains((Map.Entry<String,String>)o); - } - public boolean remove(Object o) { - return o instanceof Map.Entry<String,String> - && s.remove((Map.Entry<String,String>)o); + private static Map.Entry<String,String> checkedEntry (Object o) { + Map.Entry<String,String> e = (Map.Entry<String,String>) o; + nonNullString(e.getKey()); + nonNullString(e.getValue()); + return e; } + public boolean contains(Object o) {return s.contains(checkedEntry(o));} + public boolean remove(Object o) {return s.remove(checkedEntry(o));} } private static class CheckedValues extends AbstractCollection<String> { --- /u/martin/ws/tiger/test/java/lang/ProcessBuilder/Basic.java 2004-02-09 18:48:35.711728000 -0800 +++ /u/martin/ws/process/test/java/lang/ProcessBuilder/Basic.java 2004-04-05 13:00:20.917717000 -0700 @@ -336,32 +336,46 @@ private static void checkMapSanity(Map<String,String> map) { try { - check(map.entrySet().size() == map.keySet().size() && - map.entrySet().size() == map.values().size(), - "Map size is insane"); + Set<String> keySet = map.keySet(); + Collection<String> values = map.values(); + Set<Map.Entry<String,String>> entrySet = map.entrySet(); + + check(entrySet.size() == keySet.size() && + entrySet.size() == values.size(), + "Map sizes are insane"); StringBuilder s1 = new StringBuilder(); - for (Map.Entry<String,String> e : map.entrySet()) + for (Map.Entry<String,String> e : entrySet) s1.append(e.getKey() + "=" + e.getValue() + "\n"); StringBuilder s2 = new StringBuilder(); - for (String var : map.keySet()) + for (String var : keySet) s2.append(var + "=" + map.get(var) + "\n"); check(s1.toString().equals(s2.toString()), "entrySet() and keySet() disagree"); - Iterator<String> keys = map.keySet().iterator(); - Iterator<String> values = map.values().iterator(); - Iterator<Map.Entry<String,String>> entries = map.entrySet().iterator(); - - while (entries.hasNext()) { - Map.Entry<String,String> entry = entries.next(); - check(entry.getKey() == keys.next() && - entry.getValue() == values.next(), + Iterator<String> kIter = keySet.iterator(); + Iterator<String> vIter = values.iterator(); + Iterator<Map.Entry<String,String>> eIter = entrySet.iterator(); + + while (eIter.hasNext()) { + Map.Entry<String,String> entry = eIter.next(); + String key = kIter.next(); + String value = vIter.next(); + check(entrySet.contains(entry) && + keySet.contains(key) && + values.contains(value) && + map.containsKey(key) && + map.containsValue(value), + "Map element queries broken"); + check(entry.getKey() == key && + entry.getValue() == value, "Map iterators inconsistent"); } - check(! keys.hasNext() && ! values.hasNext()); + check(! kIter.hasNext() && + ! vIter.hasNext(), + "Run-on Map iterators"); } catch (Exception e) {unexpectedException(e);} } @@ -609,7 +623,9 @@ new Thunk() { void run() { env.keySet().remove(TRUE);}}, new Thunk() { void run() { - env.values().remove(TRUE);}}); + env.values().remove(TRUE);}}, + new Thunk() { void run() { + env.entrySet().remove(TRUE);}}); } catch (Exception e) {unexpectedException(e);} //---------------------------------------------------------------- @@ -666,6 +682,30 @@ "incorrect handling of invalid strings in environment queries");}}); } + + } catch (Exception e) {unexpectedException(e);} + + try { + final Set<Map.Entry<String,String>> entrySet = + new ProcessBuilder().environment().entrySet(); + THROWS(NullPointerException.class, + new Thunk() { void run() { + entrySet.contains(null);}}); + THROWS(ClassCastException.class, + new Thunk() { void run() { + entrySet.contains(TRUE);}}, + new Thunk() { void run() { + entrySet.contains + (new Map.Entry<Boolean,String>() { + public Boolean getKey() {return TRUE;} + public String getValue() {return "";} + public String setValue(String v) {return "";}});}}); + NOTHROW(new Thunk() { void run() { + entrySet.contains + (new Map.Entry<String,String>() { + public String getKey() {return "";} + public String getValue() {return "";} + public String setValue(String v) {return "";}});}}); } catch (Exception e) {unexpectedException(e);} //---------------------------------------------------------------- ###@###.### 2004-04-05

05-04-2004

EVALUATION Sounds good to me. ###@###.### 2004-04-05

05-04-2004