Bug 520281 - NPE possible if null values are used to match against a
filter
Change-Id: I5b4e2b75863500a91d5e278481619254addffbd6
Signed-off-by: Thomas Watson <tjwatson@us.ibm.com>
diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/filter/FilterTests.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/filter/FilterTests.java
index 72b32f4..2ba3338 100644
--- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/filter/FilterTests.java
+++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/filter/FilterTests.java
@@ -15,6 +15,7 @@
import java.util.*;
import junit.framework.*;
import org.eclipse.osgi.framework.util.CaseInsensitiveDictionaryMap;
+import org.eclipse.osgi.tests.util.MapDictionary;
import org.osgi.framework.*;
public abstract class FilterTests extends TestCase {
@@ -344,6 +345,21 @@
assertFalse("does match filter", f1.match(new DictionaryServiceReference(hash)));
}
+ public void testNullValueMatch() throws InvalidSyntaxException {
+ Dictionary<String, Object> nullProps = new MapDictionary<String, Object>();
+ nullProps.put("test.null", null);
+ nullProps.put("test.non.null", "v1");
+ assertFalse(createFilter("(test.null=*)").match(nullProps));
+ assertTrue(createFilter("(&(!(test.null=*))(test.non.null=v1))").match(nullProps));
+ }
+
+ public void testNullKeyMatch() throws InvalidSyntaxException {
+ Dictionary<String, Object> nullProps = new MapDictionary<String, Object>();
+ nullProps.put(null, "null.v1");
+ nullProps.put("test.non.null", "v1");
+ assertTrue(createFilter("(test.non.null=v1)").match(nullProps));
+ }
+
public static class SampleComparable implements Comparable {
private int value = -1;
diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/serviceregistry/ServiceRegistryTests.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/serviceregistry/ServiceRegistryTests.java
index 5de8e20..67dcbde 100644
--- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/serviceregistry/ServiceRegistryTests.java
+++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/serviceregistry/ServiceRegistryTests.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2008, 2014 IBM Corporation and others.
+ * Copyright (c) 2008, 2017 IBM Corporation and others.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
@@ -10,6 +10,7 @@
*******************************************************************************/
package org.eclipse.osgi.tests.serviceregistry;
+import java.util.Dictionary;
import java.util.Hashtable;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
@@ -17,6 +18,7 @@
import junit.framework.TestSuite;
import org.eclipse.osgi.tests.OSGiTestsActivator;
import org.eclipse.osgi.tests.bundles.AbstractBundleTests;
+import org.eclipse.osgi.tests.util.MapDictionary;
import org.osgi.framework.*;
public class ServiceRegistryTests extends AbstractBundleTests {
@@ -523,6 +525,38 @@
}
+ public void testNullValue() throws InvalidSyntaxException {
+ ServiceRegistration reg = null;
+ try {
+ Dictionary<String, Object> nullProps = new MapDictionary<String, Object>();
+ nullProps.put("test.null", null);
+ nullProps.put("test.non.null", "v1");
+ reg = OSGiTestsActivator.getContext().registerService(Object.class, new Object(), nullProps);
+ assertFalse(OSGiTestsActivator.getContext().createFilter("(test.null=*)").match(reg.getReference()));
+ assertFalse(OSGiTestsActivator.getContext().createFilter("(test.null=*)").match(reg.getReference().getProperties()));
+ assertTrue(OSGiTestsActivator.getContext().createFilter("(&(!(test.null=*))(test.non.null=v1))").match(reg.getReference()));
+ assertTrue(OSGiTestsActivator.getContext().createFilter("(&(!(test.null=*))(test.non.null=v1))").match(reg.getReference().getProperties()));
+ } finally {
+ if (reg != null)
+ reg.unregister();
+ }
+ }
+
+ public void testNullKey() throws InvalidSyntaxException {
+ ServiceRegistration reg = null;
+ try {
+ Dictionary<String, Object> nullProps = new MapDictionary<String, Object>();
+ nullProps.put(null, "null.v1");
+ nullProps.put("test.non.null", "v1");
+ reg = OSGiTestsActivator.getContext().registerService(Object.class, new Object(), nullProps);
+ assertTrue(OSGiTestsActivator.getContext().createFilter("(test.non.null=v1)").match(reg.getReference()));
+ assertTrue(OSGiTestsActivator.getContext().createFilter("(test.non.null=v1)").match(reg.getReference().getProperties()));
+ } finally {
+ if (reg != null)
+ reg.unregister();
+ }
+ }
+
private void clearResults(boolean[] results) {
for (int i = 0; i < results.length; i++)
results[i] = false;
diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/util/MapDictionary.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/util/MapDictionary.java
new file mode 100644
index 0000000..d3afdb1
--- /dev/null
+++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/util/MapDictionary.java
@@ -0,0 +1,53 @@
+/*******************************************************************************
+ * Copyright (c) 2017 IBM Corporation and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ * IBM Corporation - initial API and implementation
+ ******************************************************************************/
+package org.eclipse.osgi.tests.util;
+
+import java.util.*;
+
+public class MapDictionary<K, V> extends Dictionary<K, V> {
+ Map<K, V> map = new HashMap<K, V>();
+
+ @Override
+ public int size() {
+ return map.size();
+ }
+
+ @Override
+ public boolean isEmpty() {
+ return map.isEmpty();
+ }
+
+ @Override
+ public Enumeration<K> keys() {
+ return Collections.enumeration(map.keySet());
+ }
+
+ @Override
+ public Enumeration<V> elements() {
+ return Collections.enumeration(map.values());
+ }
+
+ @Override
+ public V get(Object key) {
+ return map.get(key);
+ }
+
+ @Override
+ public V put(K key, V value) {
+ return map.put(key, value);
+ }
+
+ @Override
+ public V remove(Object key) {
+ return map.remove(key);
+ }
+
+}
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/framework/util/CaseInsensitiveDictionaryMap.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/framework/util/CaseInsensitiveDictionaryMap.java
index 545cc90..a4700be 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/framework/util/CaseInsensitiveDictionaryMap.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/framework/util/CaseInsensitiveDictionaryMap.java
@@ -59,8 +59,15 @@
Enumeration<? extends K> keys = dictionary.keys();
while (keys.hasMoreElements()) {
K key = keys.nextElement();
- if (put(key, dictionary.get(key)) != null) {
- throw new IllegalArgumentException(NLS.bind(Msg.HEADER_DUPLICATE_KEY_EXCEPTION, key));
+ // ignore null keys
+ if (key != null) {
+ V value = dictionary.get(key);
+ // ignore null values
+ if (value != null) {
+ if (put(key, value) != null) {
+ throw new IllegalArgumentException(NLS.bind(Msg.HEADER_DUPLICATE_KEY_EXCEPTION, key));
+ }
+ }
}
}
}
@@ -76,8 +83,16 @@
this(initialCapacity(map.size()));
/* initialize the keys and values */
for (Entry<? extends K, ? extends V> e : map.entrySet()) {
- if (put(e.getKey(), e.getValue()) != null) {
- throw new IllegalArgumentException(NLS.bind(Msg.HEADER_DUPLICATE_KEY_EXCEPTION, e.getKey()));
+ K key = e.getKey();
+ // ignore null keys
+ if (key != null) {
+ V value = e.getValue();
+ // ignore null values
+ if (value != null) {
+ if (put(key, value) != null) {
+ throw new IllegalArgumentException(NLS.bind(Msg.HEADER_DUPLICATE_KEY_EXCEPTION, key));
+ }
+ }
}
}
}
@@ -112,13 +127,11 @@
/**
* {@inheritDoc}
* <p>
- * The key must be non-null.
- * <p>
* If the key is a String, the key is located in a case-insensitive manner.
*/
@Override
public V get(Object key) {
- return map.get(keyWrap(requireNonNull(key)));
+ return map.get(keyWrap(key));
}
/**
@@ -173,13 +186,11 @@
/**
* {@inheritDoc}
* <p>
- * The key must be non-null.
- * <p>
* If the key is a String, the key is removed in a case-insensitive manner.
*/
@Override
public V remove(Object key) {
- return map.remove(keyWrap(requireNonNull(key)));
+ return map.remove(keyWrap(key));
}
/**
@@ -201,23 +212,19 @@
/**
* {@inheritDoc}
* <p>
- * The key must be non-null.
- * <p>
* If the key is a String, the key is located in a case-insensitive manner.
*/
@Override
public boolean containsKey(Object key) {
- return map.containsKey(keyWrap(requireNonNull(key)));
+ return map.containsKey(keyWrap(key));
}
/**
* {@inheritDoc}
- * <p>
- * The value must be non-null.
*/
@Override
public boolean containsValue(Object value) {
- return map.containsValue(requireNonNull(value));
+ return map.containsValue(value);
}
private transient Set<Entry<K, V>> entrySet = null;
@@ -556,7 +563,7 @@
@Override
public int hashCode() {
- return entry.getKey().hashCode() ^ entry.getValue().hashCode();
+ return Objects.hashCode(entry.getKey()) ^ Objects.hashCode(entry.getValue());
}
@Override
@@ -566,13 +573,7 @@
Object k1 = entry.getKey();
@SuppressWarnings("unchecked")
Object k2 = (other instanceof CaseInsentiveEntry) ? ((CaseInsentiveEntry<K, V>) other).entry.getKey() : other.getKey();
- if ((k1 == k2) || k1.equals(k2)) {
- Object v1 = entry.getValue();
- Object v2 = other.getValue();
- if ((v1 == v2) || v1.equals(v2)) {
- return true;
- }
- }
+ return Objects.equals(k1, k2) && Objects.equals(entry.getValue(), other.getValue());
}
return false;
}
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceProperties.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceProperties.java
index b875b6c..b6646d4 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceProperties.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceProperties.java
@@ -54,7 +54,8 @@
Object key = keysEnum.nextElement();
if (key instanceof String) {
String header = (String) key;
- if (put(header, cloneValue(props.get(header))) != null) {
+ Object value = cloneValue(props.get(header));
+ if (value != null && put(header, value) != null) {
throw new IllegalArgumentException(NLS.bind(Msg.HEADER_DUPLICATE_KEY_EXCEPTION, key));
}
}
@@ -79,7 +80,8 @@
Object key = e.getKey();
if (key instanceof String) {
String header = (String) key;
- if (put(header, cloneValue(e.getValue())) != null) {
+ Object value = cloneValue(props.get(header));
+ if (value != null && put(header, value) != null) {
throw new IllegalArgumentException(NLS.bind(Msg.HEADER_DUPLICATE_KEY_EXCEPTION, key));
}
}
