Bug 565723 - Allow connect bundles to export java.* packages
Change-Id: I0e8e2a2084056757e4b17e5f9bd5eaaeb2c2e89b
Signed-off-by: Thomas Watson <tjwatson@us.ibm.com>
diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ConnectTests.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ConnectTests.java
index c84ab16..4ca64c6 100644
--- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ConnectTests.java
+++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ConnectTests.java
@@ -70,6 +70,7 @@
import org.osgi.framework.namespace.BundleNamespace;
import org.osgi.framework.namespace.HostNamespace;
import org.osgi.framework.namespace.IdentityNamespace;
+import org.osgi.framework.namespace.PackageNamespace;
import org.osgi.framework.wiring.BundleCapability;
import org.osgi.framework.wiring.BundleRevision;
import org.osgi.framework.wiring.BundleWiring;
@@ -892,7 +893,7 @@
Map<String, String> headers = new HashMap<>(FrameworkUtil.asMap(systemBundle.getHeaders()));
headers.put("test.key", "test.value");
// remove bundle manifest version to allow java export
- headers.remove(Constants.BUNDLE_MANIFESTVERSION);
+ // headers.remove(Constants.BUNDLE_MANIFESTVERSION);
TestConnectModule systemModule = new TestConnectModule(
new TestConnectContent(headers, systemBundle.adapt(BundleWiring.class).getClassLoader()));
@@ -913,6 +914,36 @@
doTestConnect(connector, Collections.emptyMap(), test);
}
+ public void testJavaExportsConnect() {
+ TestCountingModuleConnector connector = new TestCountingModuleConnector();
+ connector.setModule("javaExport", createJavaExportModule());
+
+ doTestConnect(connector, Collections.emptyMap(), (f) -> {
+ try {
+ f.start();
+ Bundle b = f.getBundleContext().installBundle("javaExport");
+ b.start();
+ assertTrue("No java export found.",
+ b.adapt(BundleWiring.class).getCapabilities(PackageNamespace.PACKAGE_NAMESPACE).stream()
+ .findFirst()
+ .map((c) -> "java.test.export"
+ .equals(c.getAttributes().get(PackageNamespace.PACKAGE_NAMESPACE)))
+ .orElse(false));
+ } catch (Throwable t) {
+ sneakyThrow(t);
+ }
+ });
+ }
+
+ private ConnectModule createJavaExportModule() {
+ Map<String, String> headers = new HashMap<>();
+ headers.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+ headers.put(Constants.BUNDLE_SYMBOLICNAME, "javaExport");
+ headers.put(Constants.EXPORT_PACKAGE, "java.test.export");
+ TestConnectContent c = new TestConnectContent(headers, null);
+ return new TestConnectModule(c);
+ }
+
private void checkHeaders(Map<String, String> expected, Dictionary<String, String> actual) {
assertEquals("Headers size not equals", expected.size(), actual.size());
for (Entry<String, String> entry : expected.entrySet()) {
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/builders/OSGiManifestBuilderFactory.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/builders/OSGiManifestBuilderFactory.java
index 6748504..c4d9826 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/builders/OSGiManifestBuilderFactory.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/builders/OSGiManifestBuilderFactory.java
@@ -97,7 +97,7 @@
int manifestVersion = getManifestVersion(manifest);
if (manifestVersion >= 2) {
- validateHeaders(manifest);
+ validateHeaders(manifest, extraExports != null);
}
Object symbolicName = getSymbolicNameAndVersion(builder, manifest, symbolicNameAlias, manifestVersion);
@@ -130,20 +130,20 @@
return builder;
}
- private static void validateHeaders(Map<String, String> manifest) throws BundleException {
+ private static void validateHeaders(Map<String, String> manifest, boolean allowJavaExports) throws BundleException {
for (String definedOSGiValidateHeader : DEFINED_OSGI_VALIDATE_HEADERS) {
String header = manifest.get(definedOSGiValidateHeader);
if (header != null) {
ManifestElement[] elements = ManifestElement.parseHeader(definedOSGiValidateHeader, header);
checkForDuplicateDirectivesAttributes(definedOSGiValidateHeader, elements);
if (definedOSGiValidateHeader == Constants.IMPORT_PACKAGE) {
- checkImportExportSyntax(definedOSGiValidateHeader, elements, false, false);
+ checkImportExportSyntax(definedOSGiValidateHeader, elements, false, false, false);
}
if (definedOSGiValidateHeader == Constants.DYNAMICIMPORT_PACKAGE) {
- checkImportExportSyntax(definedOSGiValidateHeader, elements, false, true);
+ checkImportExportSyntax(definedOSGiValidateHeader, elements, false, true, false);
}
if (definedOSGiValidateHeader == Constants.EXPORT_PACKAGE) {
- checkImportExportSyntax(definedOSGiValidateHeader, elements, true, false);
+ checkImportExportSyntax(definedOSGiValidateHeader, elements, true, false, allowJavaExports);
}
if (definedOSGiValidateHeader == Constants.FRAGMENT_HOST) {
checkExtensionBundle(definedOSGiValidateHeader, elements, manifest);
@@ -155,7 +155,8 @@
}
@SuppressWarnings("deprecation")
- private static void checkImportExportSyntax(String headerKey, ManifestElement[] elements, boolean export, boolean dynamic) throws BundleException {
+ private static void checkImportExportSyntax(String headerKey, ManifestElement[] elements, boolean export,
+ boolean dynamic, boolean allowJavaExports) throws BundleException {
if (elements == null)
return;
int length = elements.length;
@@ -169,7 +170,7 @@
throw new BundleException(message + " : " + NLS.bind(Msg.HEADER_PACKAGE_DUPLICATES, packageName), BundleException.MANIFEST_ERROR); //$NON-NLS-1$
}
// check for java.*
- if (export && packageName.startsWith("java.")) { //$NON-NLS-1$
+ if (export && !allowJavaExports && packageName.startsWith("java.")) { //$NON-NLS-1$
String message = NLS.bind(Msg.MANIFEST_INVALID_HEADER_EXCEPTION, headerKey, elements[i].toString());
throw new BundleException(message + " : " + NLS.bind(Msg.HEADER_PACKAGE_JAVA, packageName), BundleException.MANIFEST_ERROR); //$NON-NLS-1$
}
@@ -182,7 +183,7 @@
if (specVersion != null && !specVersion.equals(version))
throw new BundleException(NLS.bind(Msg.HEADER_VERSION_ERROR, Constants.VERSION_ATTRIBUTE, Constants.PACKAGE_SPECIFICATION_VERSION), BundleException.MANIFEST_ERROR);
}
- // check for bundle-symbolic-name and bundle-verion attibures
+ // check for bundle-symbolic-name and bundle-version attributes on exports
// (failure)
if (export) {
if (elements[i].getAttribute(Constants.BUNDLE_SYMBOLICNAME_ATTRIBUTE) != null) {
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/sources/PackageSource.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/sources/PackageSource.java
index 021a79e..358100f 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/sources/PackageSource.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/sources/PackageSource.java
@@ -59,7 +59,6 @@
public abstract Enumeration<URL> getResources(String name) throws IOException;
- //TODO See how this relates with FilteredSourcePackage. Overwriting or doing a double dispatch might be good.
// This is intentionally lenient; we don't force all suppliers to match (only one)
// it is better to get class cast exceptions in split package cases than miss an event
public boolean hasCommonSource(PackageSource other) {
@@ -134,7 +133,8 @@
return false;
}
// 3) for the specified bundle, find the wiring for the package. If no wiring is found return true
- PackageSource consumerSource = getSourceFromLoader(consumerBL, pkgName, className, checkInternal);
+ PackageSource consumerSource = getSourceFromLoader(consumerBL, pkgName, className, checkInternal,
+ container.getPackageAdmin());
if (consumerSource == null) {
// confirmed no source for consumer
return true;
@@ -145,7 +145,8 @@
}
// 4) For the registrant bundle, find the wiring for the package.
- PackageSource producerSource = getSourceFromLoader(producerBL, pkgName, className, checkInternal);
+ PackageSource producerSource = getSourceFromLoader(producerBL, pkgName, className, checkInternal,
+ container.getPackageAdmin());
if (producerSource == null) {
// confirmed no local class either; now check service object
if (serviceClass != null && ServiceFactory.class.isAssignableFrom(serviceClass)) {
@@ -172,15 +173,28 @@
}
private static PackageSource getSourceFromLoader(BundleLoader loader, String pkgName, String className,
- boolean checkInternal) {
+ boolean checkInternal, @SuppressWarnings("deprecation") PackageAdmin packageAdmin) {
PackageSource source = loader.getPackageSource(pkgName);
if (source != null || !checkInternal) {
return source;
}
try {
- if (loader.findLocalClass(className) != null) {
- // create a source that represents the private package
- return (new SingleSourcePackage(pkgName, loader));
+ Class<?> clazz = loader.findLocalClass(className);
+ if (clazz != null) {
+ // make sure it is from this actual loader
+ @SuppressWarnings("deprecation")
+ Bundle b = packageAdmin.getBundle(clazz);
+ if (b != null) {
+ if (loader.getWiring().getBundle() == b) {
+ // create a source that represents the private package
+ return (new SingleSourcePackage(pkgName, loader));
+ }
+ // it is from a different loader (probably something with connect)
+ BundleLoader classBundleLoader = getBundleLoader(b);
+ if (classBundleLoader != null) {
+ return (new SingleSourcePackage(pkgName, classBundleLoader));
+ }
+ }
}
} catch (ClassNotFoundException e) {
// ignore
@@ -203,7 +217,7 @@
if (producerBL == null) {
return null;
}
- PackageSource producerSource = getSourceFromLoader(producerBL, pkgName, className, checkInternal);
+ PackageSource producerSource = getSourceFromLoader(producerBL, pkgName, className, checkInternal, packageAdmin);
if (producerSource != null) {
return producerSource;
}
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/implied.permissions b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/implied.permissions
index 9273890..57c82c1 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/implied.permissions
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/implied.permissions
@@ -47,7 +47,7 @@
(java.io.FilePermission "" "read")
(java.io.FilePermission "-" "read,write,delete")
-# Added for OSGi SP R3 - this likely is not needed any more
+# All bundles are allowed to import any java.* package
(org.osgi.framework.PackagePermission "java.*" "import")
# Added for OSGi R4.3
(org.osgi.framework.CapabilityPermission "osgi.ee" "require")
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/Storage.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/Storage.java
index 22a91e2..3ee8847 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/Storage.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/Storage.java
@@ -822,7 +822,9 @@
}
}
if (generation.getBundleInfo().getBundleId() != 0) {
- ModuleRevisionBuilder builder = allowRestrictedProvides ? OSGiManifestBuilderFactory.createBuilder(mapHeaders, null, null, "") : OSGiManifestBuilderFactory.createBuilder(mapHeaders); //$NON-NLS-1$
+ ModuleRevisionBuilder builder = OSGiManifestBuilderFactory.createBuilder(mapHeaders, null, //
+ (generation.getContentType() == Type.CONNECT ? "" : null), //$NON-NLS-1$
+ (allowRestrictedProvides ? "" : null)); //$NON-NLS-1$
if ((builder.getTypes() & BundleRevision.TYPE_FRAGMENT) != 0) {
for (ModuleRevisionBuilder.GenericInfo reqInfo : builder.getRequirements()) {
if (HostNamespace.HOST_NAMESPACE.equals(reqInfo.getNamespace())) {
