Fix for 83556

The MBS now validates the configuration names more carefully. It checks for duplicates in a case-insenstive way. It also looks for invalid whitespaces at the start of a config name. It also disallows names containing \/:*?"<>

Also corrected a file that had a local variable called "enum" which is now a "sort-of" reserved keyword in Java 1.5

build/org.eclipse.cdt.managedbuilder.ui/src/org/eclipse/cdt/managedbuilder/internal/ui/PluginResources.properties

@@ -101,8 +101,9 @@ NewConfiguration.label.name=Name:
 NewConfiguration.label.group=Copy settings from
 NewConfiguration.label.copy=Default configuration:
 NewConfiguration.label.clone=Existing configuration:
-NewConfiguration.error.title=Error
 NewConfiguration.error.duplicateName=A configuration named "{0}" already exists.
+NewConfiguration.error.caseName=A configuration name that differs only in case to "{0}" exists.
+NewConfiguration.error.invalidName=The name "{0}" is invalid.
 
 # ----------- Target/Config management dialog -----------------
 ManageConfig.label.configs=Manage configurations

build/org.eclipse.cdt.managedbuilder.ui/src/org/eclipse/cdt/managedbuilder/ui/properties/BuildToolSettingsPage.java

@@ -220,9 +220,9 @@ public class BuildToolSettingsPage extends BuildSettingsPage {
 							option.getId());
 					if (enumCommand.indexOf(DEFAULT_SEPERATOR) != -1)
 						enumCommand = option.getSelectedEnum();
