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/18 15:53:32 UTC
[02/13] git commit: ACCUMULO-1986 Add data integrity checks to Key
and Mutation
ACCUMULO-1986 Add data integrity checks to Key and Mutation
This change adds checks to the constructors for Key and Mutations which
take in Thrift data structures to ensure that required fields are not
null. These checks prevent creation of invalid objects from modified
Thrift structures.
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/adee0f12
Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/adee0f12
Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/adee0f12
Branch: refs/heads/master
Commit: adee0f129f66c346e026b1803793caa233d29930
Parents: bec36bc
Author: Bill Havanki <bh...@cloudera.com>
Authored: Thu Dec 12 16:15:08 2013 -0500
Committer: Eric Newton <er...@gmail.com>
Committed: Wed Dec 18 09:23:47 2013 -0500
----------------------------------------------------------------------
.../java/org/apache/accumulo/core/data/Key.java | 13 +++
.../org/apache/accumulo/core/data/Mutation.java | 7 ++
.../org/apache/accumulo/core/data/KeyTest.java | 29 +++++-
.../apache/accumulo/core/data/MutationTest.java | 97 +++++++++++++-------
4 files changed, 111 insertions(+), 35 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/accumulo/blob/adee0f12/src/core/src/main/java/org/apache/accumulo/core/data/Key.java
----------------------------------------------------------------------
diff --git a/src/core/src/main/java/org/apache/accumulo/core/data/Key.java b/src/core/src/main/java/org/apache/accumulo/core/data/Key.java
index cfb0b5c..b6cfad7 100644
--- a/src/core/src/main/java/org/apache/accumulo/core/data/Key.java
+++ b/src/core/src/main/java/org/apache/accumulo/core/data/Key.java
@@ -291,6 +291,19 @@ public class Key implements WritableComparable<Key>, Cloneable {
this.colVisibility = toBytes(tkey.colVisibility);
this.timestamp = tkey.timestamp;
this.deleted = false;
+
+ if (row == null) {
+ throw new IllegalArgumentException("null row");
+ }
+ if (colFamily == null) {
+ throw new IllegalArgumentException("null column family");
+ }
+ if (colQualifier == null) {
+ throw new IllegalArgumentException("null column qualifier");
+ }
+ if (colVisibility == null) {
+ throw new IllegalArgumentException("null column visibility");
+ }
}
/**
http://git-wip-us.apache.org/repos/asf/accumulo/blob/adee0f12/src/core/src/main/java/org/apache/accumulo/core/data/Mutation.java
----------------------------------------------------------------------
diff --git a/src/core/src/main/java/org/apache/accumulo/core/data/Mutation.java b/src/core/src/main/java/org/apache/accumulo/core/data/Mutation.java
index 3979da9..6b2c09f 100644
--- a/src/core/src/main/java/org/apache/accumulo/core/data/Mutation.java
+++ b/src/core/src/main/java/org/apache/accumulo/core/data/Mutation.java
@@ -187,6 +187,13 @@ public class Mutation implements Writable {
this.data = ByteBufferUtil.toBytes(tmutation.data);
this.entries = tmutation.entries;
this.values = ByteBufferUtil.toBytesList(tmutation.values);
+
+ if (this.row == null) {
+ throw new IllegalArgumentException("null row");
+ }
+ if (this.data == null) {
+ throw new IllegalArgumentException("null serialized data");
+ }
}
public Mutation(Mutation m) {
http://git-wip-us.apache.org/repos/asf/accumulo/blob/adee0f12/src/core/src/test/java/org/apache/accumulo/core/data/KeyTest.java
----------------------------------------------------------------------
diff --git a/src/core/src/test/java/org/apache/accumulo/core/data/KeyTest.java b/src/core/src/test/java/org/apache/accumulo/core/data/KeyTest.java
index 9a7f0d7..3ea77e3 100644
--- a/src/core/src/test/java/org/apache/accumulo/core/data/KeyTest.java
+++ b/src/core/src/test/java/org/apache/accumulo/core/data/KeyTest.java
@@ -16,11 +16,18 @@
*/
package org.apache.accumulo.core.data;
-import junit.framework.TestCase;
+import org.junit.Test;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotSame;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
+import org.apache.accumulo.core.data.thrift.TKey;
import org.apache.hadoop.io.Text;
-public class KeyTest extends TestCase {
+public class KeyTest {
+
+ @Test
public void testDeletedCompare() {
Key k1 = new Key("r1".getBytes(), "cf".getBytes(), "cq".getBytes(), new byte[0], 0, false);
Key k2 = new Key("r1".getBytes(), "cf".getBytes(), "cq".getBytes(), new byte[0], 0, false);
@@ -33,6 +40,7 @@ public class KeyTest extends TestCase {
assertTrue(k3.compareTo(k1) < 0);
}
+ @Test
public void testCopyData() {
byte row[] = "r".getBytes();
byte cf[] = "cf".getBytes();
@@ -66,6 +74,7 @@ public class KeyTest extends TestCase {
}
+ @Test
public void testString() {
Key k1 = new Key("r1");
Key k2 = new Key(new Text("r1"));
@@ -93,8 +102,24 @@ public class KeyTest extends TestCase {
}
+ @Test
public void testVisibilityFollowingKey() {
Key k = new Key("r", "f", "q", "v");
assertEquals(k.followingKey(PartialKey.ROW_COLFAM_COLQUAL_COLVIS).toString(), "r f:q [v%00;] " + Long.MAX_VALUE + " false");
}
+
+ @Test
+ public void testThrift() {
+ Key k = new Key("r1", "cf2", "cq2", "cv");
+ TKey tk = k.toThrift();
+ Key k2 = new Key(tk);
+ assertEquals(k, k2);
+ }
+ @Test(expected=IllegalArgumentException.class)
+ public void testThrift_Invalid() {
+ Key k = new Key("r1", "cf2", "cq2", "cv");
+ TKey tk = k.toThrift();
+ tk.setRow((byte[]) null);
+ new Key(tk);
+ }
}
http://git-wip-us.apache.org/repos/asf/accumulo/blob/adee0f12/src/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java
----------------------------------------------------------------------
diff --git a/src/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java b/src/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java
index 38ddcad..2d04430 100644
--- a/src/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java
+++ b/src/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java
@@ -24,12 +24,17 @@ import java.io.IOException;
import java.util.Arrays;
import java.util.List;
-import junit.framework.TestCase;
+import org.junit.Test;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import org.apache.accumulo.core.data.thrift.TMutation;
import org.apache.accumulo.core.security.ColumnVisibility;
import org.apache.hadoop.io.Text;
-public class MutationTest extends TestCase {
+public class MutationTest {
+ @Test
public void test1() {
Mutation m = new Mutation(new Text("r1"));
m.put(new Text("cf1"), new Text("cq1"), new Value("v1".getBytes()));
@@ -47,6 +52,7 @@ public class MutationTest extends TestCase {
}
+ @Test
public void test2() throws IOException {
Mutation m = new Mutation(new Text("r1"));
m.put(new Text("cf1"), new Text("cq1"), new Value("v1".getBytes()));
@@ -113,6 +119,7 @@ public class MutationTest extends TestCase {
return m;
}
+ @Test
public void test3() throws IOException {
Mutation m = new Mutation(new Text("r1"));
for (int i = 0; i < 5; i++) {
@@ -155,6 +162,7 @@ public class MutationTest extends TestCase {
return new Value(s.getBytes());
}
+ @Test
public void testPuts() {
Mutation m = new Mutation(new Text("r1"));
@@ -175,18 +183,19 @@ public class MutationTest extends TestCase {
assertEquals(8, m.size());
assertEquals(8, updates.size());
- assertEquals(updates.get(0), "cf1", "cq1", "", 0l, false, false, "v1");
- assertEquals(updates.get(1), "cf2", "cq2", "cv2", 0l, false, false, "v2");
- assertEquals(updates.get(2), "cf3", "cq3", "", 3l, true, false, "v3");
- assertEquals(updates.get(3), "cf4", "cq4", "cv4", 4l, true, false, "v4");
+ verifyColumnUpdate(updates.get(0), "cf1", "cq1", "", 0l, false, false, "v1");
+ verifyColumnUpdate(updates.get(1), "cf2", "cq2", "cv2", 0l, false, false, "v2");
+ verifyColumnUpdate(updates.get(2), "cf3", "cq3", "", 3l, true, false, "v3");
+ verifyColumnUpdate(updates.get(3), "cf4", "cq4", "cv4", 4l, true, false, "v4");
- assertEquals(updates.get(4), "cf5", "cq5", "", 0l, false, true, "");
- assertEquals(updates.get(5), "cf6", "cq6", "cv6", 0l, false, true, "");
- assertEquals(updates.get(6), "cf7", "cq7", "", 7l, true, true, "");
- assertEquals(updates.get(7), "cf8", "cq8", "cv8", 8l, true, true, "");
+ verifyColumnUpdate(updates.get(4), "cf5", "cq5", "", 0l, false, true, "");
+ verifyColumnUpdate(updates.get(5), "cf6", "cq6", "cv6", 0l, false, true, "");
+ verifyColumnUpdate(updates.get(6), "cf7", "cq7", "", 7l, true, true, "");
+ verifyColumnUpdate(updates.get(7), "cf8", "cq8", "cv8", 8l, true, true, "");
}
+ @Test
public void testPutsString() {
Mutation m = new Mutation(new Text("r1"));
@@ -207,17 +216,18 @@ public class MutationTest extends TestCase {
assertEquals(8, m.size());
assertEquals(8, updates.size());
- assertEquals(updates.get(0), "cf1", "cq1", "", 0l, false, false, "v1");
- assertEquals(updates.get(1), "cf2", "cq2", "cv2", 0l, false, false, "v2");
- assertEquals(updates.get(2), "cf3", "cq3", "", 3l, true, false, "v3");
- assertEquals(updates.get(3), "cf4", "cq4", "cv4", 4l, true, false, "v4");
+ verifyColumnUpdate(updates.get(0), "cf1", "cq1", "", 0l, false, false, "v1");
+ verifyColumnUpdate(updates.get(1), "cf2", "cq2", "cv2", 0l, false, false, "v2");
+ verifyColumnUpdate(updates.get(2), "cf3", "cq3", "", 3l, true, false, "v3");
+ verifyColumnUpdate(updates.get(3), "cf4", "cq4", "cv4", 4l, true, false, "v4");
- assertEquals(updates.get(4), "cf5", "cq5", "", 0l, false, true, "");
- assertEquals(updates.get(5), "cf6", "cq6", "cv6", 0l, false, true, "");
- assertEquals(updates.get(6), "cf7", "cq7", "", 7l, true, true, "");
- assertEquals(updates.get(7), "cf8", "cq8", "cv8", 8l, true, true, "");
+ verifyColumnUpdate(updates.get(4), "cf5", "cq5", "", 0l, false, true, "");
+ verifyColumnUpdate(updates.get(5), "cf6", "cq6", "cv6", 0l, false, true, "");
+ verifyColumnUpdate(updates.get(6), "cf7", "cq7", "", 7l, true, true, "");
+ verifyColumnUpdate(updates.get(7), "cf8", "cq8", "cv8", 8l, true, true, "");
}
+ @Test
public void testPutsStringString() {
Mutation m = new Mutation(new Text("r1"));
@@ -238,15 +248,15 @@ public class MutationTest extends TestCase {
assertEquals(8, m.size());
assertEquals(8, updates.size());
- assertEquals(updates.get(0), "cf1", "cq1", "", 0l, false, false, "v1");
- assertEquals(updates.get(1), "cf2", "cq2", "cv2", 0l, false, false, "v2");
- assertEquals(updates.get(2), "cf3", "cq3", "", 3l, true, false, "v3");
- assertEquals(updates.get(3), "cf4", "cq4", "cv4", 4l, true, false, "v4");
+ verifyColumnUpdate(updates.get(0), "cf1", "cq1", "", 0l, false, false, "v1");
+ verifyColumnUpdate(updates.get(1), "cf2", "cq2", "cv2", 0l, false, false, "v2");
+ verifyColumnUpdate(updates.get(2), "cf3", "cq3", "", 3l, true, false, "v3");
+ verifyColumnUpdate(updates.get(3), "cf4", "cq4", "cv4", 4l, true, false, "v4");
- assertEquals(updates.get(4), "cf5", "cq5", "", 0l, false, true, "");
- assertEquals(updates.get(5), "cf6", "cq6", "cv6", 0l, false, true, "");
- assertEquals(updates.get(6), "cf7", "cq7", "", 7l, true, true, "");
- assertEquals(updates.get(7), "cf8", "cq8", "cv8", 8l, true, true, "");
+ verifyColumnUpdate(updates.get(4), "cf5", "cq5", "", 0l, false, true, "");
+ verifyColumnUpdate(updates.get(5), "cf6", "cq6", "cv6", 0l, false, true, "");
+ verifyColumnUpdate(updates.get(6), "cf7", "cq7", "", 7l, true, true, "");
+ verifyColumnUpdate(updates.get(7), "cf8", "cq8", "cv8", 8l, true, true, "");
}
/**
@@ -254,6 +264,7 @@ public class MutationTest extends TestCase {
* the first set of column updates (and value lengths). Hadoop input formats reuse objects when reading, so if Mutations are used with an input format (or as
* the input to a combiner or reducer) then they will be used in this fashion.
*/
+ @Test
public void testMultipleReadFieldsCalls() throws IOException {
// Create test mutations and write them to a byte output stream
Mutation m1 = new Mutation("row1");
@@ -290,9 +301,9 @@ public class MutationTest extends TestCase {
assertEquals(size1, m.size());
assertEquals(nb1, m.numBytes());
assertEquals(3, m.getUpdates().size());
- assertEquals(m.getUpdates().get(0), "cf1.1", "cq1.1", "A|B", 0L, false, false, "val1.1");
- assertEquals(m.getUpdates().get(1), "cf1.2", "cq1.2", "C|D", 0L, false, false, "val1.2");
- assertEquals(m.getUpdates().get(2), "cf1.3", "cq1.3", "E|F", 0L, false, false, new String(val1_3));
+ verifyColumnUpdate(m.getUpdates().get(0), "cf1.1", "cq1.1", "A|B", 0L, false, false, "val1.1");
+ verifyColumnUpdate(m.getUpdates().get(1), "cf1.2", "cq1.2", "C|D", 0L, false, false, "val1.2");
+ verifyColumnUpdate(m.getUpdates().get(2), "cf1.3", "cq1.3", "E|F", 0L, false, false, new String(val1_3));
// Reuse the same mutation object (which is what happens in the hadoop framework
// when objects are read by an input format)
@@ -302,10 +313,10 @@ public class MutationTest extends TestCase {
assertEquals(size2, m.size());
assertEquals(nb2, m.numBytes());
assertEquals(1, m.getUpdates().size());
- assertEquals(m.getUpdates().get(0), "cf2", "cq2", "G|H", 1234L, true, false, new String(val2));
+ verifyColumnUpdate(m.getUpdates().get(0), "cf2", "cq2", "G|H", 1234L, true, false, new String(val2));
}
- private void assertEquals(ColumnUpdate cu, String cf, String cq, String cv, long ts, boolean timeSet, boolean deleted, String val) {
+ private void verifyColumnUpdate(ColumnUpdate cu, String cf, String cq, String cv, long ts, boolean timeSet, boolean deleted, String val) {
assertEquals(cf, new String(cu.getColumnFamily()));
assertEquals(cq, new String(cu.getColumnQualifier()));
@@ -317,6 +328,7 @@ public class MutationTest extends TestCase {
assertEquals(val, new String(cu.getValue()));
}
+ @Test
public void test4() throws Exception {
Mutation m1 = new Mutation(new Text("r1"));
@@ -343,11 +355,12 @@ public class MutationTest extends TestCase {
assertEquals("r1", new String(m2.getRow()));
assertEquals(2, m2.getUpdates().size());
assertEquals(2, m2.size());
- assertEquals(m2.getUpdates().get(0), "cf1", "cq1", "", 0l, false, false, "v1");
- assertEquals(m2.getUpdates().get(1), "cf2", "cq2", "cv2", 0l, false, false, "v2");
+ verifyColumnUpdate(m2.getUpdates().get(0), "cf1", "cq1", "", 0l, false, false, "v1");
+ verifyColumnUpdate(m2.getUpdates().get(1), "cf2", "cq2", "cv2", 0l, false, false, "v2");
}
+ @Test
public void testEquals() {
Mutation m1 = new Mutation("r1");
@@ -416,4 +429,22 @@ public class MutationTest extends TestCase {
assertFalse(m3.equals(m5));
assertFalse(m4.equals(m5));
}
+
+ @Test
+ public void testThrift() {
+ Mutation m1 = new Mutation("r1");
+ m1.put("cf1", "cq1", "v1");
+ TMutation tm1 = m1.toThrift();
+ Mutation m2 = new Mutation(tm1);
+ assertEquals(m1, m2);
+ }
+ @Test(expected=IllegalArgumentException.class)
+ public void testThrift_Invalid() {
+ Mutation m1 = new Mutation("r1");
+ m1.put("cf1", "cq1", "v1");
+ TMutation tm1 = m1.toThrift();
+ tm1.setRow((byte[]) null);
+ new Mutation(tm1);
+ }
+
}