Bug 514333 - Preferences store access can lead to unattended workspaceI20170918-2000

location initialization

DataArea checks now if "osgi.dataAreaRequiresExplicitInit" system
property is set to "true". In that case it only allows to initialize
location if the location provides a NON-default value (service.isSet()).

If the instance location is not set explicitly and the system property
is enabled, DataArea will throw IllegalStateException on any attempt to
initialize it. If the PreferencesService is the one who triggers the
unattended init of the DataArea, it will also propagate the
IllegalStateException now.

The default behavior if the system property is not set remains
unchanged.

Change-Id: I9cb9a24c876225148620488d9dfaa6579465e982
Signed-off-by: Andrey Loskutov <loskutov@gmx.de>

2 files changed, 28 insertions, 2 deletions

diff --git a/bundles/org.eclipse.equinox.common/src/org/eclipse/core/internal/runtime/DataArea.java b/bundles/org.eclipse.equinox.common/src/org/eclipse/core/internal/runtime/DataArea.java
index 3db52a1bc..fce5b99ed 100644
--- a/bundles/org.eclipse.equinox.common/src/org/eclipse/core/internal/runtime/DataArea.java
+++ b/bundles/org.eclipse.equinox.common/src/org/eclipse/core/internal/runtime/DataArea.java
@@ -25,6 +25,7 @@ import org.osgi.framework.Bundle;
  */
 public class DataArea {
 	private static final String OPTION_DEBUG = "org.eclipse.equinox.common/debug"; //$NON-NLS-1$;
+	private static final String REQUIRES_EXPLICIT_INIT = "osgi.dataAreaRequiresExplicitInit"; //$NON-NLS-1$;
 
 	/* package */static final String F_META_AREA = ".metadata"; //$NON-NLS-1$
 	/* package */static final String F_PLUGIN_DATA = ".plugins"; //$NON-NLS-1$
@@ -48,7 +49,15 @@ public class DataArea {
 		Location service = activator.getInstanceLocation();
 		if (service == null)
 			throw new IllegalStateException(CommonMessages.meta_noDataModeSpecified);
+
+		boolean explicitInitRequired = Boolean.valueOf(Activator.getContext().getProperty(REQUIRES_EXPLICIT_INIT));
+		if (explicitInitRequired && !service.isSet()) {
+			// See bug 514333: don't allow clients to initialize instance location if the instance area is not explicitly defined yet
+			throw new IllegalStateException(CommonMessages.meta_instanceDataUnspecified);
+		}
+
 		try {
+			// This will try to init url either from the specified location value or from service default
 			URL url = service.getURL();
 			if (url == null)
 				throw new IllegalStateException(CommonMessages.meta_instanceDataUnspecified);
diff --git a/bundles/org.eclipse.equinox.preferences/src/org/eclipse/core/internal/preferences/PreferencesService.java b/bundles/org.eclipse.equinox.preferences/src/org/eclipse/core/internal/preferences/PreferencesService.java
index e9559a12b..1569ffe47 100644
--- a/bundles/org.eclipse.equinox.preferences/src/org/eclipse/core/internal/preferences/PreferencesService.java
+++ b/bundles/org.eclipse.equinox.preferences/src/org/eclipse/core/internal/preferences/PreferencesService.java
@@ -16,6 +16,7 @@ import java.io.*;
 import java.lang.ref.WeakReference;
 import java.util.*;
 import java.util.Map.Entry;
+import java.util.concurrent.atomic.AtomicReference;
 import org.eclipse.core.internal.runtime.RuntimeLog;
 import org.eclipse.core.runtime.*;
 import org.eclipse.core.runtime.preferences.*;
@@ -595,12 +596,16 @@ public class PreferencesService implements IPreferencesService {
 		final ArrayList<Preferences> result = new ArrayList<>();
 		for (int i = 0; i < order.length; i++) {
 			final String scopeString = order[i];
+			AtomicReference<IllegalStateException> error = new AtomicReference<>();
 			SafeRunner.run(new ISafeRunnable() {
+
+				private IScopeContext context;
+
 				@Override
 				public void run() throws Exception {
 					boolean found = false;
 					for (int j = 0; contexts != null && j < contexts.length; j++) {
-						IScopeContext context = contexts[j];
+						context = contexts[j];
 						if (context != null && context.getName().equals(scopeString)) {
 							Preferences node = context.getNode(qualifier);
 							if (node != null) {
@@ -622,9 +627,21 @@ public class PreferencesService implements IPreferencesService {
 
 				@Override
 				public void handleException(Throwable exception) {
-					log(new Status(IStatus.ERROR, Activator.PI_PREFERENCES, PrefsMessages.preferences_contextError, exception));
+					// See bug 514333 and org.eclipse.core.internal.runtime.DataArea.assertLocationInitialized()
+					// If we see an IllegalStateException flying and the explicit init is required, we should not continue
+					if (context instanceof InstanceScope && exception instanceof IllegalStateException && Boolean.getBoolean("osgi.dataAreaRequiresExplicitInit")) { //$NON-NLS-1$
+						error.set((IllegalStateException) exception);
+					} else {
+						log(new Status(IStatus.ERROR, Activator.PI_PREFERENCES, PrefsMessages.preferences_contextError, exception));
+					}
 				}
 			});
+
+			IllegalStateException illegalState = error.get();
+			if (illegalState != null) {
+				// See bug 514333: don't allow clients to read (empty) instance prefs if the instance area is not initialized yet
+				throw illegalState;
+			}
 		}
 		return result.toArray(new Preferences[result.size()]);
 	}