From d30072d0082d4520b10b49ef0a4b0969bd7fea19 Mon Sep 17 00:00:00 2001 From: Stefan Bischof Date: Sun, 1 Nov 2026 14:51:20 +0100 Subject: [PATCH 1/2] Convert typed attributes in all manifest headers Felix applied the type of a typed attribute (name:Long=42) only for Provide-Capability, Require-Capability and Bundle-SymbolicName. Export-Package kept the value as a String, which fails ExportAnnotationsTestCase#testAttributes of the OSGi bundle annotations TCK. Import-Package, DynamicImport-Package, Require-Bundle and Fragment-Host dropped the type as well, so a list attribute there could never match a typed capability. The conversion now runs for all of these headers. A list value in a requirement header becomes one filter per element, so it matches a capability list that contains all of them. The version and specification-version check compares versions instead of strings, since version can now be typed. --- .../framework/capabilityset/SimpleFilter.java | 37 +++-- .../util/manifestparser/ManifestParser.java | 32 +++- .../capabilityset/SimpleFilterTest.java | 10 ++ .../manifestparser/ManifestParserTest.java | 153 ++++++++++++++++++ 4 files changed, 207 insertions(+), 25 deletions(-) diff --git a/framework/src/main/java/org/apache/felix/framework/capabilityset/SimpleFilter.java b/framework/src/main/java/org/apache/felix/framework/capabilityset/SimpleFilter.java index c112d8ef17..d24f7b463d 100644 --- a/framework/src/main/java/org/apache/felix/framework/capabilityset/SimpleFilter.java +++ b/framework/src/main/java/org/apache/felix/framework/capabilityset/SimpleFilter.java @@ -21,6 +21,7 @@ import org.osgi.framework.VersionRange; import java.util.ArrayList; +import java.util.Collection; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -629,26 +630,18 @@ public static SimpleFilter convert(Map attrs) } } } - else + else if (entry.getValue() instanceof Collection) { - List values = SimpleFilter.parseSubstring(entry.getValue().toString()); - if (values.size() > 1) + // A list attribute matches if it contains all of the values. + for (Object value : (Collection) entry.getValue()) { - filters.add( - new SimpleFilter( - entry.getKey(), - values, - SimpleFilter.SUBSTRING)); - } - else - { - filters.add( - new SimpleFilter( - entry.getKey(), - values.get(0), - SimpleFilter.EQ)); + filters.add(convertValue(entry.getKey(), value)); } } + else + { + filters.add(convertValue(entry.getKey(), entry.getValue())); + } } SimpleFilter sf = null; @@ -657,7 +650,7 @@ public static SimpleFilter convert(Map attrs) { sf = filters.get(0); } - else if (attrs.size() > 1) + else if (filters.size() > 1) { sf = new SimpleFilter(null, filters, SimpleFilter.AND); } @@ -668,4 +661,14 @@ else if (filters.isEmpty()) return sf; } + + private static SimpleFilter convertValue(String name, Object value) + { + List values = SimpleFilter.parseSubstring(value.toString()); + if (values.size() > 1) + { + return new SimpleFilter(name, values, SimpleFilter.SUBSTRING); + } + return new SimpleFilter(name, values.get(0), SimpleFilter.EQ); + } } \ No newline at end of file diff --git a/framework/src/main/java/org/apache/felix/framework/util/manifestparser/ManifestParser.java b/framework/src/main/java/org/apache/felix/framework/util/manifestparser/ManifestParser.java index 670290e0eb..e0b569ccc0 100644 --- a/framework/src/main/java/org/apache/felix/framework/util/manifestparser/ManifestParser.java +++ b/framework/src/main/java/org/apache/felix/framework/util/manifestparser/ManifestParser.java @@ -334,6 +334,8 @@ private static List normalizeImportClauses( Logger logger, List clauses, String mv) throws BundleException { + convertTypedAttributes(clauses); + // Verify that the values are equals if the package specifies // both version and specification-version attributes. Set dupeSet = new HashSet<>(); @@ -346,7 +348,7 @@ private static List normalizeImportClauses( if ((v != null) && (sv != null)) { // Verify they are equal. - if (!((String) v).trim().equals(((String) sv).trim())) + if (!new VersionRange(v.toString().trim()).equals(new VersionRange(sv.toString().trim()))) { throw new IllegalArgumentException( "Both version and specification-version are specified, but they are not equal."); @@ -509,6 +511,8 @@ private static List normalizeDynamicImportClauses( Logger logger, List clauses, String mv) throws BundleException { + convertTypedAttributes(clauses); + // Verify that the values are equals if the package specifies // both version and specification-version attributes. for (ParsedHeaderClause clause : clauses) @@ -534,7 +538,7 @@ private static List normalizeDynamicImportClauses( if ((v != null) && (sv != null)) { // Verify they are equal. - if (!((String) v).trim().equals(((String) sv).trim())) + if (!new VersionRange(v.toString().trim()).equals(new VersionRange(sv.toString().trim()))) { throw new IllegalArgumentException( "Both version and specification-version are specified, but they are not equal."); @@ -736,6 +740,13 @@ private static List normalizeCapabilityClauses( // Should we error here if we are not an R4 bundle? } + return convertTypedAttributes(clauses); + } + + private static List convertTypedAttributes( + List clauses) + throws BundleException + { // Convert attributes into specified types. for (ParsedHeaderClause clause : clauses) { @@ -770,7 +781,7 @@ else if (type.startsWith("List")) || ((startIdx < 0) && (endIdx > 0))) { throw new BundleException( - "Invalid Provide-Capability attribute list type for '" + "Invalid attribute list type for '" + entry.getKey() + "' : " + type); @@ -806,7 +817,7 @@ else if (listType.equals("Long")) else { throw new BundleException( - "Unknown Provide-Capability attribute list type for '" + "Unknown attribute list type for '" + entry.getKey() + "' : " + type); @@ -819,7 +830,7 @@ else if (listType.equals("Long")) else { throw new BundleException( - "Unknown Provide-Capability attribute type for '" + "Unknown attribute type for '" + entry.getKey() + "' : " + type); @@ -874,6 +885,8 @@ private static List normalizeExportClauses( String mv, String bsn, Version bv, boolean connectModule) throws BundleException { + convertTypedAttributes(clauses); + for (ParsedHeaderClause clause : clauses) { // Verify that the named package has not already been declared. @@ -909,7 +922,7 @@ else if (pkgName.length() == 0) if ((v != null) && (sv != null)) { // Verify they are equal. - if (!((String) v).trim().equals(((String) sv).trim())) + if (!Version.parseVersion(v.toString()).equals(Version.parseVersion(sv.toString()))) { throw new IllegalArgumentException( "Both version and specification-version are specified, but they are not equal."); @@ -1549,8 +1562,8 @@ private static List parseFragmentHost( String mv = getManifestVersion(headerMap); if ((mv != null) && mv.equals("2")) { - List clauses = parseStandardHeader( - (String) headerMap.get(Constants.FRAGMENT_HOST)); + List clauses = convertTypedAttributes(parseStandardHeader( + (String) headerMap.get(Constants.FRAGMENT_HOST))); if (clauses.size() > 0) { // Make sure that only one fragment host symbolic name is specified. @@ -1749,6 +1762,7 @@ private static String getBreeVersionClause(Version ver) private static List normalizeRequireClauses( Logger logger, List clauses, String mv) + throws BundleException { // R3 bundles cannot require other bundles. if (!mv.equals("2")) @@ -1757,6 +1771,8 @@ private static List normalizeRequireClauses( } else { + convertTypedAttributes(clauses); + // Convert bundle version attribute to VersionRange type. for (ParsedHeaderClause clause : clauses) { diff --git a/framework/src/test/java/org/apache/felix/framework/capabilityset/SimpleFilterTest.java b/framework/src/test/java/org/apache/felix/framework/capabilityset/SimpleFilterTest.java index 7b1e2765c8..3e8d86d331 100644 --- a/framework/src/test/java/org/apache/felix/framework/capabilityset/SimpleFilterTest.java +++ b/framework/src/test/java/org/apache/felix/framework/capabilityset/SimpleFilterTest.java @@ -22,10 +22,20 @@ import org.junit.jupiter.api.Test; +import java.util.Arrays; +import java.util.Collections; import java.util.List; class SimpleFilterTest { + @Test + void convertListAttribute() + { + SimpleFilter sf = SimpleFilter.convert( + Collections.singletonMap("colors", Arrays.asList("red", "blue"))); + assertThat(sf).hasToString("(&(colors=red)(colors=blue))"); + } + @Test void substringMatching() { diff --git a/framework/src/test/java/org/apache/felix/framework/util/manifestparser/ManifestParserTest.java b/framework/src/test/java/org/apache/felix/framework/util/manifestparser/ManifestParserTest.java index e8dbdb10d8..81dc3ccc0f 100644 --- a/framework/src/test/java/org/apache/felix/framework/util/manifestparser/ManifestParserTest.java +++ b/framework/src/test/java/org/apache/felix/framework/util/manifestparser/ManifestParserTest.java @@ -41,8 +41,11 @@ import org.osgi.framework.InvalidSyntaxException; import org.osgi.framework.Version; import org.osgi.framework.connect.ConnectContent; +import org.osgi.framework.namespace.BundleNamespace; +import org.osgi.framework.namespace.HostNamespace; import org.osgi.framework.namespace.IdentityNamespace; import org.osgi.framework.namespace.NativeNamespace; +import org.osgi.framework.namespace.PackageNamespace; import org.osgi.framework.wiring.BundleCapability; import org.osgi.framework.wiring.BundleRequirement; @@ -165,6 +168,143 @@ void attributes() throws BundleException { assertThat(rList).hasSize(3); } + @Test + void exportPackageTypedAttributes() throws BundleException + { + Map headers = new HashMap<>(); + headers.put(Constants.BUNDLE_MANIFESTVERSION, "2"); + headers.put(Constants.BUNDLE_SYMBOLICNAME, "com.example.exporter"); + headers.put(Constants.EXPORT_PACKAGE, + "com.example;version=\"1.0\";x-directive:=directiveValue;attr=value;" + + "longAttr:Long=42;stringAttr:String=stringValue;doubleAttr:Double=4.2;" + + "versionAttr:Version=4.2;longList:List=\"2,3,4\";" + + "stringList:List=\"one,two,three\";doubleList:List=\"2.3,3.4,4.5\";" + + "versionList:List=\"2.3,3.4,4.5\"," + + "com.example.typed;version:Version=2.0"); + + ManifestParser mp = new ManifestParser(null, null, null, headers); + + BundleCapability pc = findPackageCapability(mp.getCapabilities(), "com.example"); + assertThat(pc.getAttributes()) + .containsEntry("attr", "value") + .containsEntry("longAttr", Long.valueOf(42)) + .containsEntry("stringAttr", "stringValue") + .containsEntry("doubleAttr", Double.valueOf(4.2)) + .containsEntry("versionAttr", new Version("4.2")) + .containsEntry("longList", Arrays.asList(Long.valueOf(2), Long.valueOf(3), Long.valueOf(4))) + .containsEntry("stringList", Arrays.asList("one", "two", "three")) + .containsEntry("doubleList", Arrays.asList(Double.valueOf(2.3), Double.valueOf(3.4), Double.valueOf(4.5))) + .containsEntry("versionList", Arrays.asList(new Version("2.3"), new Version("3.4"), new Version("4.5"))) + .containsEntry(PackageNamespace.CAPABILITY_VERSION_ATTRIBUTE, new Version("1.0")) + .doesNotContainKey("x-directive"); + assertThat(pc.getDirectives()).containsEntry("x-directive", "directiveValue"); + + BundleCapability typed = findPackageCapability(mp.getCapabilities(), "com.example.typed"); + assertThat(typed.getAttributes()) + .containsEntry(PackageNamespace.CAPABILITY_VERSION_ATTRIBUTE, new Version("2.0")); + } + + @Test + void typedVersionWithSpecificationVersion() throws BundleException + { + Map headers = new HashMap<>(); + headers.put(Constants.BUNDLE_MANIFESTVERSION, "2"); + headers.put(Constants.BUNDLE_SYMBOLICNAME, "com.example.versions"); + headers.put(Constants.EXPORT_PACKAGE, "com.example;version:Version=1.0;specification-version=\"1.0\""); + headers.put(Constants.IMPORT_PACKAGE, "com.example;version:Version=1.0;specification-version=\"1.0\""); + + ManifestParser mp = new ManifestParser(null, null, null, headers); + + assertThat(findPackageCapability(mp.getCapabilities(), "com.example").getAttributes()) + .containsEntry(PackageNamespace.CAPABILITY_VERSION_ATTRIBUTE, new Version("1.0")); + assertThat(findRequirement(mp.getRequirements(), PackageNamespace.PACKAGE_NAMESPACE).getDirectives()) + .containsEntry(Constants.FILTER_DIRECTIVE, "(&(osgi.wiring.package=com.example)(version>=1.0.0))"); + } + + @Test + void importPackageTypedAttributes() throws Exception + { + BundleCapability pc = findPackageCapability(typedExporter().getCapabilities(), "com.example"); + + Map headers = new HashMap<>(); + headers.put(Constants.BUNDLE_MANIFESTVERSION, "2"); + headers.put(Constants.BUNDLE_SYMBOLICNAME, "com.example.importer"); + headers.put(Constants.IMPORT_PACKAGE, "com.example;version:Version=1.0;" + TYPED_MATCHING_ATTRS); + ManifestParser mp = new ManifestParser(null, null, null, headers); + + assertMatches(findRequirement(mp.getRequirements(), PackageNamespace.PACKAGE_NAMESPACE), pc); + } + + @Test + void dynamicImportPackageTypedAttributes() throws Exception + { + BundleCapability pc = findPackageCapability(typedExporter().getCapabilities(), "com.example"); + + List reqs = ManifestParser.parseDynamicImportHeader(null, null, + "com.example;" + TYPED_MATCHING_ATTRS); + + assertMatches(findRequirement(reqs, PackageNamespace.PACKAGE_NAMESPACE), pc); + } + + @Test + void requireBundleTypedAttributes() throws Exception + { + BundleCapability bc = findCapability(typedHost().getCapabilities(), BundleNamespace.BUNDLE_NAMESPACE); + + Map headers = new HashMap<>(); + headers.put(Constants.BUNDLE_MANIFESTVERSION, "2"); + headers.put(Constants.BUNDLE_SYMBOLICNAME, "com.example.requirer"); + headers.put(Constants.REQUIRE_BUNDLE, "com.example.host;" + TYPED_MATCHING_ATTRS); + ManifestParser mp = new ManifestParser(null, null, null, headers); + + assertMatches(findRequirement(mp.getRequirements(), BundleNamespace.BUNDLE_NAMESPACE), bc); + } + + @Test + void fragmentHostTypedAttributes() throws Exception + { + BundleCapability hc = findCapability(typedHost().getCapabilities(), HostNamespace.HOST_NAMESPACE); + + Map headers = new HashMap<>(); + headers.put(Constants.BUNDLE_MANIFESTVERSION, "2"); + headers.put(Constants.BUNDLE_SYMBOLICNAME, "com.example.fragment"); + headers.put(Constants.FRAGMENT_HOST, "com.example.host;" + TYPED_MATCHING_ATTRS); + ManifestParser mp = new ManifestParser(null, null, null, headers); + + assertMatches(findRequirement(mp.getRequirements(), HostNamespace.HOST_NAMESPACE), hc); + } + + private static final String TYPED_ATTRS = "longAttr:Long=42;doubleAttr:Double=4.2;versionAttr:Version=4.2;" + + "stringList:List=\"one,two,three\";versionList:List=\"2.3,3.4,4.5\""; + + private static final String TYPED_MATCHING_ATTRS = "longAttr:Long=42;doubleAttr:Double=4.2;versionAttr:Version=4.2;" + + "stringList:List=\"one,three\";versionList:List=\"3.4\""; + + private static ManifestParser typedExporter() throws BundleException + { + Map headers = new HashMap<>(); + headers.put(Constants.BUNDLE_MANIFESTVERSION, "2"); + headers.put(Constants.BUNDLE_SYMBOLICNAME, "com.example.exporter"); + headers.put(Constants.EXPORT_PACKAGE, "com.example;version=1.0;" + TYPED_ATTRS); + return new ManifestParser(null, null, null, headers); + } + + private static ManifestParser typedHost() throws BundleException + { + Map headers = new HashMap<>(); + headers.put(Constants.BUNDLE_MANIFESTVERSION, "2"); + headers.put(Constants.BUNDLE_SYMBOLICNAME, "com.example.host;" + TYPED_ATTRS); + return new ManifestParser(null, null, null, headers); + } + + private static void assertMatches(BundleRequirement req, BundleCapability cap) throws InvalidSyntaxException + { + assertThat(req.matches(cap)).as("%s matches %s", req, cap).isTrue(); + String filter = req.getDirectives().get(Constants.FILTER_DIRECTIVE); + assertThat(FrameworkUtil.createFilter(filter).matches(cap.getAttributes())) + .as("%s matches %s", filter, cap.getAttributes()).isTrue(); + } + @Test void convertNativeCode() throws InvalidSyntaxException { @@ -214,6 +354,19 @@ private BundleCapability findCapability(Collection capabilitie return null; } + private BundleCapability findPackageCapability(Collection capabilities, String pkgName) + { + for (BundleCapability capability : capabilities) + { + if (PackageNamespace.PACKAGE_NAMESPACE.equals(capability.getNamespace()) + && pkgName.equals(capability.getAttributes().get(PackageNamespace.PACKAGE_NAMESPACE))) + { + return capability; + } + } + return null; + } + private BundleRequirement findRequirement(Collection requirements, String namespace) { for(BundleRequirement requirement: requirements) From 6268090cfd4af908ff7fc06818520fff40254e76 Mon Sep 17 00:00:00 2001 From: Stefan Bischof Date: Tue, 3 Nov 2026 18:05:53 +0100 Subject: [PATCH 2/2] Address review of the typed attribute conversion Malformed typed values, unknown attribute types and a mismatch of version and specification-version now fail with a BundleException (MANIFEST_ERROR) that names the attribute, instead of a raw NumberFormatException or IllegalArgumentException. Typed attributes are converted for R4 bundles only in Import-Package, DynamicImport-Package and Export-Package, so R3 bundles keep dropping unknown attributes with a warning. The version, specification-version and bundle-version attributes must be of type String or Version. Since the previous commit compares versions instead of strings, version="1.0" and specification-version="1.0.0" are now accepted as equal, which was rejected before. An empty list in a requirement header now only requires the attribute to be present instead of matching any capability. The list semantics are documented on SimpleFilter.convert. The attribute types are cleared after conversion, so converting a clause again does not split a list a second time. Adds tests for a single bounded version range, empty lists, values that must not match, malformed values, unknown types, R3 bundles and a typed bundle-version on Require-Bundle and Fragment-Host. --- .../framework/capabilityset/SimpleFilter.java | 18 +- .../util/manifestparser/ManifestParser.java | 386 +++++++++--------- .../capabilityset/SimpleFilterTest.java | 24 ++ .../manifestparser/ManifestParserTest.java | 201 +++++++-- 4 files changed, 400 insertions(+), 229 deletions(-) diff --git a/framework/src/main/java/org/apache/felix/framework/capabilityset/SimpleFilter.java b/framework/src/main/java/org/apache/felix/framework/capabilityset/SimpleFilter.java index d24f7b463d..e560cb0733 100644 --- a/framework/src/main/java/org/apache/felix/framework/capabilityset/SimpleFilter.java +++ b/framework/src/main/java/org/apache/felix/framework/capabilityset/SimpleFilter.java @@ -570,8 +570,12 @@ private static int skipWhitespace(String s, int startIdx) * over the map's entry set. If ordering of attributes is important (e.g., * for hitting attribute indices), then the map's entry set should iterate * in the desired order. Equality testing is assumed for all attribute types - * other than version ranges, which are handled appropriated. If the attribute - * map is empty, then a filter that matches anything is returned. + * other than version ranges and collections, which are handled appropriated. + * A collection, such as a {@code List} typed attribute, becomes one equality + * test per element, so it matches a capability attribute that contains all + * of the elements; the OSGi specification does not define list matching for + * these headers. An empty collection only tests that the attribute is present. + * If the attribute map is empty, then a filter that matches anything is returned. * @param attrs Map of attributes to convert to a filter. * @return A filter corresponding to the attributes. */ @@ -632,8 +636,14 @@ public static SimpleFilter convert(Map attrs) } else if (entry.getValue() instanceof Collection) { - // A list attribute matches if it contains all of the values. - for (Object value : (Collection) entry.getValue()) + // A list attribute matches if it contains all of the values, + // an empty list still requires the attribute to be present. + Collection values = (Collection) entry.getValue(); + if (values.isEmpty()) + { + filters.add(new SimpleFilter(entry.getKey(), "*", SimpleFilter.PRESENT)); + } + for (Object value : values) { filters.add(convertValue(entry.getKey(), value)); } diff --git a/framework/src/main/java/org/apache/felix/framework/util/manifestparser/ManifestParser.java b/framework/src/main/java/org/apache/felix/framework/util/manifestparser/ManifestParser.java index e0b569ccc0..181cf3971f 100644 --- a/framework/src/main/java/org/apache/felix/framework/util/manifestparser/ManifestParser.java +++ b/framework/src/main/java/org/apache/felix/framework/util/manifestparser/ManifestParser.java @@ -334,46 +334,18 @@ private static List normalizeImportClauses( Logger logger, List clauses, String mv) throws BundleException { - convertTypedAttributes(clauses); + // Typed attributes are an R4 feature, R3 bundles keep their lenient handling. + if (mv.equals("2")) + { + convertTypedAttributes(clauses); + } // Verify that the values are equals if the package specifies // both version and specification-version attributes. Set dupeSet = new HashSet<>(); for (ParsedHeaderClause clause : clauses) { - // Check for "version" and "specification-version" attributes - // and verify they are the same if both are specified. - Object v = clause.m_attrs.get(Constants.VERSION_ATTRIBUTE); - Object sv = clause.m_attrs.get(Constants.PACKAGE_SPECIFICATION_VERSION); - if ((v != null) && (sv != null)) - { - // Verify they are equal. - if (!new VersionRange(v.toString().trim()).equals(new VersionRange(sv.toString().trim()))) - { - throw new IllegalArgumentException( - "Both version and specification-version are specified, but they are not equal."); - } - } - - // Ensure that only the "version" attribute is used and convert - // it to the VersionRange type. - if ((v != null) || (sv != null)) - { - clause.m_attrs.remove(Constants.PACKAGE_SPECIFICATION_VERSION); - v = (v == null) ? sv : v; - clause.m_attrs.put( - Constants.VERSION_ATTRIBUTE, - new VersionRange(v.toString())); - } - - // If bundle version is specified, then convert its type to VersionRange. - v = clause.m_attrs.get(Constants.BUNDLE_VERSION_ATTRIBUTE); - if (v != null) - { - clause.m_attrs.put( - Constants.BUNDLE_VERSION_ATTRIBUTE, - new VersionRange(v.toString())); - } + convertVersionRanges(clause); // Verify no duplicate imports. for (String pkgName : clause.m_paths) @@ -511,7 +483,11 @@ private static List normalizeDynamicImportClauses( Logger logger, List clauses, String mv) throws BundleException { - convertTypedAttributes(clauses); + // Typed attributes are an R4 feature, R3 bundles keep their lenient handling. + if (mv.equals("2")) + { + convertTypedAttributes(clauses); + } // Verify that the values are equals if the package specifies // both version and specification-version attributes. @@ -531,39 +507,7 @@ private static List normalizeDynamicImportClauses( clause.m_dirs.put(Constants.RESOLUTION_DIRECTIVE, FelixConstants.RESOLUTION_DYNAMIC); - // Check for "version" and "specification-version" attributes - // and verify they are the same if both are specified. - Object v = clause.m_attrs.get(Constants.VERSION_ATTRIBUTE); - Object sv = clause.m_attrs.get(Constants.PACKAGE_SPECIFICATION_VERSION); - if ((v != null) && (sv != null)) - { - // Verify they are equal. - if (!new VersionRange(v.toString().trim()).equals(new VersionRange(sv.toString().trim()))) - { - throw new IllegalArgumentException( - "Both version and specification-version are specified, but they are not equal."); - } - } - - // Ensure that only the "version" attribute is used and convert - // it to the VersionRange type. - if ((v != null) || (sv != null)) - { - clause.m_attrs.remove(Constants.PACKAGE_SPECIFICATION_VERSION); - v = (v == null) ? sv : v; - clause.m_attrs.put( - Constants.VERSION_ATTRIBUTE, - new VersionRange(v.toString())); - } - - // If bundle version is specified, then convert its type to VersionRange. - v = clause.m_attrs.get(Constants.BUNDLE_VERSION_ATTRIBUTE); - if (v != null) - { - clause.m_attrs.put( - Constants.BUNDLE_VERSION_ATTRIBUTE, - new VersionRange(v.toString())); - } + convertVersionRanges(clause); // Dynamic imports can have duplicates, verify that no partial package name wild carding is used for (String pkgName : clause.m_paths) @@ -752,94 +696,181 @@ private static List convertTypedAttributes( { for (Entry entry : clause.m_types.entrySet()) { - String type = entry.getValue(); - if (!type.equals("String")) + Object value = clause.m_attrs.get(entry.getKey()); + try { - if (type.equals("Double")) - { - clause.m_attrs.put( - entry.getKey(), - Double.valueOf(clause.m_attrs.get(entry.getKey()).toString().trim())); - } - else if (type.equals("Version")) - { - clause.m_attrs.put( - entry.getKey(), - new Version(clause.m_attrs.get(entry.getKey()).toString().trim())); - } - else if (type.equals("Long")) - { - clause.m_attrs.put( - entry.getKey(), - Long.valueOf(clause.m_attrs.get(entry.getKey()).toString().trim())); - } - else if (type.startsWith("List")) - { - int startIdx = type.indexOf('<'); - int endIdx = type.indexOf('>'); - if (((startIdx > 0) && (endIdx <= startIdx)) - || ((startIdx < 0) && (endIdx > 0))) - { - throw new BundleException( - "Invalid attribute list type for '" - + entry.getKey() - + "' : " - + type); - } + clause.m_attrs.put( + entry.getKey(), + convertTypedValue(entry.getKey(), entry.getValue(), value.toString())); + } + catch (IllegalArgumentException ex) + { + throw new BundleException( + "Invalid value for attribute '" + + entry.getKey() + + "' of type " + + entry.getValue() + + " : " + + value, + BundleException.MANIFEST_ERROR, + ex); + } + } + // The attributes have their types now, so converting the clause + // again must not split a list value a second time. + clause.m_types.clear(); + } - String listType = "String"; - if (endIdx > startIdx) - { - listType = type.substring(startIdx + 1, endIdx).trim(); - } + return clauses; + } - List tokens = parseDelimitedString( - clause.m_attrs.get(entry.getKey()).toString(), ",", false); - List values = new ArrayList<>(tokens.size()); - for (String token : tokens) - { - if (listType.equals("String")) - { - values.add(token); - } - else if (listType.equals("Double")) - { - values.add(Double.valueOf(token.trim())); - } - else if (listType.equals("Version")) - { - values.add(new Version(token.trim())); - } - else if (listType.equals("Long")) - { - values.add(Long.valueOf(token.trim())); - } - else - { - throw new BundleException( - "Unknown attribute list type for '" - + entry.getKey() - + "' : " - + type); - } - } - clause.m_attrs.put( - entry.getKey(), - values); - } - else - { - throw new BundleException( - "Unknown attribute type for '" - + entry.getKey() - + "' : " - + type); - } + private static Object convertTypedValue(String key, String type, String value) + throws BundleException + { + if (type.equals("String")) + { + return value; + } + else if (type.equals("Double")) + { + return Double.valueOf(value.trim()); + } + else if (type.equals("Version")) + { + return new Version(value.trim()); + } + else if (type.equals("Long")) + { + return Long.valueOf(value.trim()); + } + else if (type.startsWith("List")) + { + int startIdx = type.indexOf('<'); + int endIdx = type.indexOf('>'); + if (((startIdx > 0) && (endIdx <= startIdx)) + || ((startIdx < 0) && (endIdx > 0))) + { + throw new BundleException( + "Invalid attribute list type for '" + + key + + "' : " + + type, + BundleException.MANIFEST_ERROR); + } + + String listType = "String"; + if (endIdx > startIdx) + { + listType = type.substring(startIdx + 1, endIdx).trim(); + } + + List tokens = parseDelimitedString(value, ",", false); + List values = new ArrayList<>(tokens.size()); + for (String token : tokens) + { + if (listType.equals("String")) + { + values.add(token); + } + else if (listType.equals("Double")) + { + values.add(Double.valueOf(token.trim())); + } + else if (listType.equals("Version")) + { + values.add(new Version(token.trim())); + } + else if (listType.equals("Long")) + { + values.add(Long.valueOf(token.trim())); + } + else + { + throw new BundleException( + "Unknown attribute list type for '" + + key + + "' : " + + type, + BundleException.MANIFEST_ERROR); } } + return values; + } + throw new BundleException( + "Unknown attribute type for '" + + key + + "' : " + + type, + BundleException.MANIFEST_ERROR); + } + + // Converts the version and bundle-version attributes of an import + // clause to the VersionRange type. + private static void convertVersionRanges(ParsedHeaderClause clause) + throws BundleException + { + // Ensure that only the "version" attribute is used. + String v = getPackageVersion(clause, true); + if (v != null) + { + clause.m_attrs.put( + Constants.VERSION_ATTRIBUTE, + new VersionRange(v)); } - return clauses; + v = getVersionAttribute(clause, Constants.BUNDLE_VERSION_ATTRIBUTE); + if (v != null) + { + clause.m_attrs.put( + Constants.BUNDLE_VERSION_ATTRIBUTE, + new VersionRange(v)); + } + } + + // Merges the deprecated specification-version attribute into the version + // attribute and returns the version, or null if neither is specified. + private static String getPackageVersion(ParsedHeaderClause clause, boolean range) + throws BundleException + { + String v = getVersionAttribute(clause, Constants.VERSION_ATTRIBUTE); + String sv = getVersionAttribute(clause, Constants.PACKAGE_SPECIFICATION_VERSION); + clause.m_attrs.remove(Constants.PACKAGE_SPECIFICATION_VERSION); + if ((v != null) && (sv != null)) + { + // Compare versions rather than strings, a typed version:Version=1.0 + // is 1.0.0 while specification-version="1.0" stays a string. + boolean equal = range + ? new VersionRange(v).equals(new VersionRange(sv)) + : Version.parseVersion(v).equals(Version.parseVersion(sv)); + if (!equal) + { + throw new BundleException( + "Both version and specification-version are specified, but they are not equal.", + BundleException.MANIFEST_ERROR); + } + } + return (v != null) ? v : sv; + } + + // Returns a version attribute as a string, or null if it is not specified. + private static String getVersionAttribute(ParsedHeaderClause clause, String name) + throws BundleException + { + Object value = clause.m_attrs.get(name); + if (value == null) + { + return null; + } + if (!(value instanceof String) && !(value instanceof Version)) + { + throw new BundleException( + "Attribute '" + + name + + "' must be of type String or Version : " + + value, + BundleException.MANIFEST_ERROR); + } + return value.toString().trim(); } private static List convertProvideCapabilities( @@ -885,7 +916,11 @@ private static List normalizeExportClauses( String mv, String bsn, Version bv, boolean connectModule) throws BundleException { - convertTypedAttributes(clauses); + // Typed attributes are an R4 feature, R3 bundles keep their lenient handling. + if (mv.equals("2")) + { + convertTypedAttributes(clauses); + } for (ParsedHeaderClause clause : clauses) { @@ -915,37 +950,12 @@ else if (pkgName.length() == 0) } } - // Check for "version" and "specification-version" attributes - // and verify they are the same if both are specified. - Object v = clause.m_attrs.get(Constants.VERSION_ATTRIBUTE); - Object sv = clause.m_attrs.get(Constants.PACKAGE_SPECIFICATION_VERSION); - if ((v != null) && (sv != null)) - { - // Verify they are equal. - if (!Version.parseVersion(v.toString()).equals(Version.parseVersion(sv.toString()))) - { - throw new IllegalArgumentException( - "Both version and specification-version are specified, but they are not equal."); - } - } - - // Always add the default version if not specified. - if ((v == null) && (sv == null)) - { - v = Version.emptyVersion; - } - // Ensure that only the "version" attribute is used and convert - // it to the appropriate type. - if ((v != null) || (sv != null)) - { - // Convert version attribute to type Version. - clause.m_attrs.remove(Constants.PACKAGE_SPECIFICATION_VERSION); - v = (v == null) ? sv : v; - clause.m_attrs.put( - Constants.VERSION_ATTRIBUTE, - Version.parseVersion(v.toString())); - } + // it to type Version, always adding the default version if not specified. + String v = getPackageVersion(clause, false); + clause.m_attrs.put( + Constants.VERSION_ATTRIBUTE, + (v == null) ? Version.emptyVersion : Version.parseVersion(v)); // If this is an R4 bundle, then make sure it doesn't specify // bundle symbolic name or bundle version attributes. @@ -1582,14 +1592,10 @@ else if (clauses.get(0).m_paths.size() > 1) // If the bundle-version attribute is specified, then convert // it to the proper type. - Object value = clauses.get(0).m_attrs.get(Constants.BUNDLE_VERSION_ATTRIBUTE); - value = (value == null) ? "0.0.0" : value; - if (value != null) - { - clauses.get(0).m_attrs.put( - Constants.BUNDLE_VERSION_ATTRIBUTE, - new VersionRange(value.toString())); - } + String value = getVersionAttribute(clauses.get(0), Constants.BUNDLE_VERSION_ATTRIBUTE); + clauses.get(0).m_attrs.put( + Constants.BUNDLE_VERSION_ATTRIBUTE, + new VersionRange((value == null) ? "0.0.0" : value)); // Note that we use a linked hash map here to ensure the // host symbolic name is first, which will make indexing @@ -1776,12 +1782,12 @@ private static List normalizeRequireClauses( // Convert bundle version attribute to VersionRange type. for (ParsedHeaderClause clause : clauses) { - Object value = clause.m_attrs.get(Constants.BUNDLE_VERSION_ATTRIBUTE); + String value = getVersionAttribute(clause, Constants.BUNDLE_VERSION_ATTRIBUTE); if (value != null) { clause.m_attrs.put( Constants.BUNDLE_VERSION_ATTRIBUTE, - new VersionRange(value.toString())); + new VersionRange(value)); } } } diff --git a/framework/src/test/java/org/apache/felix/framework/capabilityset/SimpleFilterTest.java b/framework/src/test/java/org/apache/felix/framework/capabilityset/SimpleFilterTest.java index 3e8d86d331..bb02e78454 100644 --- a/framework/src/test/java/org/apache/felix/framework/capabilityset/SimpleFilterTest.java +++ b/framework/src/test/java/org/apache/felix/framework/capabilityset/SimpleFilterTest.java @@ -21,10 +21,13 @@ import static org.assertj.core.api.Assertions.assertThat; import org.junit.jupiter.api.Test; +import org.osgi.framework.VersionRange; import java.util.Arrays; import java.util.Collections; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; class SimpleFilterTest { @@ -36,6 +39,27 @@ void convertListAttribute() assertThat(sf).hasToString("(&(colors=red)(colors=blue))"); } + @Test + void convertEmptyListAttribute() + { + SimpleFilter sf = SimpleFilter.convert( + Collections.singletonMap("colors", Collections.emptyList())); + assertThat(sf).hasToString("(colors=*)").isEqualTo(SimpleFilter.parse("(colors=*)")); + + Map attrs = new LinkedHashMap<>(); + attrs.put("osgi.wiring.package", "org.example"); + attrs.put("colors", Collections.emptyList()); + assertThat(SimpleFilter.convert(attrs)).hasToString("(&(osgi.wiring.package=org.example)(colors=*))"); + } + + @Test + void convertSingleBoundedVersionRange() + { + SimpleFilter sf = SimpleFilter.convert( + Collections.singletonMap("version", new VersionRange("[1.0,2.0)"))); + assertThat(sf).hasToString("(&(version>=1.0.0)(!(version>=2.0.0)))"); + } + @Test void substringMatching() { diff --git a/framework/src/test/java/org/apache/felix/framework/util/manifestparser/ManifestParserTest.java b/framework/src/test/java/org/apache/felix/framework/util/manifestparser/ManifestParserTest.java index 81dc3ccc0f..be89adce27 100644 --- a/framework/src/test/java/org/apache/felix/framework/util/manifestparser/ManifestParserTest.java +++ b/framework/src/test/java/org/apache/felix/framework/util/manifestparser/ManifestParserTest.java @@ -19,6 +19,7 @@ package org.apache.felix.framework.util.manifestparser; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -30,6 +31,7 @@ import java.util.Map; import org.apache.felix.framework.BundleRevisionImpl; +import org.apache.felix.framework.Logger; import org.apache.felix.framework.cache.ConnectContentContent; import org.apache.felix.framework.cache.Content; import org.apache.felix.framework.util.FelixConstants; @@ -226,15 +228,41 @@ void importPackageTypedAttributes() throws Exception { BundleCapability pc = findPackageCapability(typedExporter().getCapabilities(), "com.example"); - Map headers = new HashMap<>(); - headers.put(Constants.BUNDLE_MANIFESTVERSION, "2"); - headers.put(Constants.BUNDLE_SYMBOLICNAME, "com.example.importer"); - headers.put(Constants.IMPORT_PACKAGE, "com.example;version:Version=1.0;" + TYPED_MATCHING_ATTRS); - ManifestParser mp = new ManifestParser(null, null, null, headers); + ManifestParser mp = parse("com.example.importer", + Constants.IMPORT_PACKAGE, "com.example;version:Version=1.0;" + TYPED_MATCHING_ATTRS); assertMatches(findRequirement(mp.getRequirements(), PackageNamespace.PACKAGE_NAMESPACE), pc); } + @Test + void importPackageTypedAttributesNotMatching() throws Exception + { + BundleCapability pc = findPackageCapability(typedExporter().getCapabilities(), "com.example"); + + for (String attr : Arrays.asList("longAttr:Long=43", "doubleAttr:Double=4.3", + "versionAttr:Version=4.3", "stringList:List=\"one,four\"", + "versionList:List=\"3.5\"")) + { + ManifestParser mp = parse("com.example.importer", Constants.IMPORT_PACKAGE, "com.example;" + attr); + + assertNotMatches(findRequirement(mp.getRequirements(), PackageNamespace.PACKAGE_NAMESPACE), pc); + } + } + + @Test + void importPackageEmptyListRequiresAttribute() throws Exception + { + BundleCapability pc = findPackageCapability(typedExporter().getCapabilities(), "com.example"); + + ManifestParser present = parse("com.example.importer", + Constants.IMPORT_PACKAGE, "com.example;stringList:List=\"\""); + ManifestParser missing = parse("com.example.importer", + Constants.IMPORT_PACKAGE, "com.example;otherList:List=\"\""); + + assertMatches(findRequirement(present.getRequirements(), PackageNamespace.PACKAGE_NAMESPACE), pc); + assertNotMatches(findRequirement(missing.getRequirements(), PackageNamespace.PACKAGE_NAMESPACE), pc); + } + @Test void dynamicImportPackageTypedAttributes() throws Exception { @@ -251,58 +279,109 @@ void requireBundleTypedAttributes() throws Exception { BundleCapability bc = findCapability(typedHost().getCapabilities(), BundleNamespace.BUNDLE_NAMESPACE); - Map headers = new HashMap<>(); - headers.put(Constants.BUNDLE_MANIFESTVERSION, "2"); - headers.put(Constants.BUNDLE_SYMBOLICNAME, "com.example.requirer"); - headers.put(Constants.REQUIRE_BUNDLE, "com.example.host;" + TYPED_MATCHING_ATTRS); - ManifestParser mp = new ManifestParser(null, null, null, headers); + ManifestParser mp = parse("com.example.requirer", + Constants.REQUIRE_BUNDLE, "com.example.host;" + TYPED_MATCHING_ATTRS); assertMatches(findRequirement(mp.getRequirements(), BundleNamespace.BUNDLE_NAMESPACE), bc); } + @Test + void requireBundleTypedBundleVersion() throws Exception + { + BundleCapability bc = findCapability(typedHost().getCapabilities(), BundleNamespace.BUNDLE_NAMESPACE); + + ManifestParser matching = parse("com.example.requirer", + Constants.REQUIRE_BUNDLE, "com.example.host;bundle-version:Version=1.0"); + ManifestParser notMatching = parse("com.example.requirer", + Constants.REQUIRE_BUNDLE, "com.example.host;bundle-version:Version=2.0"); + + assertMatches(findRequirement(matching.getRequirements(), BundleNamespace.BUNDLE_NAMESPACE), bc); + assertNotMatches(findRequirement(notMatching.getRequirements(), BundleNamespace.BUNDLE_NAMESPACE), bc); + } + @Test void fragmentHostTypedAttributes() throws Exception { BundleCapability hc = findCapability(typedHost().getCapabilities(), HostNamespace.HOST_NAMESPACE); - Map headers = new HashMap<>(); - headers.put(Constants.BUNDLE_MANIFESTVERSION, "2"); - headers.put(Constants.BUNDLE_SYMBOLICNAME, "com.example.fragment"); - headers.put(Constants.FRAGMENT_HOST, "com.example.host;" + TYPED_MATCHING_ATTRS); - ManifestParser mp = new ManifestParser(null, null, null, headers); + ManifestParser mp = parse("com.example.fragment", + Constants.FRAGMENT_HOST, "com.example.host;" + TYPED_MATCHING_ATTRS); assertMatches(findRequirement(mp.getRequirements(), HostNamespace.HOST_NAMESPACE), hc); } - private static final String TYPED_ATTRS = "longAttr:Long=42;doubleAttr:Double=4.2;versionAttr:Version=4.2;" - + "stringList:List=\"one,two,three\";versionList:List=\"2.3,3.4,4.5\""; + @Test + void fragmentHostTypedBundleVersion() throws Exception + { + BundleCapability hc = findCapability(typedHost().getCapabilities(), HostNamespace.HOST_NAMESPACE); - private static final String TYPED_MATCHING_ATTRS = "longAttr:Long=42;doubleAttr:Double=4.2;versionAttr:Version=4.2;" - + "stringList:List=\"one,three\";versionList:List=\"3.4\""; + ManifestParser matching = parse("com.example.fragment", + Constants.FRAGMENT_HOST, "com.example.host;bundle-version:Version=1.0"); + ManifestParser notMatching = parse("com.example.fragment", + Constants.FRAGMENT_HOST, "com.example.host;bundle-version:Version=2.0"); - private static ManifestParser typedExporter() throws BundleException + assertMatches(findRequirement(matching.getRequirements(), HostNamespace.HOST_NAMESPACE), hc); + assertNotMatches(findRequirement(notMatching.getRequirements(), HostNamespace.HOST_NAMESPACE), hc); + } + + @Test + void malformedTypedValue() { - Map headers = new HashMap<>(); - headers.put(Constants.BUNDLE_MANIFESTVERSION, "2"); - headers.put(Constants.BUNDLE_SYMBOLICNAME, "com.example.exporter"); - headers.put(Constants.EXPORT_PACKAGE, "com.example;version=1.0;" + TYPED_ATTRS); - return new ManifestParser(null, null, null, headers); + assertManifestError("'longAttr' of type Long : abc", + Constants.EXPORT_PACKAGE, "com.example;longAttr:Long=abc"); + assertManifestError("'versionList' of type List : 1.0,x", + Constants.IMPORT_PACKAGE, "com.example;versionList:List=\"1.0,x\""); + assertManifestError("'doubleAttr' of type Double : x", + Constants.REQUIRE_BUNDLE, "com.example.host;doubleAttr:Double=x"); } - private static ManifestParser typedHost() throws BundleException + @Test + void versionAttributeMustBeStringOrVersion() { - Map headers = new HashMap<>(); - headers.put(Constants.BUNDLE_MANIFESTVERSION, "2"); - headers.put(Constants.BUNDLE_SYMBOLICNAME, "com.example.host;" + TYPED_ATTRS); - return new ManifestParser(null, null, null, headers); + assertManifestError("'version' must be of type String or Version", + Constants.EXPORT_PACKAGE, "com.example;version:List=\"1.0,2.0\""); + assertManifestError("'version' must be of type String or Version", + Constants.IMPORT_PACKAGE, "com.example;version:Long=1"); + assertManifestError("'bundle-version' must be of type String or Version", + Constants.FRAGMENT_HOST, "com.example.host;bundle-version:List=\"1.0,2.0\""); } - private static void assertMatches(BundleRequirement req, BundleCapability cap) throws InvalidSyntaxException + @Test + void unknownTypedAttributeType() { - assertThat(req.matches(cap)).as("%s matches %s", req, cap).isTrue(); - String filter = req.getDirectives().get(Constants.FILTER_DIRECTIVE); - assertThat(FrameworkUtil.createFilter(filter).matches(cap.getAttributes())) - .as("%s matches %s", filter, cap.getAttributes()).isTrue(); + assertManifestError("Unknown attribute type for 'x' : Foo", + Constants.EXPORT_PACKAGE, "com.example;x:Foo=1"); + assertManifestError("Unknown attribute list type for 'x' : List", + Constants.IMPORT_PACKAGE, "com.example;x:List=1"); + assertManifestError("Invalid attribute list type for 'x' : List headers = new HashMap<>(); + headers.put(Constants.BUNDLE_SYMBOLICNAME, "com.example.r3"); + headers.put(Constants.EXPORT_PACKAGE, "com.example;x:Long=abc"); + headers.put(Constants.IMPORT_PACKAGE, "com.example.other;x:Long=abc"); + + ManifestParser mp = new ManifestParser(new Logger(), null, null, headers); + + assertThat(findPackageCapability(mp.getCapabilities(), "com.example").getAttributes()) + .doesNotContainKey("x"); + assertThat(findRequirement(mp.getRequirements(), PackageNamespace.PACKAGE_NAMESPACE).getDirectives()) + .containsEntry(Constants.FILTER_DIRECTIVE, "(&(osgi.wiring.package=com.example.other)(version>=0.0.0))"); } @Test @@ -378,4 +457,56 @@ private BundleRequirement findRequirement(Collection requirem } return null; } + + private static final String TYPED_ATTRS = "longAttr:Long=42;doubleAttr:Double=4.2;versionAttr:Version=4.2;" + + "stringList:List=\"one,two,three\";versionList:List=\"2.3,3.4,4.5\""; + + private static final String TYPED_MATCHING_ATTRS = "longAttr:Long=42;doubleAttr:Double=4.2;versionAttr:Version=4.2;" + + "stringList:List=\"one,three\";versionList:List=\"3.4\""; + + private static ManifestParser typedExporter() throws BundleException + { + return parse("com.example.exporter", Constants.EXPORT_PACKAGE, "com.example;version=1.0;" + TYPED_ATTRS); + } + + private static ManifestParser typedHost() throws BundleException + { + return parse("com.example.host;" + TYPED_ATTRS, Constants.BUNDLE_VERSION, "1.2.0"); + } + + private static ManifestParser parse(String bsn, String... headers) throws BundleException + { + Map map = new HashMap<>(); + map.put(Constants.BUNDLE_MANIFESTVERSION, "2"); + map.put(Constants.BUNDLE_SYMBOLICNAME, bsn); + for (int i = 0; i < headers.length; i += 2) + { + map.put(headers[i], headers[i + 1]); + } + return new ManifestParser(null, null, null, map); + } + + private static void assertMatches(BundleRequirement req, BundleCapability cap) throws InvalidSyntaxException + { + assertThat(req.matches(cap)).as("%s matches %s", req, cap).isTrue(); + String filter = req.getDirectives().get(Constants.FILTER_DIRECTIVE); + assertThat(FrameworkUtil.createFilter(filter).matches(cap.getAttributes())) + .as("%s matches %s", filter, cap.getAttributes()).isTrue(); + } + + private static void assertNotMatches(BundleRequirement req, BundleCapability cap) throws InvalidSyntaxException + { + assertThat(req.matches(cap)).as("%s does not match %s", req, cap).isFalse(); + String filter = req.getDirectives().get(Constants.FILTER_DIRECTIVE); + assertThat(FrameworkUtil.createFilter(filter).matches(cap.getAttributes())) + .as("%s does not match %s", filter, cap.getAttributes()).isFalse(); + } + + private static void assertManifestError(String message, String... headers) + { + assertThatThrownBy(() -> parse("com.example.test", headers)) + .isInstanceOfSatisfying(BundleException.class, + ex -> assertThat(ex.getType()).isEqualTo(BundleException.MANIFEST_ERROR)) + .hasMessageContaining(message); + } }