Return-Path: X-Original-To: apmail-hbase-commits-archive@www.apache.org Delivered-To: apmail-hbase-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 2E9BC18D75 for ; Mon, 25 May 2015 08:23:36 +0000 (UTC) Received: (qmail 52773 invoked by uid 500); 25 May 2015 08:23:36 -0000 Delivered-To: apmail-hbase-commits-archive@hbase.apache.org Received: (qmail 52731 invoked by uid 500); 25 May 2015 08:23:36 -0000 Mailing-List: contact commits-help@hbase.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@hbase.apache.org Delivered-To: mailing list commits@hbase.apache.org Received: (qmail 52722 invoked by uid 99); 25 May 2015 08:23:36 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 25 May 2015 08:23:35 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id D8973DFAF3; Mon, 25 May 2015 08:23:35 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: anoopsamjohn@apache.org To: commits@hbase.apache.org Message-Id: X-Mailer: ASF-Git Admin Mailer Subject: hbase git commit: HBASE-13734 Improper timestamp checking with VisibilityScanDeleteTracker. Date: Mon, 25 May 2015 08:23:35 +0000 (UTC) Repository: hbase Updated Branches: refs/heads/master f28e39529 -> d45e0a7d4 HBASE-13734 Improper timestamp checking with VisibilityScanDeleteTracker. Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/d45e0a7d Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/d45e0a7d Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/d45e0a7d Branch: refs/heads/master Commit: d45e0a7d41923c132985ea207dfb72feccded7a9 Parents: f28e395 Author: anoopsjohn Authored: Mon May 25 13:53:13 2015 +0530 Committer: anoopsjohn Committed: Mon May 25 13:53:13 2015 +0530 ---------------------------------------------------------------------- .../visibility/VisibilityScanDeleteTracker.java | 86 ++++++++------- .../TestVisibilityLabelsWithDeletes.java | 106 +++++++++++++++++++ 2 files changed, 153 insertions(+), 39 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/d45e0a7d/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityScanDeleteTracker.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityScanDeleteTracker.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityScanDeleteTracker.java index 80e1d5d..ea7ce33 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityScanDeleteTracker.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityScanDeleteTracker.java @@ -20,12 +20,7 @@ package org.apache.hadoop.hbase.security.visibility; import java.io.IOException; import java.util.ArrayList; -import java.util.HashMap; -import java.util.Iterator; import java.util.List; -import java.util.Map; -import java.util.Map.Entry; -import java.util.Set; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -37,6 +32,7 @@ import org.apache.hadoop.hbase.Tag; import org.apache.hadoop.hbase.regionserver.ScanDeleteTracker; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.Pair; +import org.apache.hadoop.hbase.util.Triple; /** * Similar to ScanDeletTracker but tracks the visibility expression also before @@ -55,12 +51,12 @@ public class VisibilityScanDeleteTracker extends ScanDeleteTracker { // type would solve this problem and also ensure that the combination of different type // of deletes with diff ts would also work fine // Track per TS - private Map, Byte>> visibilityTagsDeleteFamily = - new HashMap, Byte>>(); + private List, Byte, Long>> visibilityTagsDeleteFamily = + new ArrayList, Byte, Long>>(); // Delete family version with different ts and different visibility expression could come. // Need to track it per ts. - private Map, Byte>> visibilityTagsDeleteFamilyVersion = - new HashMap, Byte>>(); + private List, Byte, Long>> visibilityTagsDeleteFamilyVersion = + new ArrayList, Byte, Long>>(); private List, Byte>> visibilityTagsDeleteColumns; // Tracking as List is to handle same ts cell but different visibility tag. // TODO : Need to handle puts with same ts but different vis tags. @@ -80,8 +76,10 @@ public class VisibilityScanDeleteTracker extends ScanDeleteTracker { byte type = delCell.getTypeByte(); if (type == KeyValue.Type.DeleteFamily.getCode()) { hasFamilyStamp = true; - //familyStamps.add(delCell.getTimestamp()); - extractDeleteCellVisTags(delCell, KeyValue.Type.DeleteFamily); + boolean hasVisTag = extractDeleteCellVisTags(delCell, KeyValue.Type.DeleteFamily); + if (!hasVisTag && timestamp > familyStamp) { + familyStamp = timestamp; + } return; } else if (type == KeyValue.Type.DeleteFamilyVersion.getCode()) { familyVersionStamps.add(timestamp); @@ -115,8 +113,9 @@ public class VisibilityScanDeleteTracker extends ScanDeleteTracker { extractDeleteCellVisTags(delCell, KeyValue.Type.codeToType(type)); } - private void extractDeleteCellVisTags(Cell delCell, Type type) { + private boolean extractDeleteCellVisTags(Cell delCell, Type type) { // If tag is present in the delete + boolean hasVisTag = false; if (delCell.getTagsLength() > 0) { switch (type) { case DeleteFamily: @@ -124,8 +123,9 @@ public class VisibilityScanDeleteTracker extends ScanDeleteTracker { if (visibilityTagsDeleteFamily != null) { Byte deleteCellVisTagsFormat = VisibilityUtils.extractVisibilityTags(delCell, delTags); if (!delTags.isEmpty()) { - visibilityTagsDeleteFamily.put(delCell.getTimestamp(), new Pair, Byte>( - delTags, deleteCellVisTagsFormat)); + visibilityTagsDeleteFamily.add(new Triple, Byte, Long>( + delTags, deleteCellVisTagsFormat, delCell.getTimestamp())); + hasVisTag = true; } } break; @@ -133,8 +133,9 @@ public class VisibilityScanDeleteTracker extends ScanDeleteTracker { delTags = new ArrayList(); Byte deleteCellVisTagsFormat = VisibilityUtils.extractVisibilityTags(delCell, delTags); if (!delTags.isEmpty()) { - visibilityTagsDeleteFamilyVersion.put(delCell.getTimestamp(), new Pair, Byte>( - delTags, deleteCellVisTagsFormat)); + visibilityTagsDeleteFamilyVersion.add(new Triple, Byte, Long>(delTags, + deleteCellVisTagsFormat, delCell.getTimestamp())); + hasVisTag = true; } break; case DeleteColumn: @@ -146,6 +147,7 @@ public class VisibilityScanDeleteTracker extends ScanDeleteTracker { if (!delTags.isEmpty()) { visibilityTagsDeleteColumns.add(new Pair, Byte>(delTags, deleteCellVisTagsFormat)); + hasVisTag = true; } break; case Delete: @@ -157,6 +159,7 @@ public class VisibilityScanDeleteTracker extends ScanDeleteTracker { if (!delTags.isEmpty()) { visiblityTagsDeleteColumnVersion.add(new Pair, Byte>(delTags, deleteCellVisTagsFormat)); + hasVisTag = true; } break; default: @@ -180,6 +183,7 @@ public class VisibilityScanDeleteTracker extends ScanDeleteTracker { throw new IllegalArgumentException("Invalid delete type"); } } + return hasVisTag; } @Override @@ -190,26 +194,28 @@ public class VisibilityScanDeleteTracker extends ScanDeleteTracker { try { if (hasFamilyStamp) { if (visibilityTagsDeleteFamily != null) { - Set, Byte>>> deleteFamilies = visibilityTagsDeleteFamily - .entrySet(); - Iterator, Byte>>> iterator = deleteFamilies.iterator(); - while (iterator.hasNext()) { - Entry, Byte>> entry = iterator.next(); - if (timestamp <= entry.getKey()) { + for (int i = 0; i < visibilityTagsDeleteFamily.size(); i++) { + // visibilityTagsDeleteFamily is ArrayList + Triple, Byte, Long> triple = visibilityTagsDeleteFamily.get(i); + if (timestamp <= triple.getThird()) { List putVisTags = new ArrayList(); Byte putCellVisTagsFormat = VisibilityUtils.extractVisibilityTags(cell, putVisTags); boolean matchFound = VisibilityLabelServiceManager .getInstance() .getVisibilityLabelService() - .matchVisibility(putVisTags, putCellVisTagsFormat, entry.getValue().getFirst(), - entry.getValue().getSecond()); + .matchVisibility(putVisTags, putCellVisTagsFormat, triple.getFirst(), + triple.getSecond()); if (matchFound) { + // A return type of FAMILY_DELETED will cause skip for all remaining cells from this + // family. We would like to match visibility expression on every put cells after + // this and only remove those matching with the family delete visibility. So we are + // returning FAMILY_VERSION_DELETED from here. return DeleteResult.FAMILY_VERSION_DELETED; } } } } else { - if (!VisibilityUtils.isVisibilityTagsPresent(cell)) { + if (!VisibilityUtils.isVisibilityTagsPresent(cell) && timestamp<=familyStamp) { // No tags return DeleteResult.FAMILY_VERSION_DELETED; } @@ -217,18 +223,20 @@ public class VisibilityScanDeleteTracker extends ScanDeleteTracker { } if (familyVersionStamps.contains(Long.valueOf(timestamp))) { if (visibilityTagsDeleteFamilyVersion != null) { - Pair, Byte> tags = visibilityTagsDeleteFamilyVersion.get(Long - .valueOf(timestamp)); - if (tags != null) { - List putVisTags = new ArrayList(); - Byte putCellVisTagsFormat = VisibilityUtils.extractVisibilityTags(cell, putVisTags); - boolean matchFound = VisibilityLabelServiceManager - .getInstance() - .getVisibilityLabelService() - .matchVisibility(putVisTags, putCellVisTagsFormat, tags.getFirst(), - tags.getSecond()); - if (matchFound) { - return DeleteResult.FAMILY_VERSION_DELETED; + for (int i = 0; i < visibilityTagsDeleteFamilyVersion.size(); i++) { + // visibilityTagsDeleteFamilyVersion is ArrayList + Triple, Byte, Long> triple = visibilityTagsDeleteFamilyVersion.get(i); + if (timestamp == triple.getThird()) { + List putVisTags = new ArrayList(); + Byte putCellVisTagsFormat = VisibilityUtils.extractVisibilityTags(cell, putVisTags); + boolean matchFound = VisibilityLabelServiceManager + .getInstance() + .getVisibilityLabelService() + .matchVisibility(putVisTags, putCellVisTagsFormat, triple.getFirst(), + triple.getSecond()); + if (matchFound) { + return DeleteResult.FAMILY_VERSION_DELETED; + } } } } else { @@ -309,8 +317,8 @@ public class VisibilityScanDeleteTracker extends ScanDeleteTracker { public void reset() { super.reset(); visibilityTagsDeleteColumns = null; - visibilityTagsDeleteFamily = new HashMap, Byte>>(); - visibilityTagsDeleteFamilyVersion = new HashMap, Byte>>(); + visibilityTagsDeleteFamily = new ArrayList, Byte, Long>>(); + visibilityTagsDeleteFamilyVersion = new ArrayList, Byte, Long>>(); visiblityTagsDeleteColumnVersion = null; } } http://git-wip-us.apache.org/repos/asf/hbase/blob/d45e0a7d/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabelsWithDeletes.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabelsWithDeletes.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabelsWithDeletes.java index a8f7ec3..2e549b2 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabelsWithDeletes.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabelsWithDeletes.java @@ -29,6 +29,7 @@ import org.apache.hadoop.hbase.client.Admin; import org.apache.hadoop.hbase.client.Connection; import org.apache.hadoop.hbase.client.ConnectionFactory; import org.apache.hadoop.hbase.client.Delete; +import org.apache.hadoop.hbase.client.Get; import org.apache.hadoop.hbase.client.Put; import org.apache.hadoop.hbase.client.Result; import org.apache.hadoop.hbase.client.ResultScanner; @@ -2827,6 +2828,111 @@ public class TestVisibilityLabelsWithDeletes { } } + @Test + public void testDeleteWithNoVisibilitiesForPutsAndDeletes() throws Exception { + final TableName tableName = TableName.valueOf(TEST_NAME.getMethodName()); + Admin hBaseAdmin = TEST_UTIL.getHBaseAdmin(); + HColumnDescriptor colDesc = new HColumnDescriptor(fam); + colDesc.setMaxVersions(5); + HTableDescriptor desc = new HTableDescriptor(tableName); + desc.addFamily(colDesc); + hBaseAdmin.createTable(desc); + Put p = new Put (Bytes.toBytes("row1")); + p.addColumn(fam, qual, value); + Table table = TEST_UTIL.getConnection().getTable(tableName); + table.put(p); + p = new Put (Bytes.toBytes("row1")); + p.addColumn(fam, qual1, value); + table.put(p); + p = new Put (Bytes.toBytes("row2")); + p.addColumn(fam, qual, value); + table.put(p); + p = new Put (Bytes.toBytes("row2")); + p.addColumn(fam, qual1, value); + table.put(p); + Delete d = new Delete(Bytes.toBytes("row1")); + table.delete(d); + Get g = new Get(Bytes.toBytes("row1")); + g.setMaxVersions(); + g.setAuthorizations(new Authorizations(SECRET, PRIVATE)); + Result result = table.get(g); + assertEquals(0, result.rawCells().length); + + p = new Put (Bytes.toBytes("row1")); + p.addColumn(fam, qual, value); + table.put(p); + result = table.get(g); + assertEquals(1, result.rawCells().length); + } + + @Test + public void testDeleteWithFamilyDeletesOfSameTsButDifferentVisibilities() throws Exception { + final TableName tableName = TableName.valueOf(TEST_NAME.getMethodName()); + Admin hBaseAdmin = TEST_UTIL.getHBaseAdmin(); + HColumnDescriptor colDesc = new HColumnDescriptor(fam); + colDesc.setMaxVersions(5); + HTableDescriptor desc = new HTableDescriptor(tableName); + desc.addFamily(colDesc); + hBaseAdmin.createTable(desc); + Table table = TEST_UTIL.getConnection().getTable(tableName); + long t1 = 1234L; + CellVisibility cellVisibility1 = new CellVisibility(SECRET); + CellVisibility cellVisibility2 = new CellVisibility(PRIVATE); + // Cell row1:info:qual:1234 with visibility SECRET + Put p = new Put(row1); + p.addColumn(fam, qual, t1, value); + p.setCellVisibility(cellVisibility1); + table.put(p); + + // Cell row1:info:qual1:1234 with visibility PRIVATE + p = new Put(row1); + p.addColumn(fam, qual1, t1, value); + p.setCellVisibility(cellVisibility2); + table.put(p); + + Delete d = new Delete(row1); + d.addFamily(fam, t1); + d.setCellVisibility(cellVisibility2); + table.delete(d); + d = new Delete(row1); + d.addFamily(fam, t1); + d.setCellVisibility(cellVisibility1); + table.delete(d); + + Get g = new Get(row1); + g.setMaxVersions(); + g.setAuthorizations(new Authorizations(SECRET, PRIVATE)); + Result result = table.get(g); + assertEquals(0, result.rawCells().length); + + // Cell row2:info:qual:1234 with visibility SECRET + p = new Put(row2); + p.addColumn(fam, qual, t1, value); + p.setCellVisibility(cellVisibility1); + table.put(p); + + // Cell row2:info:qual1:1234 with visibility PRIVATE + p = new Put(row2); + p.addColumn(fam, qual1, t1, value); + p.setCellVisibility(cellVisibility2); + table.put(p); + + d = new Delete(row2); + d.addFamilyVersion(fam, t1); + d.setCellVisibility(cellVisibility2); + table.delete(d); + d = new Delete(row2); + d.addFamilyVersion(fam, t1); + d.setCellVisibility(cellVisibility1); + table.delete(d); + + g = new Get(row2); + g.setMaxVersions(); + g.setAuthorizations(new Authorizations(SECRET, PRIVATE)); + result = table.get(g); + assertEquals(0, result.rawCells().length); + } + public static Table createTableAndWriteDataWithLabels(TableName tableName, String... labelExps) throws Exception { Table table = null;