Martin Mucha has uploaded a new change for review. Change subject: core: lenient validation of mac pool ranges in engine-config ......................................................................
core: lenient validation of mac pool ranges in engine-config • fail when there's no usable mac in supplied ranges • don't fail when there ranges are empty prior to clipping Change-Id: Id7e47c1ab1c2ffd340e1ff2596ee567557324a28 Bug-Url: https://bugzilla.redhat.com/1165025 Signed-off-by: Martin Mucha <mmu...@redhat.com> --- M backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/MacAddressRangeUtils.java M backend/manager/tools/src/main/java/org/ovirt/engine/core/config/entity/helper/MacAddressPoolRangesValueHelper.java M backend/manager/tools/src/test/java/org/ovirt/engine/core/config/entity/helper/MacAddressPoolRangesValueHelperTest.java 3 files changed, 73 insertions(+), 61 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/90/35390/1 diff --git a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/MacAddressRangeUtils.java b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/MacAddressRangeUtils.java index 3beb76b..7d6e820 100644 --- a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/MacAddressRangeUtils.java +++ b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/MacAddressRangeUtils.java @@ -33,25 +33,39 @@ } public static Collection<LongRange> parseRangeString(String ranges) { - if (StringUtils.isEmpty(ranges)) { + List<String[]> rangeStringBoundaries = rangeStringToStringBoundaries(ranges); + + return parseRangeStringBoundaries(rangeStringBoundaries); + } + + public static Collection<LongRange> parseRangeStringBoundaries(List<String[]> rangeStringBoundaries) { + DisjointRanges disjointRanges = new DisjointRanges(); + for (String[] rangeStringBoundary : rangeStringBoundaries) { + disjointRanges.addRange(macToLong(rangeStringBoundary[0]), macToLong(rangeStringBoundary[1])); + } + + return clipMultiCastsFromRanges(disjointRanges.getRanges()); + } + + public static List<String[]> rangeStringToStringBoundaries(String rangeString) { + if (StringUtils.isEmpty(rangeString)) { return Collections.emptyList(); } - String[] rangesArray = ranges.split("[,]", -1); - DisjointRanges disjointRanges = new DisjointRanges(); - + List<String[]> result = new ArrayList<>(); + String[] rangesArray = rangeString.split("[,]", -1); for (int i = 0; i < rangesArray.length; i++) { String[] startEndArray = rangesArray[i].split("[-]", -1); if (startEndArray.length == 2) { - disjointRanges.addRange(macToLong(startEndArray[0]), macToLong(startEndArray[1])); + result.add(new String[]{ startEndArray[0], startEndArray[1]}); } else { throw new IllegalArgumentException( "Failed to initialize Mac Pool range. Please fix Mac Pool range: rangesArray[i]"); } } - return clipMultiCastsFromRanges(disjointRanges.getRanges()); + return result; } private static Collection<LongRange> clipMultiCastsFromRanges(Collection<LongRange> ranges) { @@ -122,22 +136,4 @@ return Long.parseLong(StringUtils.remove(mac, ':'), HEX_RADIX); } - public static boolean isRangeValid(String start, String end) { - long startNum = macToLong(start); - long endNum = macToLong(end); - - if (startNum > endNum) { - return false; - } - - Collection<LongRange> ranges = parseRangeString(start + "-" + end); - - for (LongRange range : ranges) { - if (range.getMaximumLong() - range.getMinimumLong() < 0) { - return false; - } - } - - return true; - } } diff --git a/backend/manager/tools/src/main/java/org/ovirt/engine/core/config/entity/helper/MacAddressPoolRangesValueHelper.java b/backend/manager/tools/src/main/java/org/ovirt/engine/core/config/entity/helper/MacAddressPoolRangesValueHelper.java index e772c94..fa72dd6 100644 --- a/backend/manager/tools/src/main/java/org/ovirt/engine/core/config/entity/helper/MacAddressPoolRangesValueHelper.java +++ b/backend/manager/tools/src/main/java/org/ovirt/engine/core/config/entity/helper/MacAddressPoolRangesValueHelper.java @@ -1,8 +1,11 @@ package org.ovirt.engine.core.config.entity.helper; +import java.util.Collection; +import java.util.List; import java.util.regex.Pattern; import org.apache.commons.lang.StringUtils; +import org.apache.commons.lang.math.LongRange; import org.ovirt.engine.core.config.entity.ConfigKey; import org.ovirt.engine.core.utils.MacAddressRangeUtils; @@ -26,46 +29,61 @@ return new ValidationResult(false, "The MAC address range cannot be empty."); } - String[] ranges = value.split(","); - for (String range : ranges) { - String[] rangeParts = range.split("-"); - if (rangeParts.length == 2) { - String rangeStart = rangeParts[0].toLowerCase(); - String rangeEnd = rangeParts[1].toLowerCase(); - if (!validateRangePart(rangeStart)) { - return new ValidationResult(false, "The range start is an invalid MAC address. " + rangeStart - + " should be in a format of AA:AA:AA:AA:AA:AA"); - } + try { + List<String[]> rangesBoundaries = MacAddressRangeUtils.rangeStringToStringBoundaries(value); + ValidationResult rangeValidation = validateRangesSyntax(rangesBoundaries); + if (!rangeValidation.isOk()) { + return rangeValidation; + } - if (!validateRangePart(rangeEnd)) { - return new ValidationResult(false, "The range end is an invalid MAC address. " + rangeStart - + " should be in a format of AA:AA:AA:AA:AA:AA"); - } - - if (rangeStart.compareTo(rangeEnd) > 0) { - return new ValidationResult(false, - String.format("The entered range is invalid. %s should be ordered from lower to higer" - + " MAC address. Did you mean %s-%s ?", - range, - rangeEnd, - rangeStart)); - } - - if (!MacAddressRangeUtils.isRangeValid(rangeStart, rangeEnd)) { - return new ValidationResult(false, - String.format("The entered range is invalid. %s contains no valid MAC addresses.", range)); - } - + if (rangesAreEmpty(rangesBoundaries)) { + return new ValidationResult(false, + String.format("The entered ranges is invalid. %s contains no valid MAC addresses.", value)); } else { - return new ValidationResult(false, "The entered value is in imporper format. " + value - + " should be in a format of AA:AA:AA:AA:AA:AA-BB:BB:BB:BB:BB:BB,..."); + return new ValidationResult(true); + } + } catch (IllegalArgumentException e) { + return new ValidationResult(false, "The entered value is in improper format. " + value + + " should be in a format of AA:AA:AA:AA:AA:AA-BB:BB:BB:BB:BB:BB,..."); + } + } + + public static boolean rangesAreEmpty(List<String[]> rangesBoundaries) { + Collection<LongRange> ranges = MacAddressRangeUtils.parseRangeStringBoundaries(rangesBoundaries); + + if (ranges.isEmpty()) { + return true; + } + + for (LongRange range : ranges) { + if (range.getMaximumLong() - range.getMinimumLong() >= 0) { + return false; } } + return true; + } + + private ValidationResult validateRangesSyntax(List<String[]> rangesBoundaries) { + for (String[] rangeBoundaries : rangesBoundaries) { + for (String rangeBoundary : rangeBoundaries) { + ValidationResult startPartValidation = validateRangePart(rangeBoundary); + if (!startPartValidation.isOk()) { + return startPartValidation; + } + } + } return new ValidationResult(true); } - private boolean validateRangePart(String rangePart) { - return MAC_ADDRESS_PATTERN.matcher(rangePart).matches(); + private ValidationResult validateRangePart(String rangePart) { + rangePart.toLowerCase(); + boolean matches = MAC_ADDRESS_PATTERN.matcher(rangePart).matches(); + if (!matches) { + return new ValidationResult(false, "The range start/end is an invalid MAC address. " + rangePart + + " should be in a format of AA:AA:AA:AA:AA:AA"); + } else { + return new ValidationResult(true); + } } } diff --git a/backend/manager/tools/src/test/java/org/ovirt/engine/core/config/entity/helper/MacAddressPoolRangesValueHelperTest.java b/backend/manager/tools/src/test/java/org/ovirt/engine/core/config/entity/helper/MacAddressPoolRangesValueHelperTest.java index 415b4cd..63e0e19 100644 --- a/backend/manager/tools/src/test/java/org/ovirt/engine/core/config/entity/helper/MacAddressPoolRangesValueHelperTest.java +++ b/backend/manager/tools/src/test/java/org/ovirt/engine/core/config/entity/helper/MacAddressPoolRangesValueHelperTest.java @@ -24,7 +24,7 @@ @Test public void validateRanges() { - assertEquals(expectedResult, validator.validate(null, ranges).isOk()); + assertEquals(ranges+" should be "+(expectedResult?"valid":"invalid"), expectedResult, validator.validate(null, ranges).isOk()); } @Parameterized.Parameters @@ -38,13 +38,11 @@ { "AA:AA:AA:AA:AA:AA-AA:AA:AA:AA:AA:AB,AA:AA:AA:AA:AA:AA-AA:AA:AA:AA:AA:AB", true }, { "AA:AA:AA:AA:AA:AA-AA:AA:AA:AA:AA:AB,CC:CC:CC:CC:CC:CC-CC:CC:CC:CC:CC:CD", true }, { "CC:CC:CC:CC:CC:CC-CC:CC:CC:CC:CC:CD,AA:AA:AA:AA:AA:AA-AA:AA:AA:AA:AA:AB", true }, - { "BB:BB:BB:BB:BB:BB-AA:AA:AA:AA:AA:AA", false }, - { "BB:BB:BB:BB:BB:BB-aa:aa:aa:aa:aa:aa", false }, - { "bb:bb:bb:bb:bb:bb-AA:AA:AA:AA:AA:AA", false }, { "BB:BB:BB:BB:BB:BA,BB:BB:BB:BB:BB:BB", false }, { "AA:AA:AA:AA:AA,BB:BB:BB:BB:BB:BB", false }, { "AA-AA-AA-AA-AA-AA-BB-BB-BB-BB-BB-BB", false }, { "AA:AA:AA:AA:AA:AA-AA:AA:AA:AA:AA:AB,XA:AA:AA:AA:AA:AA-BB:BB:BB:BB:BB:BB", false }, + { "01:00:00:00:00:00-01:FF:FF:FF:FF:FF", false}, { null, false }, { "", false }, { " ", false }, -- To view, visit http://gerrit.ovirt.org/35390 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Id7e47c1ab1c2ffd340e1ff2596ee567557324a28 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.5 Gerrit-Owner: Martin Mucha <mmu...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
