Bug 490890 - Avoid holding system properties objectY20160407-1000I20160405-0800

Change-Id: I9ad88f33b03e3130ea73e990e107025f53b1abe0
Signed-off-by: Thomas Watson <tjwatson@us.ibm.com>

3 files changed, 135 insertions, 24 deletions

diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/SystemBundleTests.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/SystemBundleTests.java
index 5c64f8fbe..c1127c9a7 100755
--- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/SystemBundleTests.java
+++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/SystemBundleTests.java
@@ -2324,6 +2324,78 @@ public class SystemBundleTests extends AbstractBundleTests {
 		equinox.stop();
 	}
 
+	public void testBackedBySystemReplaceSystemProperties() throws BundleException {
+		File config = OSGiTestsActivator.getContext().getDataFile(getName()); //$NON-NLS-1$
+		Map<String, Object> configuration = new HashMap<String, Object>();
+		configuration.put(Constants.FRAMEWORK_STORAGE, config.getAbsolutePath());
+		configuration.put("osgi.framework.useSystemProperties", "true");
+		Equinox equinox = new Equinox(configuration);
+		equinox.start();
+		ServiceReference<EnvironmentInfo> envRef = equinox.getBundleContext().getServiceReference(EnvironmentInfo.class);
+		EnvironmentInfo envInfo = equinox.getBundleContext().getService(envRef);
+
+		// replace the system properties with a copy
+		Properties copy = new Properties();
+		copy.putAll(System.getProperties());
+		System.setProperties(copy);
+
+		// set a system prop for this test after replacement of the properties object
+		String systemKey = getName() + ".system";
+		System.setProperty(systemKey, getName());
+
+		// make sure context properties reflect the new test prop
+		assertEquals("Wrong context value", getName(), equinox.getBundleContext().getProperty(systemKey));
+
+		// also test EnvironmentInfo properties
+		assertEquals("Wrong context value", getName(), envInfo.getProperty(systemKey));
+		assertEquals("Wrong EquinoxConfiguration config value", getName(), ((EquinoxConfiguration) envInfo).getConfiguration(systemKey));
+
+		// set environment info prop
+		String envKey = getName() + ".env";
+		envInfo.setProperty(envKey, getName());
+
+		// make sure the system properties reflect the new test prop
+		assertEquals("Wrong system value", getName(), System.getProperty(envKey));
+		equinox.stop();
+	}
+
+	public void testLocalConfigReplaceSystemProperties() throws BundleException {
+		File config = OSGiTestsActivator.getContext().getDataFile(getName()); //$NON-NLS-1$
+		Map<String, Object> configuration = new HashMap<String, Object>();
+		configuration.put(Constants.FRAMEWORK_STORAGE, config.getAbsolutePath());
+		Equinox equinox = new Equinox(configuration);
+		equinox.start();
+		ServiceReference<EnvironmentInfo> envRef = equinox.getBundleContext().getServiceReference(EnvironmentInfo.class);
+		EnvironmentInfo envInfo = equinox.getBundleContext().getService(envRef);
+
+		// replace the system properties with a copy
+		Properties copy = new Properties();
+		copy.putAll(System.getProperties());
+		System.setProperties(copy);
+
+		// set a system prop for this test after replacement of the properties object
+		String systemKey = getName() + ".system";
+		System.setProperty(systemKey, getName());
+
+		// make sure context properties reflect the new system test prop.
+		// remember context properties are backed by system properties
+		assertEquals("Wrong context value", getName(), equinox.getBundleContext().getProperty(systemKey));
+
+		// also test EnvironmentInfo properties.
+		// remember the getProperty method is backed by system properties
+		assertEquals("Wrong context value", getName(), envInfo.getProperty(systemKey));
+		// config options are not backed by system properties when framework is not using system properties for configuration
+		assertNull("Wrong EquinoxConfiguration config value", ((EquinoxConfiguration) envInfo).getConfiguration(systemKey));
+
+		// set environment info prop
+		String envKey = getName() + ".env";
+		envInfo.setProperty(envKey, getName());
+
+		// make sure the system properties does NOT reflect the new test prop
+		assertNull("Wrong system value", System.getProperty(envKey));
+		equinox.stop();
+	}
+
 	public void testSystemNLFragment() throws BundleException {
 		File config = OSGiTestsActivator.getContext().getDataFile(getName()); //$NON-NLS-1$
 		Map<String, Object> configuration = new HashMap<String, Object>();
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/framework/util/SecureAction.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/framework/util/SecureAction.java
index 6b51ba69a..081d81b50 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/framework/util/SecureAction.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/framework/util/SecureAction.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2003, 2013 IBM Corporation and others.
+ * Copyright (c) 2003, 2016 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
@@ -14,6 +14,7 @@ package org.eclipse.osgi.framework.util;
 import java.io.*;
 import java.net.*;
 import java.security.*;
+import java.util.Properties;
 import java.util.zip.ZipException;
 import java.util.zip.ZipFile;
 import org.eclipse.osgi.container.Module;
@@ -78,6 +79,21 @@ public class SecureAction {
 	}
 
 	/**
+	 * Returns a system properties.  Same as calling
+	 * System.getProperties().
+	 * @return the system properties.
+	 */
+	public Properties getProperties() {
+		if (System.getSecurityManager() == null)
+			return System.getProperties();
+		return AccessController.doPrivileged(new PrivilegedAction<Properties>() {
+			public Properties run() {
+				return System.getProperties();
+			}
+		}, controlContext);
+	}
+
+	/**
 	 * Creates a FileInputStream from a File.  Same as calling
 	 * new FileInputStream(File).
 	 * @param file the File to craete a FileInputStream from.
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/EquinoxConfiguration.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/EquinoxConfiguration.java
index 38c1f4e82..e2899c4f6 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/EquinoxConfiguration.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/EquinoxConfiguration.java
@@ -211,7 +211,7 @@ public class EquinoxConfiguration implements EnvironmentInfo {
 
 	public static final class ConfigValues {
 		/**
-		 * Value of {@link #configuration} properties that should be considered
+		 * Value of {@link #localConfig} properties that should be considered
 		 * <code>null</code> and for which access should not fall back to
 		 * {@link System#getProperty(String)}.
 		 * The instance must be compared by identity (==, not equals) and must not
@@ -222,25 +222,26 @@ public class EquinoxConfiguration implements EnvironmentInfo {
 
 		private final boolean useSystemProperties;
 		private final Map<String, Object> initialConfig;
-		private final Properties configuration;
+		private final Properties localConfig;
 
 		public ConfigValues(Map<String, ?> initialConfiguration) {
 			this.initialConfig = initialConfiguration == null ? new HashMap<String, Object>(0) : new HashMap<String, Object>(initialConfiguration);
 			Object useSystemPropsValue = initialConfig.get(PROP_USE_SYSTEM_PROPERTIES);
 			this.useSystemProperties = useSystemPropsValue == null ? false : Boolean.parseBoolean(useSystemPropsValue.toString());
-			this.configuration = useSystemProperties ? System.getProperties() : new Properties();
+			Properties tempConfiguration = useSystemProperties ? EquinoxContainer.secureAction.getProperties() : new Properties();
 			// do this the hard way to handle null values
 			for (Map.Entry<String, ?> initialEntry : this.initialConfig.entrySet()) {
 				if (initialEntry.getValue() == null) {
 					if (useSystemProperties) {
-						this.configuration.remove(initialEntry.getKey());
+						tempConfiguration.remove(initialEntry.getKey());
 					} else {
-						this.configuration.put(initialEntry.getKey(), NULL_CONFIG);
+						tempConfiguration.put(initialEntry.getKey(), NULL_CONFIG);
 					}
 				} else {
-					this.configuration.put(initialEntry.getKey(), initialEntry.getValue());
+					tempConfiguration.put(initialEntry.getKey(), initialEntry.getValue());
 				}
 			}
+			localConfig = useSystemProperties ? null : tempConfiguration;
 		}
 
 		void loadConfigIni(URL configIni) {
@@ -404,11 +405,11 @@ public class EquinoxConfiguration implements EnvironmentInfo {
 
 		public String getProperty(String key) {
 			// have to access configuration directly instead of using getConfiguration to get access to NULL_CONFIG values
-			String result = configuration.getProperty(key);
+			String result = internalGet(key);
 			if (result == NULL_CONFIG) {
 				return null;
 			}
-			return result == null ? System.getProperty(key) : result;
+			return result == null ? EquinoxContainer.secureAction.getProperty(key) : result;
 		}
 
 		public String setProperty(String key, String value) {
@@ -419,36 +420,58 @@ public class EquinoxConfiguration implements EnvironmentInfo {
 		}
 
 		public String getConfiguration(String key) {
-			String result = configuration.getProperty(key);
+			String result = internalGet(key);
 			return result == NULL_CONFIG ? null : result;
 		}
 
+		private String internalGet(String key) {
+			if (useSystemProperties) {
+				return EquinoxContainer.secureAction.getProperty(key);
+			}
+			return localConfig.getProperty(key);
+		}
+
 		public String getConfiguration(String key, String defaultValue) {
 			String result = getConfiguration(key);
 			return result == null ? defaultValue : result;
 		}
 
 		public String setConfiguration(String key, String value) {
-			Object result = configuration.put(key, value);
+			Object result = internalPut(key, value);
 			return result instanceof String && result != NULL_CONFIG ? (String) result : null;
 		}
 
 		public String clearConfiguration(String key) {
-			Object result = configuration.remove(key);
+			Object result = internalRemove(key);
 			if (!useSystemProperties) {
-				configuration.put(key, NULL_CONFIG);
+				internalPut(key, NULL_CONFIG);
 			}
 			return result instanceof String && result != NULL_CONFIG ? (String) result : null;
 		}
 
+		private Object internalPut(String key, String value) {
+			if (useSystemProperties) {
+				return EquinoxContainer.secureAction.getProperties().put(key, value);
+			}
+			return localConfig.put(key, value);
+		}
+
+		private Object internalRemove(String key) {
+			if (useSystemProperties) {
+				return EquinoxContainer.secureAction.getProperties().remove(key);
+			}
+			return localConfig.remove(key);
+		}
+
 		public Map<String, String> getConfiguration() {
-			// must sync on configuration to avoid concurrent modification exception
-			synchronized (configuration) {
-				Map<String, String> result = new HashMap<String, String>(configuration.size());
-				for (Object key : configuration.keySet()) {
+			Properties props = useSystemProperties ? EquinoxContainer.secureAction.getProperties() : localConfig;
+			// must sync on props to avoid concurrent modification exception
+			synchronized (props) {
+				Map<String, String> result = new HashMap<String, String>(props.size());
+				for (Object key : props.keySet()) {
 					if (key instanceof String) {
 						String skey = (String) key;
-						String sValue = configuration.getProperty(skey);
+						String sValue = props.getProperty(skey);
 						if (sValue != NULL_CONFIG) {
 							result.put(skey, sValue);
 						}
@@ -607,7 +630,7 @@ public class EquinoxConfiguration implements EnvironmentInfo {
 		// Some OSes have multiple library extensions
 		// We should provide defaults to the known ones
 		// For example Mac OS X uses dylib and jnilib (bug 380350)
-		String os = System.getProperty(EquinoxConfiguration.PROP_JVM_OS_NAME);
+		String os = EquinoxContainer.secureAction.getProperty(EquinoxConfiguration.PROP_JVM_OS_NAME);
 		return os == null || !os.startsWith("Mac OS") ? null : "dylib,jnilib"; //$NON-NLS-1$ //$NON-NLS-2$
 	}
 
@@ -885,7 +908,7 @@ public class EquinoxConfiguration implements EnvironmentInfo {
 		setConfiguration(FRAMEWORK_VENDOR, ECLIPSE_FRAMEWORK_VENDOR);
 		String value = getConfiguration(FRAMEWORK_PROCESSOR);
 		if (value == null) {
-			value = System.getProperty(PROP_JVM_OS_ARCH);
+			value = EquinoxContainer.secureAction.getProperty(PROP_JVM_OS_ARCH);
 			if (value != null) {
 				setConfiguration(FRAMEWORK_PROCESSOR, aliasMapper.getCanonicalProcessor(value));
 			}
@@ -893,7 +916,7 @@ public class EquinoxConfiguration implements EnvironmentInfo {
 
 		value = getConfiguration(FRAMEWORK_OS_NAME);
 		if (value == null) {
-			value = System.getProperty(PROP_JVM_OS_NAME);
+			value = EquinoxContainer.secureAction.getProperty(PROP_JVM_OS_NAME);
 			if (value != null) {
 				setConfiguration(FRAMEWORK_OS_NAME, aliasMapper.getCanonicalOSName(value));
 			}
@@ -901,7 +924,7 @@ public class EquinoxConfiguration implements EnvironmentInfo {
 
 		value = getConfiguration(FRAMEWORK_OS_VERSION);
 		if (value == null) {
-			value = System.getProperty(PROP_JVM_OS_VERSION);
+			value = EquinoxContainer.secureAction.getProperty(PROP_JVM_OS_VERSION);
 			if (value != null) {
 				// only use the value upto the first space
 				int space = value.indexOf(' ');
@@ -974,7 +997,7 @@ public class EquinoxConfiguration implements EnvironmentInfo {
 		// argument then use the default.
 		String osValue = getConfiguration(PROP_OSGI_OS);
 		if (osValue == null) {
-			osValue = guessOS(System.getProperty(PROP_JVM_OS_NAME));
+			osValue = guessOS(EquinoxContainer.secureAction.getProperty(PROP_JVM_OS_NAME));
 			setConfiguration(PROP_OSGI_OS, osValue);
 		}
 
@@ -990,7 +1013,7 @@ public class EquinoxConfiguration implements EnvironmentInfo {
 		// argument then use the default.
 		String archValue = getConfiguration(PROP_OSGI_ARCH);
 		if (archValue == null) {
-			String name = System.getProperty(PROP_JVM_OS_ARCH);
+			String name = EquinoxContainer.secureAction.getProperty(PROP_JVM_OS_ARCH);
 			// Map i386 architecture to x86
 			if (name.equalsIgnoreCase(INTERNAL_ARCH_I386))
 				archValue = Constants.ARCH_X86;