Bug 240885 Non-ASCII chars are not properly storedv20080722-0430

8 files changed, 74 insertions, 17 deletions

diff --git a/bundles/org.eclipse.equinox.security.tests/src/org/eclipse/equinox/internal/security/tests/storage/SecurePreferencesTest.java b/bundles/org.eclipse.equinox.security.tests/src/org/eclipse/equinox/internal/security/tests/storage/SecurePreferencesTest.java
index 48f969dcc..f6b945e3e 100644
--- a/bundles/org.eclipse.equinox.security.tests/src/org/eclipse/equinox/internal/security/tests/storage/SecurePreferencesTest.java
+++ b/bundles/org.eclipse.equinox.security.tests/src/org/eclipse/equinox/internal/security/tests/storage/SecurePreferencesTest.java
@@ -42,6 +42,9 @@ abstract public class SecurePreferencesTest extends StorageAbstractTest {
 	final private static String clearTextKey = "data";
 	final private static String clearTextValue = "-> this should not be encrypted <-";
 
+	final private static String unicodeKey = "unicodeKey";
+	final private static String unicodeValue = "va\u0432lue\u0433";
+
 	public SecurePreferencesTest() {
 		super();
 	}
@@ -71,6 +74,7 @@ abstract public class SecurePreferencesTest extends StorageAbstractTest {
 		node3.put(key, value, true);
 		node3.put(secondKey, secondValue, true);
 		node3.put(clearTextKey, clearTextValue, false);
+		node3.put(unicodeKey, unicodeValue, true);
 
 		node2.remove(secondKey);
 
@@ -109,11 +113,12 @@ abstract public class SecurePreferencesTest extends StorageAbstractTest {
 		assertEquals(secondValue, node3.get(secondKey, defaultValue));
 		assertEquals(defaultValue, node3.get(unassignedKey, defaultValue));
 		assertEquals(clearTextValue, node3.get(clearTextKey, defaultValue));
+		assertEquals(unicodeValue, node3.get(unicodeKey, defaultValue));
 
 		String[] leafKeys = node3.keys();
 		assertNotNull(leafKeys);
-		assertEquals(leafKeys.length, 3);
-		findAll(new String[] {clearTextKey, key, secondKey}, leafKeys);
+		assertEquals(leafKeys.length, 4);
+		findAll(new String[] {clearTextKey, key, secondKey, unicodeKey}, leafKeys);
 	}
 
 	/**
diff --git a/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/auth/nls/SecAuthMessages.java b/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/auth/nls/SecAuthMessages.java
index b4d92fc19..d978f1af4 100644
--- a/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/auth/nls/SecAuthMessages.java
+++ b/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/auth/nls/SecAuthMessages.java
@@ -28,6 +28,7 @@ public class SecAuthMessages extends NLS {
 	public static String loginFailure;
 	public static String startFirst;
 	public static String stopFirst;
+	public static String unsupoprtedCharEncoding;
 
 	// Configuration provider
 	public static String unexpectedConfigElement;
diff --git a/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/auth/nls/messages.properties b/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/auth/nls/messages.properties
index fe16feea2..f87774348 100644
--- a/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/auth/nls/messages.properties
+++ b/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/auth/nls/messages.properties
@@ -21,6 +21,7 @@ elementUnexpected = Specified IConfigurationElement is not of type \"{0}\".
 loginFailure = Failed to log in.
 startFirst = Secure platform is not running. Make sure that secure platform is started.
 stopFirst = Secure platform is already running. Make sure that secure platform is stopped first.
+unsupoprtedCharEncoding = Default secure storage encoding \"{0}\" is not supported by the Java virtual machine.
 
 ## Configuration provider
 invalidConfigURL = Invalid URL \"{0}\" specified by \"{1}\" for Configuration provider.
diff --git a/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/storage/JavaEncryption.java b/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/storage/JavaEncryption.java
index fa485f2a8..2eb60280b 100644
--- a/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/storage/JavaEncryption.java
+++ b/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/storage/JavaEncryption.java
@@ -295,9 +295,9 @@ public class JavaEncryption {
 		try {
 			cipherAlgorithm = testCipher;
 			keyFactoryAlgorithm = testKeyFactory;
-			CryptoData encrypted = internalEncrypt(samplePassword, sampleText.getBytes());
+			CryptoData encrypted = internalEncrypt(samplePassword, StorageUtils.getBytes(sampleText));
 			byte[] roundtripBytes = internalDecrypt(samplePassword, encrypted);
-			String result = new String(roundtripBytes);
+			String result = StorageUtils.getString(roundtripBytes);
 			return sampleText.equals(result);
 		} catch (Exception e) {
 			// internal implementation throws both checked and unchecked
diff --git a/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/storage/PasswordManagement.java b/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/storage/PasswordManagement.java
index c45ce1856..925ed8a46 100644
--- a/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/storage/PasswordManagement.java
+++ b/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/storage/PasswordManagement.java
@@ -77,7 +77,7 @@ public class PasswordManagement {
 			return;
 		}
 		try {
-			byte[] data = new String(password.getPassword().getPassword()).getBytes();
+			byte[] data = StorageUtils.getBytes(new String(password.getPassword().getPassword()));
 			CryptoData encryptedValue = root.getCipher().encrypt(internalPasswordExt, data);
 			node.internalPut(PASSWORD_RECOVERY_KEY, encryptedValue.toString());
 			root.markModified();
@@ -127,7 +127,7 @@ public class PasswordManagement {
 		try {
 			CryptoData encryptedData = new CryptoData(node.internalGet(PASSWORD_RECOVERY_KEY));
 			byte[] data = root.getCipher().decrypt(internalPasswordExt, encryptedData);
-			return new String(data);
+			return StorageUtils.getString(data);
 		} catch (IllegalStateException e) {
 			return null;
 		} catch (IllegalBlockSizeException e) {
@@ -173,7 +173,7 @@ public class PasswordManagement {
 		try {
 			// normally use digest of what was entered
 			MessageDigest digest = MessageDigest.getInstance(DIGEST_ALGORITHM);
-			byte[] digested = digest.digest(mix.toString().getBytes());
+			byte[] digested = digest.digest(StorageUtils.getBytes(mix.toString()));
 			internalPassword = EncodingUtils.encodeBase64(digested);
 		} catch (NoSuchAlgorithmException e) {
 			// just use the text as is; it is nicer to use digest but in this case no big deal
diff --git a/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/storage/SecurePreferences.java b/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/storage/SecurePreferences.java
index 8653826ad..73822c8ac 100644
--- a/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/storage/SecurePreferences.java
+++ b/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/storage/SecurePreferences.java
@@ -193,7 +193,7 @@ public class SecurePreferences {
 			for (Iterator i = values.keySet().iterator(); i.hasNext();) {
 				String key = (String) i.next();
 				PersistedPath extenalTag = new PersistedPath(thisNodePath, key);
-				properties.put(extenalTag.toString(), values.get(key));
+				properties.setProperty(extenalTag.toString(), (String) values.get(key));
 			}
 		}
 
@@ -215,8 +215,8 @@ public class SecurePreferences {
 		checkRemoved();
 
 		if (!encrypt || value == null) {
-			CryptoData clearValue = new CryptoData(null, null, value == null ? null : value.getBytes());
-			internalPut(key, clearValue.toString());
+			CryptoData clearValue = new CryptoData(null, null, StorageUtils.getBytes(value));
+			internalPut(key, clearValue.toString()); // uses Base64 to encode byte sequences
 			markModified();
 			return;
 		}
@@ -225,7 +225,7 @@ public class SecurePreferences {
 		if (passwordExt == null) {
 			boolean storeDecrypted = !CallbacksProvider.getDefault().runningUI() || InternalExchangeUtils.isJUnitApp();
 			if (storeDecrypted) { // for JUnits and headless runs we store value as clear text and log a error
-				CryptoData clearValue = new CryptoData(null, null, value == null ? null : value.getBytes());
+				CryptoData clearValue = new CryptoData(null, null, StorageUtils.getBytes(value));
 				internalPut(key, clearValue.toString());
 				markModified();
 				// Make this as visible as possible. Both print out the output and log a error
@@ -238,7 +238,7 @@ public class SecurePreferences {
 		}
 
 		// value must not be null at this point
-		CryptoData encryptedValue = getRoot().getCipher().encrypt(getRoot().getPassword(null, container, true), value.getBytes());
+		CryptoData encryptedValue = getRoot().getCipher().encrypt(getRoot().getPassword(null, container, true), StorageUtils.getBytes(value));
 		internalPut(key, encryptedValue.toString());
 		markModified();
 	}
@@ -256,7 +256,7 @@ public class SecurePreferences {
 		if (moduleID == null) { // clear-text value, not encrypted
 			if (data.getData() == null)
 				return null;
-			return new String(data.getData());
+			return StorageUtils.getString(data.getData());
 		}
 
 		PasswordExt passwordExt = getRoot().getPassword(moduleID, container, false);
@@ -265,7 +265,7 @@ public class SecurePreferences {
 
 		try {
 			byte[] clearText = getRoot().getCipher().decrypt(passwordExt, data);
-			return new String(clearText);
+			return StorageUtils.getString(clearText);
 		} catch (IllegalBlockSizeException e) { // invalid password?
 			throw new StorageException(StorageException.DECRYPTION_ERROR, e);
 		} catch (BadPaddingException e) { // invalid password?
diff --git a/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/storage/SecurePreferencesRoot.java b/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/storage/SecurePreferencesRoot.java
index 33f346983..c682219ff 100644
--- a/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/storage/SecurePreferencesRoot.java
+++ b/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/storage/SecurePreferencesRoot.java
@@ -257,7 +257,7 @@ public class SecurePreferencesRoot extends SecurePreferences implements IStorage
 				passwordExt = new PasswordExt(password, key);
 				if (newPassword) {
 					String test = createTestString();
-					CryptoData encryptedValue = getCipher().encrypt(passwordExt, test.getBytes());
+					CryptoData encryptedValue = getCipher().encrypt(passwordExt, StorageUtils.getBytes(test));
 					node.internalPut(key, encryptedValue.toString());
 					markModified();
 					setupPasswordRecovery = true;
@@ -269,7 +269,7 @@ public class SecurePreferencesRoot extends SecurePreferences implements IStorage
 				CryptoData data = new CryptoData(encryptedData);
 				try {
 					byte[] decryptedData = getCipher().decrypt(passwordExt, data);
-					String test = new String(decryptedData);
+					String test = StorageUtils.getString(decryptedData);
 					if (verifyTestString(test)) {
 						validPassword = true;
 						break;
@@ -347,7 +347,7 @@ public class SecurePreferencesRoot extends SecurePreferences implements IStorage
 		CryptoData encryptedValue;
 		try {
 			String test = createTestString();
-			encryptedValue = getCipher().encrypt(passwordExt, test.getBytes());
+			encryptedValue = getCipher().encrypt(passwordExt, StorageUtils.getBytes(test));
 		} catch (StorageException e) {
 			String msg = NLS.bind(SecAuthMessages.encryptingError, key, PASSWORD_VERIFICATION_NODE);
 			AuthPlugin.getDefault().logError(msg, e);
diff --git a/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/storage/StorageUtils.java b/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/storage/StorageUtils.java
index 4087e2f17..3a8e5144c 100644
--- a/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/storage/StorageUtils.java
+++ b/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/storage/StorageUtils.java
@@ -15,6 +15,7 @@ import java.net.URL;
 import java.net.URLConnection;
 import org.eclipse.equinox.internal.security.auth.AuthPlugin;
 import org.eclipse.equinox.internal.security.auth.nls.SecAuthMessages;
+import org.eclipse.osgi.util.NLS;
 
 /**
  * PLEASE READ BEFORE CHANGING THIS FILE
@@ -33,10 +34,17 @@ import org.eclipse.equinox.internal.security.auth.nls.SecAuthMessages;
 public class StorageUtils {
 
 	/**
+	 * Characters encoding used by the secure storage.
+	 */
+	final public static String CHAR_ENCODING = "UTF-8"; //$NON-NLS-1$
+
+	/**
 	 * Default name of the storage file
 	 */
 	final private static String propertiesFileName = ".eclipse/org.eclipse.equinox.security/secure_storage"; //$NON-NLS-1$
 
+	static private boolean firstCharsetException = true;
+
 	/**
 	 * Default locations:
 	 * 1) user.home
@@ -109,4 +117,46 @@ public class StorageUtils {
 		return ("file".equals(url.getProtocol())); //$NON-NLS-1$
 	}
 
+	/**
+	 * The {@link String#getBytes()} truncates non-ASCII chars. As a result 
+	 * new String(string.getBytes()) is not the same as the original string. Moreover,
+	 * the default Java encoding can be changed via system variables or startup conditions.
+	 */
+	static public byte[] getBytes(String string) {
+		if (string == null)
+			return null;
+		try {
+			return string.getBytes(CHAR_ENCODING);
+		} catch (UnsupportedEncodingException e) {
+			if (firstCharsetException) { // log error once per session
+				String msg = NLS.bind(SecAuthMessages.unsupoprtedCharEncoding, StorageUtils.CHAR_ENCODING);
+				AuthPlugin.getDefault().logMessage(msg);
+				firstCharsetException = false;
+			}
+			return string.getBytes();
+		}
+	}
+
+	/**
+	 * The new String(byte[]) method uses default system encoding which
+	 * might not properly process non-ASCII characters. 
+	 * 
+	 * Pairing {@link #getBytes(String)} and {@link #getString(byte[])} methods allows round trip 
+	 * of non-ASCII characters. 
+	 */
+	static public String getString(byte[] bytes) {
+		if (bytes == null)
+			return null;
+		try {
+			return new String(bytes, CHAR_ENCODING);
+		} catch (UnsupportedEncodingException e) {
+			if (firstCharsetException) { // log error once per session
+				String msg = NLS.bind(SecAuthMessages.unsupoprtedCharEncoding, StorageUtils.CHAR_ENCODING);
+				AuthPlugin.getDefault().logMessage(msg);
+				firstCharsetException = false;
+			}
+			return new String(bytes);
+		}
+	}
+
 }