-					String enum = option.getEnumCommand(enumCommand);
+					String enumeration = option.getEnumCommand(enumCommand);
-					if (enum.length() > 0) {
+					if (enumeration.length() > 0) {
-						buf.append(enum);
+						buf.append(enumeration);
 					}
 					break;
 				case IOption.STRING :
@@ -339,18 +339,18 @@ public class BuildToolSettingsPage extends BuildSettingsPage {
 								}
 								break;
 							case IOption.ENUMERATED :
-								String enum = ""; //$NON-NLS-1$
+								String enumeration = ""; //$NON-NLS-1$
 								String[] enumValues = opt.getApplicableValues();
 								for (int i = 0; i < enumValues.length; i++) {
 									if (opt.getEnumCommand(enumValues[i]).equals(
 											optionValue)) {
-										enum = enumValues[i];
+										enumeration = enumValues[i];
 										optionValueExist = true;
 									}
 								}
-								if (!enum.equals("")) //$NON-NLS-1$
+								if (!enumeration.equals("")) //$NON-NLS-1$
 									getToolSettingsPreferenceStore()
-											.setValue(opt.getId(), enum);
+											.setValue(opt.getId(), enumeration);
 								break;
 							case IOption.STRING_LIST :
 							case IOption.INCLUDE_PATH :

build/org.eclipse.cdt.managedbuilder.ui/src/org/eclipse/cdt/managedbuilder/ui/properties/ManageConfigDialog.java

@@ -57,8 +57,10 @@ public class ManageConfigDialog extends Dialog {
 	private SortedMap existingConfigs;
 	// The target the configs belong to
 	private IManagedProject managedProject;
-	// Map of new configurations chosen by the user
+	/** All new configs added by the user but not yet part of target */
-	private SortedMap newConfigs;
+	private SortedMap newAddedConfigs;
+	/** All new configs removed by the user but not yet part of target */
+	private SortedMap removedNewConfigs;
 	// The title of the dialog.
 	private String title = ""; //$NON-NLS-1$
 	
@@ -269,12 +271,33 @@ public class ManageConfigDialog extends Dialog {
 	 * @return Map   
 	 */
 	public SortedMap getNewConfigs() {
-		if (newConfigs == null) {
+		if (newAddedConfigs == null) {
-			newConfigs = new TreeMap();
+			newAddedConfigs = new TreeMap();
 		}
-		return newConfigs;
+		return newAddedConfigs;
 	}
 
+	// Answers a list of new configuration names that have been added-- 
+	// or added and removed--by the user, but that have not yet been added 
+	// to the target
+	private ArrayList getNewConfigNames() {
+		ArrayList names = new ArrayList();
+		names.addAll(getNewConfigs().keySet());
+		names.addAll(getRemovedNewConfigs().keySet());
+		return names;
+	}
+	
+	
+	// This data structure hangs on to a new configuration that is added
+	// by the user, then removed before it is added to the target. This is 
+	// a required bookeeping step because the user may change their minds and 
+	// restore the deleted configuration. 
+	private SortedMap getRemovedNewConfigs() {
+		if (removedNewConfigs == null) {
+			removedNewConfigs = new TreeMap();
+		}
+		return removedNewConfigs;
+	}
 	/*
 	 * @return the <code>IProject</code> associated with the managed project
 	 */
@@ -286,8 +309,10 @@ public class ManageConfigDialog extends Dialog {
 	 * Event handler for the add button
 	 */
 	protected void handleNewPressed() {
+		// Pop-up a dialog to properly handle the request
 		NewConfigurationDialog dialog = new NewConfigurationDialog(getShell(), 
 																   managedProject, 
+																   getNewConfigNames(),
 																   ManagedBuilderUIMessages.getResourceString(CONF_DLG));
 		if (dialog.open() == NewConfigurationDialog.OK) {
 			// Get the new name and configuration to base the new config on
@@ -309,18 +334,18 @@ public class ManageConfigDialog extends Dialog {
 		int selectionIndex = currentConfigList.getSelectionIndex();
 		if (selectionIndex != -1){
 			String selectedConfigName = currentConfigList.getItem(selectionIndex);
-			String selectedConfigId = null;
 			
-			// If this is a newly added config, remove it from that map
+			// If this is a newly added config, remove it from the new map 
+			// and add it to a special map to support the restore use case
 			if (getNewConfigs().containsKey(selectedConfigName)) {
 				IConfiguration selectedConfig = (IConfiguration) getNewConfigs().get(selectedConfigName); 
-				selectedConfigId = selectedConfig.getId();
+				getRemovedNewConfigs().put(selectedConfigName, selectedConfig);
 				getNewConfigs().remove(selectedConfigName);
 			} else {
 				// If it is not a new item, the ID is in the existing list
-				selectedConfigId = (String) getExistingConfigs().get(selectedConfigName);
+				String selectedConfigId = (String) getExistingConfigs().get(selectedConfigName);
+				getDeletedConfigs().put(selectedConfigName, selectedConfigId);
 			}
-			getDeletedConfigs().put(selectedConfigName, selectedConfigId);
 
 			// Clean up the UI lists
 			currentConfigList.remove(selectionIndex);
@@ -337,21 +362,20 @@ public class ManageConfigDialog extends Dialog {
 	protected void handleRestorePressed() {
 		// Determine which configuration was selected
 		int selectionIndex = deletedConfigList.getSelectionIndex();
-		// Move the selected element from the deleted list to the current list
+		// Move the selected element from the correct deleted list to the current list
 		if (selectionIndex != -1){
 			// Get the name of the item to delete
 			String selectedConfigName = deletedConfigList.getItem(selectionIndex);
-			String selectedConfigId = (String) getDeletedConfigs().get(selectedConfigName);
 			
-			// If this was a new config (it won't be in the existing list) then add it back there
+			// The deleted config may be one of the existing configs or one of the
-			if (!getExistingConfigs().containsKey(selectedConfigName)) {
+			// new configs that have not been added to the target yet
-				IConfiguration restoredConfig = managedProject.getConfiguration(selectedConfigId);
+			if (getRemovedNewConfigs().containsKey(selectedConfigName)) {
+				IConfiguration restoredConfig = managedProject.getConfiguration(selectedConfigName);
 				getNewConfigs().put(selectedConfigName, restoredConfig);
+				getRemovedNewConfigs().remove(selectedConfigName);
+			} else {
+				getDeletedConfigs().remove(selectedConfigName);
 			}
-			
-			// Remove it from the deleted map
-			getDeletedConfigs().remove(selectedConfigName);
-
 			// Clean up the UI
 			deletedConfigList.remove(selectionIndex);
 			deletedConfigList.setSelection(selectionIndex - 1);

build/org.eclipse.cdt.managedbuilder.ui/src/org/eclipse/cdt/managedbuilder/ui/properties/NewConfigurationDialog.java

@@ -12,11 +12,13 @@ package org.eclipse.cdt.managedbuilder.ui.properties;
 
 import org.eclipse.cdt.managedbuilder.core.IProjectType;
 import org.eclipse.cdt.managedbuilder.core.IManagedProject;
+import java.util.ArrayList;
+import java.util.Iterator;
+import org.eclipse.cdt.internal.ui.dialogs.StatusDialog;
+import org.eclipse.cdt.internal.ui.dialogs.StatusInfo;
 import org.eclipse.cdt.managedbuilder.core.IConfiguration;
 import org.eclipse.cdt.managedbuilder.internal.ui.ManagedBuilderUIMessages;
-import org.eclipse.jface.dialogs.Dialog;
 import org.eclipse.jface.dialogs.IDialogConstants;
-import org.eclipse.jface.dialogs.MessageDialog;
 import org.eclipse.swt.SWT;
 import org.eclipse.swt.events.ModifyEvent;
 import org.eclipse.swt.events.ModifyListener;
@@ -34,7 +36,7 @@ import org.eclipse.swt.widgets.Label;
 import org.eclipse.swt.widgets.Shell;
 import org.eclipse.swt.widgets.Text;
 
-public class NewConfigurationDialog extends Dialog {
+public class NewConfigurationDialog extends StatusDialog {
 	// String constants
 	private static final String PREFIX = "NewConfiguration";	//$NON-NLS-1$
 	private static final String LABEL = PREFIX + ".label";	//$NON-NLS-1$
@@ -43,37 +45,45 @@ public class NewConfigurationDialog extends Dialog {
 	private static final String GROUP = LABEL + ".group";	//$NON-NLS-1$
 	private static final String COPY = LABEL + ".copy";	//$NON-NLS-1$
 	private static final String CLONE = LABEL + ".clone";	//$NON-NLS-1$
-	private static final String TITLE = ERROR + ".title";	//$NON-NLS-1$	
 	private static final String DUPLICATE = ERROR + ".duplicateName";	//$NON-NLS-1$	
+	private static final String CASE = ERROR + ".caseName";	//$NON-NLS-1$	
+	private static final String INVALID = ERROR + ".invalidName";	//$NON-NLS-1$	
 
 	// Widgets
 	private Button btnClone;
 	private Button btnCopy;
-	private Button btnOk;
 	private Text configName;
 	private Combo copyConfigSelector;
 	private Combo cloneConfigSelector;
 		
 	// Bookeeping
 	private boolean clone;
+	/** Default configurations defined in the toolchain description */
 	private IConfiguration[] defaultConfigs;
+	/** Configurations defined in the target */
 	private IConfiguration[] definedConfigs;
 	private IConfiguration parentConfig;
 	private IManagedProject managedProject;
 	private String newName;
-	private String title = ""; //$NON-NLS-1$
+	/** A list containing config names that have been defined but not added to the target */
+	final private ArrayList reservedNames;
+	final private String title;
 
 	
 	/**
 	 * @param parentShell
+	 * @param managedTarget
+	 * @param nameList A list of names that have been added by the user but have not yet been added to the target 
+	 * @param title The title of the dialog
 	 */
-	protected NewConfigurationDialog(Shell parentShell, IManagedProject managedProject, String title) {
+	protected NewConfigurationDialog(Shell parentShell, IManagedProject managedProject, ArrayList nameList, String title) {
 		super(parentShell);
 		this.title = title;
 		setShellStyle(getShellStyle()|SWT.RESIZE);
 		newName = new String();
 		parentConfig = null;
 		this.managedProject = managedProject;
+		reservedNames = nameList;
 		
 		// The default behaviour is to clone the settings
 		clone = true;
@@ -133,13 +143,12 @@ public class NewConfigurationDialog extends Dialog {
 	 * @see org.eclipse.jface.dialogs.Dialog#createButtonsForButtonBar(org.eclipse.swt.widgets.Composite)
 	 */
 	protected void createButtonsForButtonBar(Composite parent) {
-		btnOk = createButton(parent, IDialogConstants.OK_ID, IDialogConstants.OK_LABEL, true);
+		super.createButtonsForButtonBar(parent);
-		createButton(parent, IDialogConstants.CANCEL_ID, IDialogConstants.CANCEL_LABEL, false);
 		configName.setFocus();
 		if (configName != null) {
 			configName.setText(newName);
 		}
-		updateButtonState();
+		validateState();
 	}
 
 	protected Control createDialogArea(Composite parent) {
@@ -163,7 +172,7 @@ public class NewConfigurationDialog extends Dialog {
 		configName.setLayoutData(gd);
 		configName.addModifyListener(new ModifyListener() {
 			public void modifyText(ModifyEvent e) {
-				updateButtonState();
+				validateState();
 			}
 		});
 		
@@ -201,7 +210,7 @@ public class NewConfigurationDialog extends Dialog {
 		copyConfigSelector.setLayoutData(gd);
 		copyConfigSelector.addSelectionListener(new SelectionAdapter() {
 			public void widgetSelected(SelectionEvent e) {
-				updateButtonState();		
+				validateState();		
 			}
 		});	
 		copyConfigSelector.setEnabled(false);
@@ -225,7 +234,7 @@ public class NewConfigurationDialog extends Dialog {
 		cloneConfigSelector.setLayoutData(gd);
 		cloneConfigSelector.addSelectionListener(new SelectionAdapter() {
 			public void widgetSelected(SelectionEvent e) {
-				updateButtonState();		
+				validateState();		
 			}
 		});	
 
@@ -286,25 +295,40 @@ public class NewConfigurationDialog extends Dialog {
 	 */
 	protected boolean isDuplicateName(String newName) {
 		// Return true if there is already a config of that name defined
-		IConfiguration [] configs = managedProject.getConfigurations();
+		for (int index = 0; index < definedConfigs.length; index++) {
-		for (int index = 0; index < configs.length; index++) {
+			IConfiguration configuration = definedConfigs[index];
-			IConfiguration configuration = configs[index];
 			if (configuration.getName().equals(newName)) {
 				return true;
 			}
 		}
+		if (reservedNames.contains(newName)) {
+			return true;
+		}
 		return false;
 	}
 	
 	/* (non-Javadoc)
-	 * Enable the OK button if there is a valid name in the text widget
+	 * Answers <code>true</code> if the name entered by the user differs 
-	 * and there is a valid selection in the base configuration combo 
+	 * only in case from an existing name.
+	 * 
+	 * @param newName
+	 * @return
 	 */
-	private void updateButtonState() {
+	protected boolean isSimilarName(String newName) {
-		if (btnOk != null) {
+		// Return true if there is already a config of that name defined on the target
-			int selectionIndex = copyConfigSelector.getSelectionIndex();			
+		for (int index = 0; index < definedConfigs.length; index++) {
-			btnOk.setEnabled(validateName() && selectionIndex != -1);
+			IConfiguration configuration = definedConfigs[index];
+			if (configuration.getName().equalsIgnoreCase(newName)) {
+				return true;
+			}
 		}
+		Iterator iter = reservedNames.listIterator();
+		while (iter.hasNext()) {
+			if (((IConfiguration)iter.next()).getName().equalsIgnoreCase(newName)) {
+				return true;
+			}
+		}
+		return false;
 	}
 
 	/* (non-Javadoc)
@@ -316,17 +340,61 @@ public class NewConfigurationDialog extends Dialog {
 		copyConfigSelector.setEnabled(!clone);
 	}
 
-	private boolean validateName() {
+	/* (non-Javadoc)
-		String currentName = configName.getText().trim(); 
+	 * Checks the argument for leading whitespaces and invalid directory name characters. 
-		int nameLength = currentName.length();
+	 * @param name
-		// Make sure the name is not a duplicate
+	 * @return <I>true</i> is the name is a valid directory name with no whitespaces
-		if (isDuplicateName(currentName)) {
+	 */
-			MessageDialog.openError(getShell(), 
+	private boolean validateName(String name) {
-					ManagedBuilderUIMessages.getResourceString(TITLE), 
+		// Iterate over the name checking for bad characters
-					ManagedBuilderUIMessages.getFormattedString(DUPLICATE, currentName)); //$NON-NLS-1$
+		char[] chars = name.toCharArray();
+		// No whitespaces at the start of a name
+		if (Character.isWhitespace(chars[0])) {
 			return false;
 		}
-		// TODO make sure there are no invalid chars in name
+		for (int index = 0; index < chars.length; ++index) {
-		return (nameLength > 0);
+			// Config name must be a valid dir name too, so we ban "\ / : * ? " < >" in the names
+			if (!Character.isLetterOrDigit(chars[index])) {
+				switch (chars[index]) {
+				case '/':
+				case '\\':
+				case ':':
+				case '*':
+				case '?':
+				case '\"':
+				case '<':
+				case '>':
+					return false;
+				default:
+					break;
+				}
+			}
+		}
+		return true;
+	}
+	/* (non-Javadoc)
+	 * Update the status message and button state based on the input selected
+	 * by the user
+	 * 
+	 */
+	private void validateState() {
+		StatusInfo status= new StatusInfo();
+		String currentName = configName.getText(); 
+		int nameLength = currentName.length();
+		// Make sure the name is not a duplicate
+		if (nameLength == 0) {
+			// Not an error
+			status.setError("");	//$NON-NLS-1$
+		} else if (isDuplicateName(currentName)) {
+			status.setError(ManagedBuilderUIMessages.getFormattedString(DUPLICATE, currentName));
+		} else if (isSimilarName(currentName)) {
+			status.setError(ManagedBuilderUIMessages.getFormattedString(CASE, currentName));
+		} else if (!validateName(currentName)) {
+			// TODO Create a decent I18N string to describe this problem
+			status.setError(ManagedBuilderUIMessages.getFormattedString(INVALID, currentName));	//$NON-NLS-1$
+		} 
+		
+		updateStatus(status);
+		return;
 	}
 }