You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by cz...@apache.org on 2021/01/24 10:23:58 UTC
[sling-org-apache-sling-feature-extension-apiregions] branch master
updated: SLING-10086 : Validate configuration properties case-insensitive
This is an automated email from the ASF dual-hosted git repository.
cziegeler pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-extension-apiregions.git
The following commit(s) were added to refs/heads/master by this push:
new fc77a74 SLING-10086 : Validate configuration properties case-insensitive
fc77a74 is described below
commit fc77a7467eb6b990a65e9d9ce01b5c7e26b253c6
Author: Carsten Ziegeler <cz...@apache.org>
AuthorDate: Sun Jan 24 11:23:41 2021 +0100
SLING-10086 : Validate configuration properties case-insensitive
---
.../apiregions/api/config/CaseInsensitiveMap.java | 281 +++++++++++++++++++++
.../apiregions/api/config/ConfigurableEntity.java | 7 +-
.../config/validation/ConfigurationValidator.java | 16 +-
.../api/config/CaseInsensitiveMapTest.java | 101 ++++++++
4 files changed, 399 insertions(+), 6 deletions(-)
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/CaseInsensitiveMap.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/CaseInsensitiveMap.java
new file mode 100644
index 0000000..d7af7ad
--- /dev/null
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/CaseInsensitiveMap.java
@@ -0,0 +1,281 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.sling.feature.extension.apiregions.api.config;
+
+import java.util.AbstractSet;
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+
+class CaseInsensitiveMap<T> implements Map<String, T> {
+
+ private final Map<CaseInsensitiveKey, T> map = new LinkedHashMap<>();
+
+ @Override
+ public void clear() {
+ this.map.clear();
+ }
+
+ @Override
+ public boolean containsKey(final Object key) {
+ return this.map.containsKey(new CaseInsensitiveKey(key.toString()));
+ }
+
+ @Override
+ public boolean containsValue(final Object value) {
+ return this.map.containsValue(value);
+ }
+
+ @Override
+ public Set<Entry<String, T>> entrySet() {
+ return new EntrySet();
+ }
+
+ @Override
+ public T get(final Object key) {
+ final CaseInsensitiveKey k = new CaseInsensitiveKey(key.toString());
+ return this.map.get(k);
+ }
+
+ @Override
+ public boolean isEmpty() {
+ return this.map.isEmpty();
+ }
+
+ @Override
+ public Set<String> keySet() {
+ return new KeySet();
+ }
+
+ @Override
+ public T put(final String key, final T value) {
+ final CaseInsensitiveKey k = new CaseInsensitiveKey(key.toString());
+ final T old = this.map.remove(k);
+ this.map.put(k, value);
+ return old;
+ }
+
+ @Override
+ public void putAll(final Map<? extends String, ? extends T> m) {
+ for(final Map.Entry<? extends String, ? extends T> entry : m.entrySet()) {
+ this.put(entry.getKey(), entry.getValue());
+ }
+ }
+
+ @Override
+ public T remove(final Object key) {
+ final CaseInsensitiveKey k = new CaseInsensitiveKey(key.toString());
+ return this.map.remove(k);
+ }
+
+ @Override
+ public int size() {
+ return this.map.size();
+ }
+
+ @Override
+ public Collection<T> values() {
+ return this.map.values();
+ }
+
+ private static final class CaseInsensitiveKey {
+
+ private final String value;
+
+ private final int hashCode;
+
+ public CaseInsensitiveKey(final String v) {
+ this.value = v;
+ this.hashCode = v.toUpperCase().hashCode();
+ }
+
+ /* (non-Javadoc)
+ * @see java.lang.Object#hashCode()
+ */
+ @Override
+ public int hashCode() {
+ return this.hashCode;
+ }
+
+ /* (non-Javadoc)
+ * @see java.lang.Object#equals(java.lang.Object)
+ */
+ @Override
+ public boolean equals(final Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (!(obj instanceof CaseInsensitiveKey)) {
+ return false;
+ }
+ final CaseInsensitiveKey other = (CaseInsensitiveKey) obj;
+ if ( value == null ) {
+ if ( other.value == null ) {
+ return true;
+ }
+ return false;
+ }
+ if ( other.value == null ) {
+ return false;
+ }
+ return value.equalsIgnoreCase(other.value);
+ }
+ }
+
+ private final class KeySet extends AbstractSet<String> {
+
+ @Override
+ public int size() {
+ return CaseInsensitiveMap.this.size();
+ }
+
+ @Override
+ public boolean isEmpty() {
+ return CaseInsensitiveMap.this.isEmpty();
+ }
+
+ @Override
+ public boolean contains(Object o) {
+ return CaseInsensitiveMap.this.containsKey(o);
+ }
+
+ @Override
+ public Iterator<String> iterator() {
+ return new KeyIterator(CaseInsensitiveMap.this.map.keySet());
+ }
+
+ @Override
+ public boolean remove(Object o) {
+ return CaseInsensitiveMap.this.remove(o) != null;
+ }
+
+ @Override
+ public void clear() {
+ CaseInsensitiveMap.this.clear();
+ }
+ }
+
+ private static final class KeyIterator implements Iterator<String> {
+ private final Iterator<CaseInsensitiveKey> i;
+
+ KeyIterator(final Collection<CaseInsensitiveKey> c) {
+ this.i = c.iterator();
+ }
+
+ @Override
+ public boolean hasNext() {
+ return i.hasNext();
+ }
+
+ @Override
+ public String next() {
+ final CaseInsensitiveKey k = i.next();
+ return k.value;
+ }
+
+ @Override
+ public void remove() {
+ i.remove();
+ }
+ }
+
+ private final class EntrySet extends AbstractSet<Entry<String, T>> {
+
+ @Override
+ public int size() {
+ return CaseInsensitiveMap.this.size();
+ }
+
+ @Override
+ public boolean isEmpty() {
+ return CaseInsensitiveMap.this.isEmpty();
+ }
+
+ @Override
+ public Iterator<Entry<String, T>> iterator() {
+ return new EntryIterator<>(CaseInsensitiveMap.this.map.entrySet());
+ }
+
+ @Override
+ public void clear() {
+ CaseInsensitiveMap.this.clear();
+ }
+ }
+
+ private static final class EntryIterator<T> implements Iterator<Entry<String, T>> {
+ private final Iterator<Entry<CaseInsensitiveKey, T>> i;
+
+ EntryIterator(final Collection<Entry<CaseInsensitiveKey, T>> c) {
+ this.i = c.iterator();
+ }
+
+ @Override
+ public boolean hasNext() {
+ return i.hasNext();
+ }
+
+ @Override
+ public Entry<String, T> next() {
+ return new CaseInsentiveEntry<>(i.next());
+ }
+
+ @Override
+ public void remove() {
+ i.remove();
+ }
+ }
+
+ private static final class CaseInsentiveEntry<T> implements Entry<String, T> {
+ private final Entry<CaseInsensitiveKey, T> entry;
+
+ CaseInsentiveEntry(final Entry<CaseInsensitiveKey, T> entry) {
+ this.entry = entry;
+ }
+
+ @Override
+ public String getKey() {
+ return entry.getKey().value;
+ }
+
+ @Override
+ public T getValue() {
+ return entry.getValue();
+ }
+
+ @Override
+ public T setValue(final T value) {
+ return entry.setValue(value);
+ }
+
+ @Override
+ public int hashCode() {
+ return entry.hashCode();
+ }
+
+ @Override
+ public boolean equals(final Object obj) {
+ if (obj instanceof CaseInsentiveEntry) {
+ final CaseInsentiveEntry<?> other = (CaseInsentiveEntry<?>) obj;
+ return Objects.equals(other.entry.getKey(), this.entry.getKey()) && Objects.equals(other.entry.getValue(), this.entry.getValue());
+ }
+ return false;
+ }
+ }
+}
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/ConfigurableEntity.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/ConfigurableEntity.java
index 4e7eb9b..c7c9f31 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/ConfigurableEntity.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/ConfigurableEntity.java
@@ -17,7 +17,6 @@
package org.apache.sling.feature.extension.apiregions.api.config;
import java.io.IOException;
-import java.util.LinkedHashMap;
import java.util.Map;
import javax.json.Json;
@@ -33,7 +32,7 @@ import javax.json.JsonValue;
public abstract class ConfigurableEntity extends DescribableEntity {
/** The properties */
- private final Map<String, PropertyDescription> properties = new LinkedHashMap<>();
+ private final Map<String, PropertyDescription> properties = new CaseInsensitiveMap<>();
/**
* Clear the object and reset to defaults
@@ -60,7 +59,9 @@ public abstract class ConfigurableEntity extends DescribableEntity {
for(final Map.Entry<String, JsonValue> innerEntry : val.asJsonObject().entrySet()) {
final PropertyDescription prop = new PropertyDescription();
prop.fromJSONObject(innerEntry.getValue().asJsonObject());
- this.getPropertyDescriptions().put(innerEntry.getKey(), prop);
+ if ( this.getPropertyDescriptions().put(innerEntry.getKey(), prop) != null ) {
+ throw new IOException("Duplicate key for property description (keys are case-insensitive) : ".concat(innerEntry.getKey()));
+ }
}
}
} catch (final JsonException | IllegalArgumentException e) {
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/ConfigurationValidator.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/ConfigurationValidator.java
index 0a4228e..a0e9bc7 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/ConfigurationValidator.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/config/validation/ConfigurationValidator.java
@@ -89,7 +89,8 @@ public class ConfigurationValidator {
final Dictionary<String, Object> properties = configuration.getConfigurationProperties();
// validate the described properties
for(final Map.Entry<String, PropertyDescription> propEntry : desc.getPropertyDescriptions().entrySet()) {
- final Object value = properties.get(propEntry.getKey());
+ // TODO - we need to make a case-insensitive lookup (see SLING-10084)
+ final Object value = properties.get(propEntry.getKey());
final PropertyValidationResult result = propertyValidator.validate(value, propEntry.getValue());
results.put(propEntry.getKey(), result);
}
@@ -100,15 +101,24 @@ public class ConfigurationValidator {
if ( !desc.getPropertyDescriptions().containsKey(propName) ) {
final PropertyValidationResult result = new PropertyValidationResult();
results.put(propName, result);
- if ( Constants.SERVICE_RANKING.equals(propName) ) {
+ if ( Constants.SERVICE_RANKING.equalsIgnoreCase(propName) ) {
final Object value = properties.get(propName);
if ( !(value instanceof Integer) ) {
result.getErrors().add("service.ranking must be of type Integer");
}
- } else if ( !ALLOWED_PROPERTIES.contains(propName) && region != Region.INTERNAL ) {
+ } else if ( !isAllowedProperty(propName) && region != Region.INTERNAL ) {
result.getErrors().add("Property is not allowed");
}
}
}
}
+
+ private boolean isAllowedProperty(final String name) {
+ for(final String allowed : ALLOWED_PROPERTIES) {
+ if ( allowed.equalsIgnoreCase(name) ) {
+ return true;
+ }
+ }
+ return false;
+ }
}
\ No newline at end of file
diff --git a/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/CaseInsensitiveMapTest.java b/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/CaseInsensitiveMapTest.java
new file mode 100644
index 0000000..ff120b3
--- /dev/null
+++ b/src/test/java/org/apache/sling/feature/extension/apiregions/api/config/CaseInsensitiveMapTest.java
@@ -0,0 +1,101 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.sling.feature.extension.apiregions.api.config;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.junit.Test;
+
+public class CaseInsensitiveMapTest {
+
+ @Test public void testKeys() {
+ final Map<String, String> map = new CaseInsensitiveMap<>();
+ assertNull(map.put("hello", "helloV"));
+ assertNull(map.put("world", "worldV"));
+
+ assertEquals(2, map.size());
+ assertEquals(2, map.values().size());
+ assertTrue(map.values().contains("helloV"));
+ assertTrue(map.values().contains("worldV"));
+
+ assertEquals(2, map.entrySet().size());
+ final Map<String, String> m = map.entrySet().stream().collect(Collectors.toMap(e -> e.getKey(), e -> e.getValue()));
+ assertEquals("helloV", m.get("hello"));
+ assertEquals("worldV", m.get("world"));
+
+ Set<String> keys = new HashSet<>(map.keySet());
+ assertEquals(2, keys.size());
+ assertTrue(keys.contains("hello"));
+ assertTrue(keys.contains("world"));
+
+ assertEquals("helloV", map.get("hello"));
+ assertEquals("worldV", map.get("world"));
+ assertNull(map.get("foo"));
+
+ assertEquals("helloV", map.get("HELLO"));
+ assertEquals("worldV", map.get("WORLD"));
+
+ assertEquals("helloV", map.put("heLLo", "bar"));
+ assertEquals(2, map.size());
+
+ keys = new HashSet<>(map.keySet());
+ assertEquals(2, keys.size());
+ assertTrue(keys.contains("heLLo"));
+ assertTrue(keys.contains("world"));
+
+ assertEquals("bar", map.get("hello"));
+ assertEquals("worldV", map.get("world"));
+
+ assertEquals("bar", map.remove("HellO"));
+ assertEquals("worldV", map.remove("WoRlD"));
+ assertTrue(map.isEmpty());
+ }
+
+ @Test public void testClear() {
+ final Map<String, String> map = new CaseInsensitiveMap<>();
+ map.put("hello", "hello");
+ map.put("world", "world");
+
+ map.clear();
+ assertTrue(map.isEmpty());
+ assertEquals(0, map.size());
+ assertTrue(map.values().isEmpty());
+ assertTrue(map.keySet().isEmpty());
+ assertTrue(map.entrySet().isEmpty());
+ }
+
+ @Test public void testContains() {
+ final Map<String, String> map = new CaseInsensitiveMap<>();
+ map.put("hello", "helloV");
+
+ assertTrue(map.containsKey("hello"));
+ assertTrue(map.containsKey("heLLo"));
+ assertFalse(map.containsKey("hell o"));
+
+ assertTrue(map.containsValue("helloV"));
+ assertFalse(map.containsValue("heLLoV"));
+ assertFalse(map.containsValue("hello"));
+ }
+}