From 1f1fdac53b61b8893c00c51862ce3719f3e717f4 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Tue, 20 Mar 2012 16:16:15 +0100 Subject: [PATCH] SONAR-2084 fix some quality flaws --- .../sonar/api/config/PropertyDefinition.java | 8 ++-- .../java/org/sonar/api/config/Settings.java | 17 +++---- .../org/sonar/api/config/AesCipherTest.java | 46 ++++++++++++++++++- 3 files changed, 58 insertions(+), 13 deletions(-) diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/config/PropertyDefinition.java b/sonar-plugin-api/src/main/java/org/sonar/api/config/PropertyDefinition.java index 2adfb8232e7..ab4446eb01e 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/config/PropertyDefinition.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/config/PropertyDefinition.java @@ -38,10 +38,11 @@ public final class PropertyDefinition { private String errorKey = null; - private static Result newError(String key) { + private static Result newError(@Nullable String key) { return new Result(key); } + @Nullable private Result(String errorKey) { this.errorKey = errorKey; } @@ -50,7 +51,8 @@ public final class PropertyDefinition { return StringUtils.isBlank(errorKey); } - public @Nullable String getErrorKey() { + @Nullable + public String getErrorKey() { return errorKey; } } @@ -142,7 +144,7 @@ public final class PropertyDefinition { } public String[] getOptions() { - return options; + return options.clone(); } public String getDescription() { diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/config/Settings.java b/sonar-plugin-api/src/main/java/org/sonar/api/config/Settings.java index 70fc4012415..9bea5640ff6 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/config/Settings.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/config/Settings.java @@ -27,6 +27,7 @@ import org.sonar.api.BatchComponent; import org.sonar.api.ServerComponent; import org.sonar.api.utils.DateUtils; +import javax.annotation.Nullable; import java.util.*; /** @@ -181,42 +182,42 @@ public class Settings implements BatchComponent, ServerComponent { return this; } - public final Settings setProperty(String key, String value) { + public final Settings setProperty(String key, @Nullable String value) { if (!clearIfNullValue(key, value)) { properties.put(key, StringUtils.trim(value)); } return this; } - public final Settings setProperty(String key, Boolean value) { + public final Settings setProperty(String key, @Nullable Boolean value) { if (!clearIfNullValue(key, value)) { properties.put(key, String.valueOf(value)); } return this; } - public final Settings setProperty(String key, Integer value) { + public final Settings setProperty(String key, @Nullable Integer value) { if (!clearIfNullValue(key, value)) { properties.put(key, String.valueOf(value)); } return this; } - public final Settings setProperty(String key, Long value) { + public final Settings setProperty(String key, @Nullable Long value) { if (!clearIfNullValue(key, value)) { properties.put(key, String.valueOf(value)); } return this; } - public final Settings setProperty(String key, Double value) { + public final Settings setProperty(String key, @Nullable Double value) { if (!clearIfNullValue(key, value)) { properties.put(key, String.valueOf(value)); } return this; } - public final Settings setProperty(String key, Date date) { + public final Settings setProperty(String key, @Nullable Date date) { return setProperty(key, date, false); } @@ -247,7 +248,7 @@ public class Settings implements BatchComponent, ServerComponent { return addProperties(props); } - public final Settings setProperty(String key, Date date, boolean includeTime) { + public final Settings setProperty(String key, @Nullable Date date, boolean includeTime) { if (!clearIfNullValue(key, date)) { properties.put(key, includeTime ? DateUtils.formatDateTime(date) : DateUtils.formatDate(date)); } @@ -275,7 +276,7 @@ public class Settings implements BatchComponent, ServerComponent { return definitions; } - private boolean clearIfNullValue(String key, Object value) { + private boolean clearIfNullValue(String key, @Nullable Object value) { if (value == null) { properties.remove(key); return true; diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/config/AesCipherTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/config/AesCipherTest.java index 0b3cc802376..81dfaaed30d 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/config/AesCipherTest.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/config/AesCipherTest.java @@ -21,7 +21,10 @@ package org.sonar.api.config; import org.apache.commons.codec.binary.Base64; import org.apache.commons.lang.StringUtils; +import org.hamcrest.Matchers; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.sonar.api.CoreProperties; import javax.crypto.BadPaddingException; @@ -37,6 +40,9 @@ import static org.junit.Assert.fail; public class AesCipherTest { + @Rule + public ExpectedException thrown = ExpectedException.none(); + @Test public void generateRandomSecretKey() { AesCipher cipher = new AesCipher(new Settings()); @@ -59,6 +65,19 @@ public class AesCipherTest { assertThat(Base64.isArrayByteBase64(encryptedText.getBytes()), is(true)); } + @Test + public void encrypt_bad_key() throws Exception { + thrown.expect(RuntimeException.class); + thrown.expectMessage("Invalid AES key"); + + URL resource = getClass().getResource("/org/sonar/api/config/AesCipherTest/bad_secret_key.txt"); + Settings settings = new Settings(); + settings.setProperty(CoreProperties.ENCRYPTION_SECRET_KEY_PATH, new File(resource.toURI()).getCanonicalPath()); + AesCipher cipher = new AesCipher(settings); + + cipher.encrypt("this is a secret"); + } + @Test public void decrypt() throws Exception { Settings settings = new Settings(); @@ -143,18 +162,41 @@ public class AesCipherTest { assertThat(secretKey.getEncoded().length, greaterThan(10)); } - @Test(expected = IllegalStateException.class) + @Test public void loadSecretKeyFromFile_file_does_not_exist() throws Exception { + thrown.expect(IllegalStateException.class); + AesCipher cipher = new AesCipher(new Settings()); cipher.loadSecretFileFromFile("/file/does/not/exist"); } - @Test(expected = IllegalStateException.class) + @Test public void loadSecretKeyFromFile_no_property() throws Exception { + thrown.expect(IllegalStateException.class); + AesCipher cipher = new AesCipher(new Settings()); cipher.loadSecretFileFromFile(null); } + @Test + public void hasSecretKey() throws Exception { + Settings settings = new Settings(); + settings.setProperty(CoreProperties.ENCRYPTION_SECRET_KEY_PATH, pathToSecretKey()); + AesCipher cipher = new AesCipher(settings); + + assertThat(cipher.hasSecretKey(), Matchers.is(true)); + } + + @Test + public void doesNotHaveSecretKey() throws Exception { + Settings settings = new Settings(); + settings.setProperty(CoreProperties.ENCRYPTION_SECRET_KEY_PATH, "/my/twitter/id/is/SimonBrandhof"); + AesCipher cipher = new AesCipher(settings); + + assertThat(cipher.hasSecretKey(), Matchers.is(false)); + } + + private String pathToSecretKey() throws Exception { URL resource = getClass().getResource("/org/sonar/api/config/AesCipherTest/aes_secret_key.txt"); return new File(resource.toURI()).getCanonicalPath(); -- 2.39.5