Validate that the start and end numbers of reserved and of extensions are
positive and that end >= start.
Change-Id: I94377f31f35a9fd15500bda93b2af1badd8efb39
diff --git a/com.google.eclipse.protobuf.test/src/com/google/eclipse/protobuf/validation/ProtobufJavaValidator_checkIndexRangeBounds.java b/com.google.eclipse.protobuf.test/src/com/google/eclipse/protobuf/validation/ProtobufJavaValidator_checkIndexRangeBounds.java
new file mode 100644
index 0000000..a04cd27
--- /dev/null
+++ b/com.google.eclipse.protobuf.test/src/com/google/eclipse/protobuf/validation/ProtobufJavaValidator_checkIndexRangeBounds.java
@@ -0,0 +1,78 @@
+/*
+ * Copyright (c) 2015 Google Inc.
+ *
+ * All rights reserved. This program and the accompanying materials are made available under the terms of the Eclipse
+ * Public License v1.0 which accompanies this distribution, and is available at
+ *
+ * http://www.eclipse.org/legal/epl-v10.html
+ */
+package com.google.eclipse.protobuf.validation;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static com.google.eclipse.protobuf.junit.core.UnitTestModule.unitTestModule;
+import static com.google.eclipse.protobuf.junit.core.XtextRule.overrideRuntimeModuleWith;
+
+import java.util.List;
+
+import org.eclipse.emf.ecore.EObject;
+import org.eclipse.emf.ecore.EStructuralFeature;
+import org.eclipse.xtext.validation.ValidationMessageAcceptor;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+
+import com.google.eclipse.protobuf.junit.core.XtextRule;
+import com.google.eclipse.protobuf.protobuf.IndexRange;
+import com.google.eclipse.protobuf.protobuf.ProtobufPackage;
+import com.google.inject.Inject;
+
+public class ProtobufJavaValidator_checkIndexRangeBounds {
+ @Rule public XtextRule xtext = overrideRuntimeModuleWith(unitTestModule());
+
+ @Inject private ProtobufJavaValidator validator;
+ private ValidationMessageAcceptor messageAcceptor;
+
+ @Before public void setUp() {
+ messageAcceptor = mock(ValidationMessageAcceptor.class);
+ validator.setMessageAcceptor(messageAcceptor);
+ }
+
+ // syntax = "proto2";
+ //
+ // message Person {
+ // reserved -2 to 2;
+ // }
+ @Test public void should_error_on_negative_bounds() {
+ List<IndexRange> indexRanges = xtext.findAll(IndexRange.class);
+ validator.checkIndexRangeBounds(indexRanges.get(0));
+ verifyError(
+ "Extensions and reserved numbers must be positive.",
+ indexRanges.get(0),
+ ProtobufPackage.Literals.INDEX_RANGE__FROM);
+ verifyNoMoreInteractions(messageAcceptor);
+ }
+
+ // syntax = "proto2";
+ //
+ // message Person {
+ // reserved 3 to 1;
+ // reserved 4 to 4;
+ // }
+ @Test public void should_error_on_end_less_than_start() {
+ List<IndexRange> indexRanges = xtext.findAll(IndexRange.class);
+ validator.checkIndexRangeBounds(indexRanges.get(0));
+ validator.checkIndexRangeBounds(indexRanges.get(1));
+ verifyError("End number must be greater than or equal to start number.", indexRanges.get(0));
+ verifyNoMoreInteractions(messageAcceptor);
+ }
+
+ private void verifyError(String message, EObject errorSource) {
+ verifyError(message, errorSource, null);
+ }
+
+ private void verifyError(String message, EObject errorSource, EStructuralFeature errorFeature) {
+ verify(messageAcceptor).acceptError(message, errorSource, errorFeature, -1, null);
+ }
+}
diff --git a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/model/util/IndexRanges.java b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/model/util/IndexRanges.java
index 47aee62..7eb1a56 100644
--- a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/model/util/IndexRanges.java
+++ b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/model/util/IndexRanges.java
@@ -20,6 +20,13 @@
* @author jogl@google.com (John Glassmyer)
*/
@Singleton public class IndexRanges {
+ /**
+ * Thrown to indicate that a range's end number is less than its start number.
+ */
+ public static class BackwardsRangeException extends Exception {
+ private static final long serialVersionUID = 1L;
+ }
+
private ProtobufGrammarAccess protobufGrammarAccess;
@Inject
@@ -27,7 +34,10 @@
this.protobufGrammarAccess = protobufGrammarAccess;
}
- public Range<Long> toLongRange(IndexRange indexRange) {
+ /**
+ * @throws BackwardsRangeException if the end number is less than the start number
+ */
+ public Range<Long> toLongRange(IndexRange indexRange) throws BackwardsRangeException {
long from = indexRange.getFrom();
Range<Long> range;
@@ -37,7 +47,13 @@
} else if (toString.equals(getMaxKeyword())) {
range = Range.atLeast(from);
} else {
- range = Range.closed(from, Long.valueOf(toString));
+ Long to = Long.valueOf(toString);
+
+ if (to < from) {
+ throw new BackwardsRangeException();
+ }
+
+ range = Range.closed(from, to);
}
return range;
diff --git a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/model/util/IndexedElements.java b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/model/util/IndexedElements.java
index 1b995d4..f0cd083 100644
--- a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/model/util/IndexedElements.java
+++ b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/model/util/IndexedElements.java
@@ -21,6 +21,7 @@
import com.google.common.collect.Iterables;
import com.google.common.collect.Range;
+import com.google.eclipse.protobuf.model.util.IndexRanges.BackwardsRangeException;
import com.google.eclipse.protobuf.protobuf.FieldOption;
import com.google.eclipse.protobuf.protobuf.Group;
import com.google.eclipse.protobuf.protobuf.IndexRange;
@@ -67,20 +68,24 @@
private long findMaxIndex(Iterable<? extends EObject> elements) {
long maxIndex = 0;
- for (EObject e : elements) {
- if (e instanceof OneOf) {
- maxIndex = max(maxIndex, findMaxIndex(((OneOf) e).getElements()));
- } else if (e instanceof IndexedElement) {
- maxIndex = max(maxIndex, indexOf((IndexedElement) e));
- if (e instanceof Group) {
- maxIndex = max(maxIndex, findMaxIndex(((Group) e).getElements()));
+ for (EObject element : elements) {
+ if (element instanceof OneOf) {
+ maxIndex = max(maxIndex, findMaxIndex(((OneOf) element).getElements()));
+ } else if (element instanceof IndexedElement) {
+ maxIndex = max(maxIndex, indexOf((IndexedElement) element));
+ if (element instanceof Group) {
+ maxIndex = max(maxIndex, findMaxIndex(((Group) element).getElements()));
}
- } else if (e instanceof Reserved) {
+ } else if (element instanceof Reserved) {
for (IndexRange indexRange :
- Iterables.filter(((Reserved) e).getReservations(), IndexRange.class)) {
- Range<Long> range = indexRanges.toLongRange(indexRange);
- if (range.hasUpperBound()) {
- maxIndex = max(maxIndex, range.upperEndpoint());
+ Iterables.filter(((Reserved) element).getReservations(), IndexRange.class)) {
+ try {
+ Range<Long> range = indexRanges.toLongRange(indexRange);
+ if (range.hasUpperBound()) {
+ maxIndex = max(maxIndex, range.upperEndpoint());
+ }
+ } catch (BackwardsRangeException e) {
+ // Do not consider reserved ranges that are invalid.
}
}
}
diff --git a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/validation/Messages.java b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/validation/Messages.java
index 85c2f9d..bda1d9f 100644
--- a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/validation/Messages.java
+++ b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/validation/Messages.java
@@ -31,6 +31,8 @@
public static String fieldNumbersMustBePositive;
public static String importingUnsupportedSyntax;
public static String importNotFound;
+ public static String indexRangeEndLessThanStart;
+ public static String indexRangeNonPositive;
public static String invalidMapKeyType;
public static String invalidMapValueType;
public static String literalNotInEnum;
diff --git a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/validation/Messages.properties b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/validation/Messages.properties
index 15aaf6a..60934f8 100644
--- a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/validation/Messages.properties
+++ b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/validation/Messages.properties
@@ -15,6 +15,8 @@
fieldNumbersMustBePositive = Field numbers must be positive integers.
importingUnsupportedSyntax = Importing unsupported file (directly or indirectly.) This may cause errors related to unresolved references.
importNotFound = Import \"%s\" was not found.
+indexRangeEndLessThanStart = End number must be greater than or equal to start number.
+indexRangeNonPositive = Extensions and reserved numbers must be positive.
invalidMapKeyType = Key in a map field must be of integral or string type.
invalidMapValueType = Value in a map field cannot be a map.
literalNotInEnum = Enum type \"%s\" has no value named \"%s\".
diff --git a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/validation/ProtobufJavaValidator.java b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/validation/ProtobufJavaValidator.java
index d37b92d..3c6e173 100644
--- a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/validation/ProtobufJavaValidator.java
+++ b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/validation/ProtobufJavaValidator.java
@@ -16,6 +16,8 @@
import static com.google.eclipse.protobuf.validation.Messages.expectedFieldNumber;
import static com.google.eclipse.protobuf.validation.Messages.expectedSyntaxIdentifier;
import static com.google.eclipse.protobuf.validation.Messages.fieldNumbersMustBePositive;
+import static com.google.eclipse.protobuf.validation.Messages.indexRangeEndLessThanStart;
+import static com.google.eclipse.protobuf.validation.Messages.indexRangeNonPositive;
import static com.google.eclipse.protobuf.validation.Messages.invalidMapKeyType;
import static com.google.eclipse.protobuf.validation.Messages.invalidMapValueType;
import static com.google.eclipse.protobuf.validation.Messages.mapWithModifier;
@@ -49,6 +51,7 @@
import com.google.common.collect.Multimap;
import com.google.common.collect.Range;
import com.google.eclipse.protobuf.model.util.IndexRanges;
+import com.google.eclipse.protobuf.model.util.IndexRanges.BackwardsRangeException;
import com.google.eclipse.protobuf.model.util.IndexedElements;
import com.google.eclipse.protobuf.model.util.Protobufs;
import com.google.eclipse.protobuf.model.util.StringLiterals;
@@ -124,22 +127,44 @@
error(msg, syntax, SYNTAX__NAME, SYNTAX_IS_NOT_KNOWN_ERROR);
}
+ @Check public void checkIndexRangeBounds(IndexRange indexRange) {
+ Range<Long> range;
+ try {
+ range = indexRanges.toLongRange(indexRange);
+ } catch (BackwardsRangeException e) {
+ error(indexRangeEndLessThanStart, indexRange, null);
+ return;
+ }
+
+ if (range.lowerEndpoint() <= 0) {
+ error(indexRangeNonPositive, indexRange, ProtobufPackage.Literals.INDEX_RANGE__FROM);
+ }
+ }
+
@Check public void checkForIndexConflicts(Message message) {
Multimap<EObject, Range<Long>> rangeUsages = LinkedHashMultimap.create();
for (Reserved reserved : getOwnedElements(message, Reserved.class)) {
for (IndexRange indexRange : Iterables.filter(reserved.getReservations(), IndexRange.class)) {
- Range<Long> range = indexRanges.toLongRange(indexRange);
- errorOnConflicts(range, rangeUsages, indexRange, null);
- rangeUsages.put(reserved, range);
+ try {
+ Range<Long> range = indexRanges.toLongRange(indexRange);
+ errorOnConflicts(range, rangeUsages, indexRange, null);
+ rangeUsages.put(reserved, range);
+ } catch (BackwardsRangeException e) {
+ // Do not try to find conflicts with invalid ranges.
+ }
}
}
for (Extensions extensions : getOwnedElements(message, Extensions.class)) {
for (IndexRange indexRange : extensions.getRanges()) {
- Range<Long> range = indexRanges.toLongRange(indexRange);
- errorOnConflicts(range, rangeUsages, indexRange, null);
- rangeUsages.put(extensions, range);
+ try {
+ Range<Long> range = indexRanges.toLongRange(indexRange);
+ errorOnConflicts(range, rangeUsages, indexRange, null);
+ rangeUsages.put(extensions, range);
+ } catch (BackwardsRangeException e) {
+ // Do not try to find conflicts with invalid ranges.
+ }
}
}
