diff --git a/cm.json/pom.xml b/cm.json/pom.xml index caad71bc53..6644f5814e 100644 --- a/cm.json/pom.xml +++ b/cm.json/pom.xml @@ -22,7 +22,7 @@ org.apache.felix felix-parent - 7 + 9 diff --git a/cm.json/src/main/java/org/apache/felix/cm/json/io/impl/ConfigurationReaderImpl.java b/cm.json/src/main/java/org/apache/felix/cm/json/io/impl/ConfigurationReaderImpl.java index 42dcf67a68..0be40b10dc 100644 --- a/cm.json/src/main/java/org/apache/felix/cm/json/io/impl/ConfigurationReaderImpl.java +++ b/cm.json/src/main/java/org/apache/felix/cm/json/io/impl/ConfigurationReaderImpl.java @@ -20,26 +20,35 @@ import java.io.IOException; import java.io.Reader; +import java.math.BigDecimal; import java.util.ArrayList; import java.util.Hashtable; import java.util.List; import java.util.Map; +import java.util.regex.Pattern; import jakarta.json.JsonException; +import jakarta.json.JsonNumber; import jakarta.json.JsonObject; +import jakarta.json.JsonString; import jakarta.json.JsonValue; import jakarta.json.JsonValue.ValueType; import org.apache.felix.cm.json.io.ConfigurationReader; import org.apache.felix.cm.json.io.ConfigurationResource; import org.apache.felix.cm.json.io.Configurations; +import org.osgi.framework.Version; import org.osgi.service.configurator.ConfiguratorConstants; -import org.osgi.util.converter.ConversionException; -import org.osgi.util.converter.Converters; public class ConfigurationReaderImpl implements ConfigurationReader, ConfigurationReader.Builder { + private static final Pattern SYMBOLIC_NAME_PATTERN = + Pattern.compile("[A-Za-z0-9_-]+(?:\\.[A-Za-z0-9_-]+)*"); + + private static final Pattern VERSION_PATTERN = Pattern.compile( + "[0-9]+(?:\\.[0-9]+(?:\\.[0-9]+(?:\\.[A-Za-z0-9_-]+)?)?)?"); + private boolean closed = false; private Reader reader; @@ -169,41 +178,47 @@ private void throwIOException(final String msg) throws IOException { * @param root The JSON root object. */ private void verifyJsonResource() throws IOException { - final Object version = JsonSupport - .convertToObject(this.jsonObject.get(ConfiguratorConstants.PROPERTY_RESOURCE_VERSION)); + final JsonValue version = this.jsonObject.get(ConfiguratorConstants.PROPERTY_RESOURCE_VERSION); if (version != null) { - int v = -1; - try { - v = Converters.standardConverter().convert(version).defaultValue(-1).to(Integer.class); - } catch ( final ConversionException ce ) { - // ignore - } - if (v == -1) { + if (version.getValueType() != ValueType.NUMBER || !((JsonNumber) version).isIntegral()) { throwIOException("Invalid resource version information : ".concat(version.toString())); } // we only support version 1 - if (v != 1) { + if (((JsonNumber) version).bigDecimalValue().compareTo(BigDecimal.ONE) != 0) { throwIOException("Unknown resource version : ".concat(version.toString())); } } - if (!verifyAsBundleResource) { - // if this is not a bundle resource - // then version and symbolic name must be set - final Object rsrcVersion = JsonSupport - .convertToObject(this.jsonObject.get(ConfiguratorConstants.PROPERTY_VERSION)); - if (rsrcVersion == null) { + final JsonValue rsrcVersion = this.jsonObject.get(ConfiguratorConstants.PROPERTY_VERSION); + if (rsrcVersion == null) { + if (!verifyAsBundleResource) { throwIOException("Missing version information"); } - if (!(rsrcVersion instanceof String)) { + } else { + if (rsrcVersion.getValueType() != ValueType.STRING) { + throwIOException("Invalid version information : ".concat(rsrcVersion.toString())); + } + final String resourceVersion = ((JsonString) rsrcVersion).getString(); + if (!VERSION_PATTERN.matcher(resourceVersion).matches()) { throwIOException("Invalid version information : ".concat(rsrcVersion.toString())); } - final Object rsrcName = JsonSupport - .convertToObject(this.jsonObject.get(ConfiguratorConstants.PROPERTY_SYMBOLIC_NAME)); - if (rsrcName == null) { + try { + new Version(resourceVersion); + } catch (final IllegalArgumentException iae) { + throwIOException("Invalid version information : ".concat(rsrcVersion.toString())); + } + } + final JsonValue rsrcName = this.jsonObject.get(ConfiguratorConstants.PROPERTY_SYMBOLIC_NAME); + if (rsrcName == null) { + if (!verifyAsBundleResource) { throwIOException("Missing symbolic name information"); } - if (!(rsrcName instanceof String)) { - throwIOException("Invalid symbolic name information : ".concat(rsrcVersion.toString())); + } else { + if (rsrcName.getValueType() != ValueType.STRING) { + throwIOException("Invalid symbolic name information : ".concat(rsrcName.toString())); + } + final String symbolicName = ((JsonString) rsrcName).getString(); + if (!SYMBOLIC_NAME_PATTERN.matcher(symbolicName).matches()) { + throwIOException("Invalid symbolic name information : ".concat(rsrcName.toString())); } } } @@ -224,7 +239,7 @@ public KeyInfo(final String mapKey) { if (isInternal) { key = key.substring(ConfigurationResource.CONFIGURATOR_PROPERTY_PREFIX.length()); } - final int pos = key.indexOf(':'); + final int pos = isInternal ? -1 : key.indexOf(':'); String typeInfo = null; if (pos != -1) { typeInfo = key.substring(pos + 1); diff --git a/cm.json/src/main/java/org/apache/felix/cm/json/io/impl/JsonSupport.java b/cm.json/src/main/java/org/apache/felix/cm/json/io/impl/JsonSupport.java index dfb59ef31f..275c995732 100644 --- a/cm.json/src/main/java/org/apache/felix/cm/json/io/impl/JsonSupport.java +++ b/cm.json/src/main/java/org/apache/felix/cm/json/io/impl/JsonSupport.java @@ -106,7 +106,14 @@ public static Object convertToObject(final JsonValue value) { } return objArray; } - return array.toString(); + final String[] stringArray = new String[array.size()]; + for (int i = 0; i < array.size(); i++) { + final JsonValue arrayValue = array.get(i); + stringArray[i] = arrayValue.getValueType() == ValueType.STRING + ? ((JsonString) arrayValue).getString() + : arrayValue.toString(); + } + return stringArray; // type OBJECT -> return map default: diff --git a/cm.json/src/test/java/org/apache/felix/cm/json/io/impl/ConfigurationReaderImplTest.java b/cm.json/src/test/java/org/apache/felix/cm/json/io/impl/ConfigurationReaderImplTest.java index bd76537c34..3c62534899 100644 --- a/cm.json/src/test/java/org/apache/felix/cm/json/io/impl/ConfigurationReaderImplTest.java +++ b/cm.json/src/test/java/org/apache/felix/cm/json/io/impl/ConfigurationReaderImplTest.java @@ -30,6 +30,7 @@ import java.io.StringReader; import java.nio.charset.StandardCharsets; import java.util.Dictionary; +import java.util.HashMap; import java.util.Hashtable; import java.util.Map; @@ -94,6 +95,159 @@ public void testReadBundleConfigurationResource() throws IOException { assertEquals(8080, configs.get("config.b").get("port")); } + @Test + public void testReadValidResourceVersion() throws IOException { + readResourceVersion("1"); + } + + @Test + public void testReadInvalidResourceVersionType() throws IOException { + assertInvalidResourceVersion("\"1\""); + } + + @Test + public void testReadFractionalResourceVersion() throws IOException { + assertInvalidResourceVersion("1.5"); + } + + private void assertInvalidResourceVersion(final String version) throws IOException { + try { + readResourceVersion(version); + fail(); + } catch (final IOException ioe) { + // expected + } + } + + private void readResourceVersion(final String version) throws IOException { + final String json = "{\n" + + " \":configurator:resource-version\" : " + version + ",\n" + + " \":configurator:version\" : \"1.0.0\",\n" + + " \":configurator:symbolic-name\" : \"feature\"\n" + + "}"; + new ConfigurationReaderImpl().build(new StringReader(json)).readConfigurationResource(); + } + + @Test + public void testReadValidSymbolicName() throws IOException { + readSymbolicName("\"com.example.feature-name_1\"", false); + } + + @Test + public void testReadInvalidSymbolicNameType() throws IOException { + assertInvalidSymbolicName("1", false); + } + + @Test + public void testReadInvalidSymbolicNameSyntax() throws IOException { + for (final String symbolicName : new String[] { + "", ".com.example", "com..example", "com.example.", + "com/example", "com:example", "com example", "com.ex\u00e4mple" + }) { + assertInvalidSymbolicName("\"" + symbolicName + "\"", false); + } + } + + @Test + public void testReadInvalidBundleResourceSymbolicName() throws IOException { + assertInvalidSymbolicName("\"com.example:bad\"", true); + } + + private void assertInvalidSymbolicName(final String symbolicName, final boolean bundleResource) + throws IOException { + try { + readSymbolicName(symbolicName, bundleResource); + fail(); + } catch (final IOException ioe) { + // expected + } + } + + private void readSymbolicName(final String symbolicName, final boolean bundleResource) throws IOException { + final String json = "{\n" + + " \":configurator:version\" : \"1.0.0\",\n" + + " \":configurator:symbolic-name\" : " + symbolicName + "\n" + + "}"; + new ConfigurationReaderImpl() + .verifyAsBundleResource(bundleResource) + .build(new StringReader(json)) + .readConfigurationResource(); + } + + @Test + public void testReadValidVersion() throws IOException { + readVersion("\"1.2.3.qualifier-1\"", false); + } + + @Test + public void testReadInvalidVersionType() throws IOException { + assertInvalidVersion("1", false); + } + + @Test + public void testReadInvalidVersionSyntax() throws IOException { + for (final String version : new String[] { + "", "+1", "1..2", "1.2.x", "1.2.3.", "1.2.3.bad qualifier", + "1.2.3.4.5", "-1.2.3", " 1.2.3" + }) { + assertInvalidVersion("\"" + version + "\"", false); + } + } + + @Test + public void testReadInvalidBundleResourceVersion() throws IOException { + assertInvalidVersion("\"1.2.invalid\"", true); + } + + private void assertInvalidVersion(final String version, final boolean bundleResource) throws IOException { + try { + readVersion(version, bundleResource); + fail(); + } catch (final IOException ioe) { + // expected + } + } + + private void readVersion(final String version, final boolean bundleResource) throws IOException { + final String json = "{\n" + + " \":configurator:version\" : " + version + ",\n" + + " \":configurator:symbolic-name\" : \"com.example.feature\"\n" + + "}"; + new ConfigurationReaderImpl() + .verifyAsBundleResource(bundleResource) + .build(new StringReader(json)) + .readConfigurationResource(); + } + + @Test + public void testPreserveConfiguratorPropertyKeys() throws IOException { + final String json = "{\n" + + " \":configurator:version\" : \"1.0.0\",\n" + + " \":configurator:symbolic-name\" : \"com.example.feature\",\n" + + " \"com.example.pid\" : {\n" + + " \":configurator:policy\" : \"default\",\n" + + " \":configurator:policy:String\" : \"force\",\n" + + " \":configurator:ranking\" : 10,\n" + + " \":configurator:ranking:Integer\" : 20\n" + + " }\n" + + "}"; + final Map properties = new HashMap<>(); + + new ConfigurationReaderImpl() + .withConfiguratorPropertyHandler((pid, key, value) -> { + assertEquals("com.example.pid", pid); + properties.put(key, value); + }) + .build(new StringReader(json)) + .readConfigurationResource(); + + assertEquals(4, properties.size()); + assertEquals("default", properties.get("policy")); + assertEquals("force", properties.get("policy:String")); + assertEquals(Long.valueOf(10), properties.get("ranking")); + assertEquals(Long.valueOf(20), properties.get("ranking:Integer")); + } + @Test public void testReadInvalidJson() throws IOException { final String json = "{\n \"a\" : 5 \n \"b\" : 2\n}"; diff --git a/cm.json/src/test/java/org/apache/felix/cm/json/io/impl/JsonSupportTest.java b/cm.json/src/test/java/org/apache/felix/cm/json/io/impl/JsonSupportTest.java index 0c149e45d6..1b7f28eae6 100644 --- a/cm.json/src/test/java/org/apache/felix/cm/json/io/impl/JsonSupportTest.java +++ b/cm.json/src/test/java/org/apache/felix/cm/json/io/impl/JsonSupportTest.java @@ -115,6 +115,16 @@ public void testConvertStringArrayToObject() { assertArrayEquals(new String[] { "hello", "3" }, (String[]) Configurations.convertToObject(mBuilder.build())); } + @Test + public void testConvertMixedArrayToObject() { + final JsonArrayBuilder builder = Json.createArrayBuilder(); + builder.add("hello"); + builder.add(3); + + assertArrayEquals(new String[] { "hello", "3" }, + (String[]) Configurations.convertToObject(builder.build())); + } + @Test public void testConvertObjectArrayToObject() { final JsonArrayBuilder sBuilder = Json.createArrayBuilder(); diff --git a/configurator/pom.xml b/configurator/pom.xml index 3d0b1770cb..f383900e44 100644 --- a/configurator/pom.xml +++ b/configurator/pom.xml @@ -22,7 +22,7 @@ org.apache.felix felix-parent - 7 + 9 @@ -114,7 +114,7 @@ org.apache.felix org.apache.felix.cm.json - 2.0.0 + 2.0.9-SNAPSHOT provided