You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by ec...@apache.org on 2013/12/12 17:25:25 UTC
[03/16] git commit: ACCUMULO-1958 Make Range constructor safe
ACCUMULO-1958 Make Range constructor safe
The public six-argument Range constructor lacked a check for the stop
key being before the start key. This change adds the check, plus a
similar, new protected constructor without the check for use by
constructors which do not need it. Checks are also included for
construction from Thrift and population via readFields.
Signed-off-by: Eric Newton <er...@gmail.com>
Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo
Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/e945c8d6
Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/e945c8d6
Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/e945c8d6
Branch: refs/heads/master
Commit: e945c8d69e9d081a0387866a0889b5da3726735b
Parents: c1fbeac
Author: Bill Havanki <bh...@cloudera.com>
Authored: Fri Dec 6 10:43:34 2013 -0500
Committer: Eric Newton <er...@gmail.com>
Committed: Thu Dec 12 11:23:07 2013 -0500
----------------------------------------------------------------------
.../org/apache/accumulo/core/data/Range.java | 58 +++++++++++++++++++-
.../apache/accumulo/core/data/RangeTest.java | 58 ++++++++++++++++++++
2 files changed, 113 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/accumulo/blob/e945c8d6/core/src/main/java/org/apache/accumulo/core/data/Range.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/data/Range.java b/core/src/main/java/org/apache/accumulo/core/data/Range.java
index 7085734..65873c3 100644
--- a/core/src/main/java/org/apache/accumulo/core/data/Range.java
+++ b/core/src/main/java/org/apache/accumulo/core/data/Range.java
@@ -18,6 +18,7 @@ package org.apache.accumulo.core.data;
import java.io.DataInput;
import java.io.DataOutput;
+import java.io.InvalidObjectException;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
@@ -170,10 +171,54 @@ public class Range implements WritableComparable<Range> {
* Copies a range
*/
public Range(Range range) {
- this(range.start, range.stop, range.startKeyInclusive, range.stopKeyInclusive, range.infiniteStartKey, range.infiniteStopKey);
+ this(range.start, range.startKeyInclusive, range.infiniteStartKey, range.stop, range.stopKeyInclusive, range.infiniteStopKey);
}
+ /**
+ * Creates a range from start to stop.
+ *
+ * @param start
+ * set this to null when negative infinity is needed
+ * @param stop
+ * set this to null when infinity is needed
+ * @param startKeyInclusive
+ * determines if the ranges includes the start key
+ * @param stopKeyInclusive
+ * determines if the range includes the end key
+ * @param infiniteStartKey
+ * true if start key is negative infinity (null)
+ * @param infiniteStopKey
+ * true if stop key is positive infinity (null)
+ * @throws IllegalArgumentException if stop is before start, or infiniteStartKey is true but start is not null, or infiniteStopKey is true but stop is not
+ * null
+ */
public Range(Key start, Key stop, boolean startKeyInclusive, boolean stopKeyInclusive, boolean infiniteStartKey, boolean infiniteStopKey) {
+ this(start, startKeyInclusive, infiniteStartKey, stop, stopKeyInclusive, infiniteStopKey);
+ if (!infiniteStartKey && !infiniteStopKey && beforeStartKey(stop)) {
+ throw new IllegalArgumentException("Start key must be less than end key in range (" + start + ", " + stop + ")");
+ }
+ }
+
+ /**
+ * Creates a range from start to stop. Unlike the public six-argument method,
+ * this one does not assure that stop is after start, which helps performance
+ * in cases where that assurance is already in place.
+ *
+ * @param start
+ * set this to null when negative infinity is needed
+ * @param startKeyInclusive
+ * determines if the ranges includes the start key
+ * @param infiniteStartKey
+ * true if start key is negative infinity (null)
+ * @param stop
+ * set this to null when infinity is needed
+ * @param stopKeyInclusive
+ * determines if the range includes the end key
+ * @param infiniteStopKey
+ * true if stop key is positive infinity (null)
+ * @throws IllegalArgumentException if infiniteStartKey is true but start is not null, or infiniteStopKey is true but stop is not null
+ */
+ protected Range(Key start, boolean startKeyInclusive, boolean infiniteStartKey, Key stop, boolean stopKeyInclusive, boolean infiniteStopKey) {
if (infiniteStartKey && start != null)
throw new IllegalArgumentException();
@@ -189,8 +234,11 @@ public class Range implements WritableComparable<Range> {
}
public Range(TRange trange) {
- this(trange.start == null ? null : new Key(trange.start), trange.stop == null ? null : new Key(trange.stop), trange.startKeyInclusive,
- trange.stopKeyInclusive, trange.infiniteStartKey, trange.infiniteStopKey);
+ this(trange.start == null ? null : new Key(trange.start), trange.startKeyInclusive, trange.infiniteStartKey,
+ trange.stop == null ? null : new Key(trange.stop), trange.stopKeyInclusive, trange.infiniteStopKey);
+ if (!infiniteStartKey && !infiniteStopKey && beforeStartKey(stop)) {
+ throw new IllegalArgumentException("Start key must be less than end key in range (" + start + ", " + stop + ")");
+ }
}
/**
@@ -566,6 +614,10 @@ public class Range implements WritableComparable<Range> {
startKeyInclusive = in.readBoolean();
stopKeyInclusive = in.readBoolean();
+
+ if (!infiniteStartKey && !infiniteStopKey && beforeStartKey(stop)) {
+ throw new InvalidObjectException("Start key must be less than end key in range (" + start + ", " + stop + ")");
+ }
}
public void write(DataOutput out) throws IOException {
http://git-wip-us.apache.org/repos/asf/accumulo/blob/e945c8d6/core/src/test/java/org/apache/accumulo/core/data/RangeTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/accumulo/core/data/RangeTest.java b/core/src/test/java/org/apache/accumulo/core/data/RangeTest.java
index a8d91b0..68d9731 100644
--- a/core/src/test/java/org/apache/accumulo/core/data/RangeTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/data/RangeTest.java
@@ -16,12 +16,18 @@
*/
package org.apache.accumulo.core.data;
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
+import java.io.InvalidObjectException;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import junit.framework.TestCase;
+import org.apache.accumulo.core.data.thrift.TRange;
import org.apache.hadoop.io.Text;
public class RangeTest extends TestCase {
@@ -761,4 +767,56 @@ public class RangeTest extends TestCase {
assertNull(Range.followingPrefix(makeText((byte) 0xff, (byte) 0xff)));
assertEquals(Range.followingPrefix(makeText((byte) 0x07, (byte) 0xff)), new Text(makeText((byte) 0x08)));
}
+
+ public void testReadFields() throws Exception {
+ Range r = nr("nuts", "soup");
+ ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ DataOutputStream dos = new DataOutputStream(baos);
+ r.write(dos);
+ dos.close();
+ ByteArrayInputStream bais = new ByteArrayInputStream(baos.toByteArray());
+ DataInputStream dis = new DataInputStream(bais);
+ Range r2 = new Range();
+ r2.readFields(dis);
+ dis.close();
+
+ assertEquals(r, r2);
+ }
+
+ public void testReadFields_Check() throws Exception {
+ Range r = new Range(new Key(new Text("soup")), true, false, new Key(new Text("nuts")), true, false);
+ ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ DataOutputStream dos = new DataOutputStream(baos);
+ r.write(dos);
+ dos.close();
+ ByteArrayInputStream bais = new ByteArrayInputStream(baos.toByteArray());
+ DataInputStream dis = new DataInputStream(bais);
+ Range r2 = new Range();
+ try {
+ r2.readFields(dis);
+ fail("readFields allowed invalid range");
+ } catch (InvalidObjectException exc) {
+ /* good! */
+ } finally {
+ dis.close();
+ }
+ }
+
+ public void testThrift() {
+ Range r = nr("nuts", "soup");
+ TRange tr = r.toThrift();
+ Range r2 = new Range(tr);
+ assertEquals(r, r2);
+ }
+
+ public void testThrift_Check() {
+ Range r = new Range(new Key(new Text("soup")), true, false, new Key(new Text("nuts")), true, false);
+ TRange tr = r.toThrift();
+ try {
+ Range r2 = new Range(tr);
+ fail("Thrift constructor allowed invalid range");
+ } catch (IllegalArgumentException exc) {
+ /* good! */
+ }
+ }
}