diff --git a/CHANGES.txt b/CHANGES.txt index de216c8688..73673e374d 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.2 + * Harden parsing of boolean values in CQL in PropertyDefinitions (CASSANDRA-17878) * Add nodetool forcecompact to remove tombstoned or ttl'd data ignoring GC grace for given table and partition keys (CASSANDRA-17711) * Offer IF (NOT) EXISTS in cqlsh completion for CREATE TYPE, DROP TYPE, CREATE ROLE and DROP ROLE (CASSANDRA-16640) * Nodetool bootstrap resume will now return an error if the operation fails (CASSANDRA-16491) diff --git a/src/java/org/apache/cassandra/cql3/statements/PropertyDefinitions.java b/src/java/org/apache/cassandra/cql3/statements/PropertyDefinitions.java index c7771728b2..65ec8fca67 100644 --- a/src/java/org/apache/cassandra/cql3/statements/PropertyDefinitions.java +++ b/src/java/org/apache/cassandra/cql3/statements/PropertyDefinitions.java @@ -17,7 +17,9 @@ */ package org.apache.cassandra.cql3.statements; -import java.util.*; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; import java.util.regex.Pattern; import org.slf4j.Logger; @@ -25,24 +27,27 @@ import org.slf4j.LoggerFactory; import org.apache.cassandra.exceptions.SyntaxException; +import static java.lang.String.format; + public class PropertyDefinitions { - private static final Pattern PATTERN_POSITIVE = Pattern.compile("(1|true|yes)"); + private static final Pattern POSITIVE_PATTERN = Pattern.compile("(1|true|yes)"); + private static final Pattern NEGATIVE_PATTERN = Pattern.compile("(0|false|no)"); protected static final Logger logger = LoggerFactory.getLogger(PropertyDefinitions.class); - protected final Map properties = new HashMap(); + protected final Map properties = new HashMap<>(); public void addProperty(String name, String value) throws SyntaxException { if (properties.put(name, value) != null) - throw new SyntaxException(String.format("Multiple definition for property '%s'", name)); + throw new SyntaxException(format("Multiple definitions for property '%s'", name)); } public void addProperty(String name, Map value) throws SyntaxException { if (properties.put(name, value) != null) - throw new SyntaxException(String.format("Multiple definition for property '%s'", name)); + throw new SyntaxException(format("Multiple definitions for property '%s'", name)); } public void validate(Set keywords, Set obsolete) throws SyntaxException @@ -55,7 +60,7 @@ public class PropertyDefinitions if (obsolete.contains(name)) logger.warn("Ignoring obsolete property {}", name); else - throw new SyntaxException(String.format("Unknown property '%s'", name)); + throw new SyntaxException(format("Unknown property '%s'", name)); } } @@ -72,13 +77,18 @@ public class PropertyDefinitions properties.remove(name); } - protected String getSimple(String name) throws SyntaxException + public boolean hasProperty(String name) + { + return properties.containsKey(name); + } + + protected String getString(String name) throws SyntaxException { Object val = properties.get(name); if (val == null) return null; if (!(val instanceof String)) - throw new SyntaxException(String.format("Invalid value for property '%s'. It should be a string", name)); + throw new SyntaxException(format("Invalid value for property '%s'. It should be a string", name)); return (String)val; } @@ -88,72 +98,73 @@ public class PropertyDefinitions if (val == null) return null; if (!(val instanceof Map)) - throw new SyntaxException(String.format("Invalid value for property '%s'. It should be a map.", name)); + throw new SyntaxException(format("Invalid value for property '%s'. It should be a map.", name)); return (Map)val; } - public Boolean hasProperty(String name) + public boolean getBoolean(String key, boolean defaultValue) throws SyntaxException { - return properties.containsKey(name); + String value = getString(key); + return value != null ? parseBoolean(key, value) : defaultValue; } - public String getString(String key, String defaultValue) throws SyntaxException + public static boolean parseBoolean(String key, String value) throws SyntaxException { - String value = getSimple(key); - return value != null ? value : defaultValue; + if (null == value) + throw new IllegalArgumentException("value argument can't be null"); + + String lowerCasedValue = value.toLowerCase(); + + if (POSITIVE_PATTERN.matcher(lowerCasedValue).matches()) + return true; + else if (NEGATIVE_PATTERN.matcher(lowerCasedValue).matches()) + return false; + + throw new SyntaxException(format("Invalid boolean value %s for '%s'. " + + "Positive values can be '1', 'true' or 'yes'. " + + "Negative values can be '0', 'false' or 'no'.", + value, key)); } - // Return a property value, typed as a Boolean - public Boolean getBoolean(String key, Boolean defaultValue) throws SyntaxException + public int getInt(String key, int defaultValue) throws SyntaxException { - String value = getSimple(key); - return (value == null) ? defaultValue : PATTERN_POSITIVE.matcher(value.toLowerCase()).matches(); + String value = getString(key); + return value != null ? parseInt(key, value) : defaultValue; + } + + public static int parseInt(String key, String value) throws SyntaxException + { + if (null == value) + throw new IllegalArgumentException("value argument can't be null"); + + try + { + return Integer.parseInt(value); + } + catch (NumberFormatException e) + { + throw new SyntaxException(format("Invalid integer value %s for '%s'", value, key)); + } } - // Return a property value, typed as a double public double getDouble(String key, double defaultValue) throws SyntaxException { - String value = getSimple(key); - if (value == null) - { - return defaultValue; - } - else - { - try - { - return Double.parseDouble(value); - } - catch (NumberFormatException e) - { - throw new SyntaxException(String.format("Invalid double value %s for '%s'", value, key)); - } - } + String value = getString(key); + return value != null ? parseDouble(key, value) : defaultValue; } - // Return a property value, typed as an Integer - public Integer getInt(String key, Integer defaultValue) throws SyntaxException + public static double parseDouble(String key, String value) throws SyntaxException { - String value = getSimple(key); - return toInt(key, value, defaultValue); - } + if (null == value) + throw new IllegalArgumentException("value argument can't be null"); - public static Integer toInt(String key, String value, Integer defaultValue) throws SyntaxException - { - if (value == null) + try { - return defaultValue; + return Double.parseDouble(value); } - else + catch (NumberFormatException e) { - try - { - return Integer.valueOf(value); - } - catch (NumberFormatException e) - { - throw new SyntaxException(String.format("Invalid integer value %s for '%s'", value, key)); - } + throw new SyntaxException(format("Invalid double value %s for '%s'", value, key)); } } } diff --git a/src/java/org/apache/cassandra/cql3/statements/schema/TableAttributes.java b/src/java/org/apache/cassandra/cql3/statements/schema/TableAttributes.java index e146e69e23..2f1eaf025c 100644 --- a/src/java/org/apache/cassandra/cql3/statements/schema/TableAttributes.java +++ b/src/java/org/apache/cassandra/cql3/statements/schema/TableAttributes.java @@ -25,7 +25,6 @@ import com.google.common.collect.Sets; import org.apache.cassandra.cql3.statements.PropertyDefinitions; import org.apache.cassandra.exceptions.ConfigurationException; -import org.apache.cassandra.exceptions.SyntaxException; import org.apache.cassandra.schema.CachingParams; import org.apache.cassandra.schema.CompactionParams; import org.apache.cassandra.schema.CompressionParams; @@ -37,6 +36,7 @@ import org.apache.cassandra.service.reads.SpeculativeRetryPolicy; import org.apache.cassandra.service.reads.repair.ReadRepairStrategy; import static java.lang.String.format; +import static org.apache.cassandra.schema.TableParams.Option.*; public final class TableAttributes extends PropertyDefinitions { @@ -74,7 +74,7 @@ public final class TableAttributes extends PropertyDefinitions public TableId getId() throws ConfigurationException { - String id = getSimple(ID); + String id = getString(ID); try { return id != null ? TableId.fromString(id) : null; @@ -100,111 +100,75 @@ public final class TableAttributes extends PropertyDefinitions if (hasOption(Option.ALLOW_AUTO_SNAPSHOT)) builder.allowAutoSnapshot(getBoolean(Option.ALLOW_AUTO_SNAPSHOT.toString(), true)); - if (hasOption(Option.BLOOM_FILTER_FP_CHANCE)) - builder.bloomFilterFpChance(getDouble(Option.BLOOM_FILTER_FP_CHANCE)); + if (hasOption(BLOOM_FILTER_FP_CHANCE)) + builder.bloomFilterFpChance(getDouble(BLOOM_FILTER_FP_CHANCE)); - if (hasOption(Option.CACHING)) - builder.caching(CachingParams.fromMap(getMap(Option.CACHING))); + if (hasOption(CACHING)) + builder.caching(CachingParams.fromMap(getMap(CACHING))); - if (hasOption(Option.COMMENT)) - builder.comment(getString(Option.COMMENT)); + if (hasOption(COMMENT)) + builder.comment(getString(COMMENT)); - if (hasOption(Option.COMPACTION)) - builder.compaction(CompactionParams.fromMap(getMap(Option.COMPACTION))); + if (hasOption(COMPACTION)) + builder.compaction(CompactionParams.fromMap(getMap(COMPACTION))); - if (hasOption(Option.COMPRESSION)) + if (hasOption(COMPRESSION)) { - //crc_check_chance was "promoted" from a compression property to a top-level-property after #9839 + //crc_check_chance was "promoted" from a compression property to a top-level-property after #9839, //so we temporarily accept it to be defined as a compression option, to maintain backwards compatibility - Map compressionOpts = getMap(Option.COMPRESSION); - if (compressionOpts.containsKey(Option.CRC_CHECK_CHANCE.toString().toLowerCase())) + Map compressionOpts = getMap(COMPRESSION); + if (compressionOpts.containsKey(CRC_CHECK_CHANCE.toString().toLowerCase())) { - Double crcCheckChance = getDeprecatedCrcCheckChance(compressionOpts); + double crcCheckChance = getDeprecatedCrcCheckChance(compressionOpts); builder.crcCheckChance(crcCheckChance); } - builder.compression(CompressionParams.fromMap(getMap(Option.COMPRESSION))); + builder.compression(CompressionParams.fromMap(getMap(COMPRESSION))); } if (hasOption(Option.MEMTABLE)) builder.memtable(MemtableParams.get(getString(Option.MEMTABLE))); - if (hasOption(Option.DEFAULT_TIME_TO_LIVE)) - builder.defaultTimeToLive(getInt(Option.DEFAULT_TIME_TO_LIVE)); + if (hasOption(DEFAULT_TIME_TO_LIVE)) + builder.defaultTimeToLive(getInt(DEFAULT_TIME_TO_LIVE)); - if (hasOption(Option.GC_GRACE_SECONDS)) - builder.gcGraceSeconds(getInt(Option.GC_GRACE_SECONDS)); + if (hasOption(GC_GRACE_SECONDS)) + builder.gcGraceSeconds(getInt(GC_GRACE_SECONDS)); - if (hasOption(Option.MAX_INDEX_INTERVAL)) - builder.maxIndexInterval(getInt(Option.MAX_INDEX_INTERVAL)); + if (hasOption(MAX_INDEX_INTERVAL)) + builder.maxIndexInterval(getInt(MAX_INDEX_INTERVAL)); - if (hasOption(Option.MEMTABLE_FLUSH_PERIOD_IN_MS)) - builder.memtableFlushPeriodInMs(getInt(Option.MEMTABLE_FLUSH_PERIOD_IN_MS)); + if (hasOption(MEMTABLE_FLUSH_PERIOD_IN_MS)) + builder.memtableFlushPeriodInMs(getInt(MEMTABLE_FLUSH_PERIOD_IN_MS)); - if (hasOption(Option.MIN_INDEX_INTERVAL)) - builder.minIndexInterval(getInt(Option.MIN_INDEX_INTERVAL)); + if (hasOption(MIN_INDEX_INTERVAL)) + builder.minIndexInterval(getInt(MIN_INDEX_INTERVAL)); - if (hasOption(Option.SPECULATIVE_RETRY)) - builder.speculativeRetry(SpeculativeRetryPolicy.fromString(getString(Option.SPECULATIVE_RETRY))); + if (hasOption(SPECULATIVE_RETRY)) + builder.speculativeRetry(SpeculativeRetryPolicy.fromString(getString(SPECULATIVE_RETRY))); - if (hasOption(Option.ADDITIONAL_WRITE_POLICY)) - builder.additionalWritePolicy(SpeculativeRetryPolicy.fromString(getString(Option.ADDITIONAL_WRITE_POLICY))); + if (hasOption(ADDITIONAL_WRITE_POLICY)) + builder.additionalWritePolicy(SpeculativeRetryPolicy.fromString(getString(ADDITIONAL_WRITE_POLICY))); - if (hasOption(Option.CRC_CHECK_CHANCE)) - builder.crcCheckChance(getDouble(Option.CRC_CHECK_CHANCE)); + if (hasOption(CRC_CHECK_CHANCE)) + builder.crcCheckChance(getDouble(CRC_CHECK_CHANCE)); - if (hasOption(Option.CDC)) - builder.cdc(getBoolean(Option.CDC.toString(), false)); + if (hasOption(CDC)) + builder.cdc(getBoolean(CDC)); - if (hasOption(Option.READ_REPAIR)) - builder.readRepair(ReadRepairStrategy.fromString(getString(Option.READ_REPAIR))); + if (hasOption(READ_REPAIR)) + builder.readRepair(ReadRepairStrategy.fromString(getString(READ_REPAIR))); return builder.build(); } - private Double getDeprecatedCrcCheckChance(Map compressionOpts) + public boolean hasOption(Option option) { - String value = compressionOpts.get(Option.CRC_CHECK_CHANCE.toString().toLowerCase()); - try - { - return Double.valueOf(value); - } - catch (NumberFormatException e) - { - throw new SyntaxException(String.format("Invalid double value %s for crc_check_chance.'", value)); - } - } - - private double getDouble(Option option) - { - String value = getString(option); - - try - { - return Double.parseDouble(value); - } - catch (NumberFormatException e) - { - throw new SyntaxException(format("Invalid double value %s for '%s'", value, option)); - } - } - - private int getInt(Option option) - { - String value = getString(option); - - try - { - return Integer.parseInt(value); - } - catch (NumberFormatException e) - { - throw new SyntaxException(String.format("Invalid integer value %s for '%s'", value, option)); - } + return hasProperty(option.toString()); } private String getString(Option option) { - String value = getSimple(option.toString()); + String value = getString(option.toString()); if (value == null) throw new IllegalStateException(format("Option '%s' is absent", option)); return value; @@ -218,8 +182,24 @@ public final class TableAttributes extends PropertyDefinitions return value; } - public boolean hasOption(Option option) + private boolean getBoolean(Option option) { - return hasProperty(option.toString()); + return parseBoolean(option.toString(), getString(option)); + } + + private int getInt(Option option) + { + return parseInt(option.toString(), getString(option)); + } + + private double getDouble(Option option) + { + return parseDouble(option.toString(), getString(option)); + } + + private double getDeprecatedCrcCheckChance(Map compressionOpts) + { + String value = compressionOpts.get(CRC_CHECK_CHANCE.toString().toLowerCase()); + return parseDouble(CRC_CHECK_CHANCE.toString(), value); } } diff --git a/src/java/org/apache/cassandra/schema/TableParams.java b/src/java/org/apache/cassandra/schema/TableParams.java index 920bb829fc..7cb0e9bdb5 100644 --- a/src/java/org/apache/cassandra/schema/TableParams.java +++ b/src/java/org/apache/cassandra/schema/TableParams.java @@ -36,6 +36,7 @@ import org.apache.cassandra.utils.ByteBufferUtil; import static java.lang.String.format; import static java.util.stream.Collectors.toMap; +import static org.apache.cassandra.schema.TableParams.Option.*; public final class TableParams { @@ -90,7 +91,7 @@ public final class TableParams { comment = builder.comment; allowAutoSnapshot = builder.allowAutoSnapshot; - bloomFilterFpChance = builder.bloomFilterFpChance == null + bloomFilterFpChance = builder.bloomFilterFpChance == -1 ? builder.compaction.defaultBloomFilterFbChance() : builder.bloomFilterFpChance; crcCheckChance = builder.crcCheckChance; @@ -151,7 +152,7 @@ public final class TableParams if (bloomFilterFpChance <= minBloomFilterFpChanceValue || bloomFilterFpChance > 1) { fail("%s must be larger than %s and less than or equal to 1.0 (got %s)", - Option.BLOOM_FILTER_FP_CHANCE, + BLOOM_FILTER_FP_CHANCE, minBloomFilterFpChanceValue, bloomFilterFpChance); } @@ -159,33 +160,33 @@ public final class TableParams if (crcCheckChance < 0 || crcCheckChance > 1.0) { fail("%s must be larger than or equal to 0 and smaller than or equal to 1.0 (got %s)", - Option.CRC_CHECK_CHANCE, + CRC_CHECK_CHANCE, crcCheckChance); } if (defaultTimeToLive < 0) - fail("%s must be greater than or equal to 0 (got %s)", Option.DEFAULT_TIME_TO_LIVE, defaultTimeToLive); + fail("%s must be greater than or equal to 0 (got %s)", DEFAULT_TIME_TO_LIVE, defaultTimeToLive); if (defaultTimeToLive > Attributes.MAX_TTL) - fail("%s must be less than or equal to %d (got %s)", Option.DEFAULT_TIME_TO_LIVE, Attributes.MAX_TTL, defaultTimeToLive); + fail("%s must be less than or equal to %d (got %s)", DEFAULT_TIME_TO_LIVE, Attributes.MAX_TTL, defaultTimeToLive); if (gcGraceSeconds < 0) - fail("%s must be greater than or equal to 0 (got %s)", Option.GC_GRACE_SECONDS, gcGraceSeconds); + fail("%s must be greater than or equal to 0 (got %s)", GC_GRACE_SECONDS, gcGraceSeconds); if (minIndexInterval < 1) - fail("%s must be greater than or equal to 1 (got %s)", Option.MIN_INDEX_INTERVAL, minIndexInterval); + fail("%s must be greater than or equal to 1 (got %s)", MIN_INDEX_INTERVAL, minIndexInterval); if (maxIndexInterval < minIndexInterval) { fail("%s must be greater than or equal to %s (%s) (got %s)", - Option.MAX_INDEX_INTERVAL, - Option.MIN_INDEX_INTERVAL, + MAX_INDEX_INTERVAL, + MIN_INDEX_INTERVAL, minIndexInterval, maxIndexInterval); } if (memtableFlushPeriodInMs < 0) - fail("%s must be greater than or equal to 0 (got %s)", Option.MEMTABLE_FLUSH_PERIOD_IN_MS, memtableFlushPeriodInMs); + fail("%s must be greater than or equal to 0 (got %s)", MEMTABLE_FLUSH_PERIOD_IN_MS, memtableFlushPeriodInMs); if (cdc && memtable.factory().writesShouldSkipCommitLog()) fail("CDC cannot work if writes skip the commit log. Check your memtable configuration."); @@ -208,6 +209,7 @@ public final class TableParams TableParams p = (TableParams) o; return comment.equals(p.comment) + && additionalWritePolicy.equals(p.additionalWritePolicy) && allowAutoSnapshot == p.allowAutoSnapshot && bloomFilterFpChance == p.bloomFilterFpChance && crcCheckChance == p.crcCheckChance @@ -230,6 +232,7 @@ public final class TableParams public int hashCode() { return Objects.hashCode(comment, + additionalWritePolicy, allowAutoSnapshot, bloomFilterFpChance, crcCheckChance, @@ -252,23 +255,24 @@ public final class TableParams public String toString() { return MoreObjects.toStringHelper(this) - .add(Option.COMMENT.toString(), comment) - .add(Option.ALLOW_AUTO_SNAPSHOT.toString(), allowAutoSnapshot) - .add(Option.BLOOM_FILTER_FP_CHANCE.toString(), bloomFilterFpChance) - .add(Option.CRC_CHECK_CHANCE.toString(), crcCheckChance) - .add(Option.GC_GRACE_SECONDS.toString(), gcGraceSeconds) - .add(Option.DEFAULT_TIME_TO_LIVE.toString(), defaultTimeToLive) - .add(Option.MEMTABLE_FLUSH_PERIOD_IN_MS.toString(), memtableFlushPeriodInMs) - .add(Option.MIN_INDEX_INTERVAL.toString(), minIndexInterval) - .add(Option.MAX_INDEX_INTERVAL.toString(), maxIndexInterval) - .add(Option.SPECULATIVE_RETRY.toString(), speculativeRetry) - .add(Option.CACHING.toString(), caching) - .add(Option.COMPACTION.toString(), compaction) - .add(Option.COMPRESSION.toString(), compression) - .add(Option.MEMTABLE.toString(), memtable) - .add(Option.EXTENSIONS.toString(), extensions) - .add(Option.CDC.toString(), cdc) - .add(Option.READ_REPAIR.toString(), readRepair) + .add(COMMENT.toString(), comment) + .add(ADDITIONAL_WRITE_POLICY.toString(), additionalWritePolicy) + .add(ALLOW_AUTO_SNAPSHOT.toString(), allowAutoSnapshot) + .add(BLOOM_FILTER_FP_CHANCE.toString(), bloomFilterFpChance) + .add(CRC_CHECK_CHANCE.toString(), crcCheckChance) + .add(GC_GRACE_SECONDS.toString(), gcGraceSeconds) + .add(DEFAULT_TIME_TO_LIVE.toString(), defaultTimeToLive) + .add(MEMTABLE_FLUSH_PERIOD_IN_MS.toString(), memtableFlushPeriodInMs) + .add(MIN_INDEX_INTERVAL.toString(), minIndexInterval) + .add(MAX_INDEX_INTERVAL.toString(), maxIndexInterval) + .add(SPECULATIVE_RETRY.toString(), speculativeRetry) + .add(CACHING.toString(), caching) + .add(COMPACTION.toString(), compaction) + .add(COMPRESSION.toString(), compression) + .add(MEMTABLE.toString(), memtable) + .add(EXTENSIONS.toString(), extensions) + .add(CDC.toString(), cdc) + .add(READ_REPAIR.toString(), readRepair) .toString(); } @@ -325,7 +329,7 @@ public final class TableParams { private String comment = ""; private boolean allowAutoSnapshot = true; - private Double bloomFilterFpChance; + private double bloomFilterFpChance = -1; private double crcCheckChance = 1.0; private int gcGraceSeconds = 864000; // 10 days private int defaultTimeToLive = 0; diff --git a/test/unit/org/apache/cassandra/cql3/statements/PropertyDefinitionsTest.java b/test/unit/org/apache/cassandra/cql3/statements/PropertyDefinitionsTest.java index 18487f76dd..cba0c86bfe 100644 --- a/test/unit/org/apache/cassandra/cql3/statements/PropertyDefinitionsTest.java +++ b/test/unit/org/apache/cassandra/cql3/statements/PropertyDefinitionsTest.java @@ -20,62 +20,46 @@ */ package org.apache.cassandra.cql3.statements; -import org.junit.After; import org.junit.Test; -import org.junit.Before; -import static org.junit.Assert.assertEquals; +import org.apache.cassandra.exceptions.SyntaxException; -public class PropertyDefinitionsTest { - - PropertyDefinitions pd; - - @Before - public void setUp() +import static org.apache.cassandra.cql3.statements.PropertyDefinitions.parseBoolean; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public class PropertyDefinitionsTest +{ + @Test + public void testPostiveBooleanParsing() { - pd = new PropertyDefinitions(); + assertTrue(parseBoolean("prop1", "1")); + assertTrue(parseBoolean("prop2", "true")); + assertTrue(parseBoolean("prop3", "True")); + assertTrue(parseBoolean("prop4", "TrUe")); + assertTrue(parseBoolean("prop5", "yes")); + assertTrue(parseBoolean("prop6", "Yes")); } - - @After - public void clear() - { - pd = null; - } - @Test - public void testGetBooleanExistant() + public void testNegativeBooleanParsing() { - String key = "one"; - pd.addProperty(key, "1"); - assertEquals(Boolean.TRUE, pd.getBoolean(key, null)); - - key = "TRUE"; - pd.addProperty(key, "TrUe"); - assertEquals(Boolean.TRUE, pd.getBoolean(key, null)); - - key = "YES"; - pd.addProperty(key, "YeS"); - assertEquals(Boolean.TRUE, pd.getBoolean(key, null)); - - key = "BAD_ONE"; - pd.addProperty(key, " 1"); - assertEquals(Boolean.FALSE, pd.getBoolean(key, null)); - - key = "BAD_TRUE"; - pd.addProperty(key, "true "); - assertEquals(Boolean.FALSE, pd.getBoolean(key, null)); - - key = "BAD_YES"; - pd.addProperty(key, "ye s"); - assertEquals(Boolean.FALSE, pd.getBoolean(key, null)); + assertFalse(parseBoolean("prop1", "0")); + assertFalse(parseBoolean("prop2", "false")); + assertFalse(parseBoolean("prop3", "False")); + assertFalse(parseBoolean("prop4", "FaLse")); + assertFalse(parseBoolean("prop6", "No")); } - - @Test - public void testGetBooleanNonexistant() + + @Test(expected = SyntaxException.class) + public void testInvalidPositiveBooleanParsing() { - assertEquals(Boolean.FALSE, pd.getBoolean("nonexistant", Boolean.FALSE)); - assertEquals(Boolean.TRUE, pd.getBoolean("nonexistant", Boolean.TRUE)); + parseBoolean("cdc", "tru"); + } + + @Test(expected = SyntaxException.class) + public void testInvalidNegativeBooleanParsing() + { + parseBoolean("cdc", "fals"); } - }