You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by si...@apache.org on 2018/04/05 07:01:31 UTC
[bookkeeper] branch master updated: Issue #1310: Filter out body
and masterKey from ProcessorV3s
This is an automated email from the ASF dual-hosted git repository.
sijie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
The following commit(s) were added to refs/heads/master by this push:
new 7ec5ff3 Issue #1310: Filter out body and masterKey from ProcessorV3s
7ec5ff3 is described below
commit 7ec5ff3372cde048fa4be464c14ba66bdf6622c7
Author: cguttapalem <cg...@salesforce.com>
AuthorDate: Thu Apr 5 00:01:23 2018 -0700
Issue #1310: Filter out body and masterKey from ProcessorV3s
Descriptions of the changes in this PR:
In certain logs instances of ProcessorV3 (for eg WriteEntryProcessorV3) are logged.
Logging entry data and credentials details is not appropriate. Filter out
those details
(have to consider other ProcessorV3s as well and testcases to validate this change)
Master Issue: #1310
Author: cguttapalem <cg...@salesforce.com>
Reviewers: Enrico Olivelli <eo...@gmail.com>, Sijie Guo <si...@apache.org>, Venkateswararao Jujjuri (JV) <None>
This closes #1311 from reddycharan/filterlogs, closes #1310
---
.../bookkeeper/proto/ReadEntryProcessorV3.java | 12 +++
.../org/apache/bookkeeper/proto/RequestUtils.java | 57 +++++++++++++++
.../bookkeeper/proto/WriteEntryProcessorV3.java | 10 +++
.../bookkeeper/proto/WriteLacProcessorV3.java | 11 +++
.../proto/TestBookieRequestProcessor.java | 85 ++++++++++++++++++++++
5 files changed, 175 insertions(+)
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/ReadEntryProcessorV3.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/ReadEntryProcessorV3.java
index eef1847..3d2a1ea 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/ReadEntryProcessorV3.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/ReadEntryProcessorV3.java
@@ -22,12 +22,15 @@ import com.google.common.util.concurrent.FutureCallback;
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.SettableFuture;
import com.google.protobuf.ByteString;
+
import io.netty.buffer.ByteBuf;
import io.netty.channel.Channel;
import io.netty.util.ReferenceCountUtil;
+
import java.io.IOException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.TimeUnit;
+
import org.apache.bookkeeper.bookie.Bookie;
import org.apache.bookkeeper.bookie.BookieException;
import org.apache.bookkeeper.proto.BookkeeperProtocol.ReadRequest;
@@ -331,5 +334,14 @@ class ReadEntryProcessorV3 extends PacketProcessorBaseV3 {
statsLogger.registerSuccessfulEvent(startTime.elapsed(TimeUnit.NANOSECONDS), TimeUnit.NANOSECONDS);
}
}
+
+ /**
+ * this toString method filters out masterKey from the output. masterKey
+ * contains the password of the ledger.
+ */
+ @Override
+ public String toString() {
+ return RequestUtils.toSafeString(request);
+ }
}
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/RequestUtils.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/RequestUtils.java
index 87d40c7..fb61165 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/RequestUtils.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/RequestUtils.java
@@ -17,6 +17,8 @@
*/
package org.apache.bookkeeper.proto;
+import com.google.common.base.MoreObjects;
+
/**
* Utilities for requests.
*/
@@ -46,4 +48,59 @@ class RequestUtils {
return request.hasFlag() && request.getFlag() == flag;
}
+ /**
+ * this toSafeString method filters out body and masterKey from the output.
+ * masterKey contains the password of the ledger and body is customer data,
+ * so it is not appropriate to have these in logs or system output.
+ */
+ public static String toSafeString(BookkeeperProtocol.Request request) {
+ MoreObjects.ToStringHelper stringHelper = MoreObjects.toStringHelper(request);
+ BookkeeperProtocol.BKPacketHeader header = request.getHeader();
+ if (request.hasAddRequest()) {
+ BookkeeperProtocol.AddRequest addRequest = request.getAddRequest();
+ includeHeaderFields(stringHelper, header);
+ stringHelper.add("ledgerId", addRequest.getLedgerId());
+ stringHelper.add("entryId", addRequest.getEntryId());
+ if (addRequest.hasFlag()) {
+ stringHelper.add("flag", addRequest.getFlag());
+ }
+ if (addRequest.hasWriteFlags()) {
+ stringHelper.add("writeFlags", addRequest.getWriteFlags());
+ }
+ return stringHelper.toString();
+ } else if (request.hasReadRequest()) {
+ BookkeeperProtocol.ReadRequest readRequest = request.getReadRequest();
+ includeHeaderFields(stringHelper, header);
+ stringHelper.add("ledgerId", readRequest.getLedgerId());
+ stringHelper.add("entryId", readRequest.getEntryId());
+ if (readRequest.hasFlag()) {
+ stringHelper.add("flag", readRequest.getFlag());
+ }
+ if (readRequest.hasPreviousLAC()) {
+ stringHelper.add("previousLAC", readRequest.getPreviousLAC());
+ }
+ if (readRequest.hasTimeOut()) {
+ stringHelper.add("timeOut", readRequest.getTimeOut());
+ }
+ return stringHelper.toString();
+ } else if (request.hasWriteLacRequest()) {
+ BookkeeperProtocol.WriteLacRequest writeLacRequest = request.getWriteLacRequest();
+ includeHeaderFields(stringHelper, header);
+ stringHelper.add("ledgerId", writeLacRequest.getLedgerId());
+ stringHelper.add("lac", writeLacRequest.getLac());
+ return stringHelper.toString();
+ } else {
+ return request.toString();
+ }
+ }
+
+ private static void includeHeaderFields(MoreObjects.ToStringHelper stringHelper,
+ BookkeeperProtocol.BKPacketHeader header) {
+ stringHelper.add("version", header.getVersion());
+ stringHelper.add("operation", header.getOperation());
+ stringHelper.add("txnId", header.getTxnId());
+ if (header.hasPriority()) {
+ stringHelper.add("priority", header.getPriority());
+ }
+ }
}
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/WriteEntryProcessorV3.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/WriteEntryProcessorV3.java
index 0854bf5..c0e097a 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/WriteEntryProcessorV3.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/WriteEntryProcessorV3.java
@@ -170,4 +170,14 @@ class WriteEntryProcessorV3 extends PacketProcessorBaseV3 {
requestProcessor.addRequestStats);
}
}
+
+ /**
+ * this toString method filters out body and masterKey from the output.
+ * masterKey contains the password of the ledger and body is customer data,
+ * so it is not appropriate to have these in logs or system output.
+ */
+ @Override
+ public String toString() {
+ return RequestUtils.toSafeString(request);
+ }
}
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/WriteLacProcessorV3.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/WriteLacProcessorV3.java
index dd4dc3c..c0866f3 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/WriteLacProcessorV3.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/WriteLacProcessorV3.java
@@ -37,6 +37,7 @@ import org.apache.bookkeeper.util.MathUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+
class WriteLacProcessorV3 extends PacketProcessorBaseV3 implements Runnable {
private static final Logger logger = LoggerFactory.getLogger(WriteLacProcessorV3.class);
@@ -112,6 +113,16 @@ class WriteLacProcessorV3 extends PacketProcessorBaseV3 implements Runnable {
sendResponse(writeLacResponse.getStatus(), resp, requestProcessor.writeLacRequestStats);
}
}
+
+ /**
+ * this toString method filters out body and masterKey from the output.
+ * masterKey contains the password of the ledger and body is customer data,
+ * so it is not appropriate to have these in logs or system output.
+ */
+ @Override
+ public String toString() {
+ return RequestUtils.toSafeString(request);
+ }
}
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/proto/TestBookieRequestProcessor.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/proto/TestBookieRequestProcessor.java
index b212e49..354fcaf 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/proto/TestBookieRequestProcessor.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/proto/TestBookieRequestProcessor.java
@@ -32,7 +32,13 @@ import com.google.protobuf.ByteString;
import org.apache.bookkeeper.bookie.Bookie;
import org.apache.bookkeeper.conf.ServerConfiguration;
import org.apache.bookkeeper.proto.BookkeeperProtocol.AddRequest;
+import org.apache.bookkeeper.proto.BookkeeperProtocol.AddRequest.Flag;
+import org.apache.bookkeeper.proto.BookkeeperProtocol.BKPacketHeader;
+import org.apache.bookkeeper.proto.BookkeeperProtocol.OperationType;
+import org.apache.bookkeeper.proto.BookkeeperProtocol.ProtocolVersion;
import org.apache.bookkeeper.proto.BookkeeperProtocol.ReadRequest;
+import org.apache.bookkeeper.proto.BookkeeperProtocol.Request;
+import org.apache.bookkeeper.proto.BookkeeperProtocol.WriteLacRequest;
import org.apache.bookkeeper.stats.NullStatsLogger;
import org.junit.Test;
@@ -114,4 +120,83 @@ public class TestBookieRequestProcessor {
.build();
assertTrue(RequestUtils.hasFlag(add, AddRequest.Flag.RECOVERY_ADD));
}
+
+ @Test
+ public void testToString() {
+ BKPacketHeader.Builder headerBuilder = BKPacketHeader.newBuilder();
+ headerBuilder.setVersion(ProtocolVersion.VERSION_THREE);
+ headerBuilder.setOperation(OperationType.ADD_ENTRY);
+ headerBuilder.setTxnId(5L);
+ BKPacketHeader header = headerBuilder.build();
+
+ AddRequest addRequest = AddRequest.newBuilder().setLedgerId(10).setEntryId(1)
+ .setMasterKey(ByteString.copyFrom("masterKey".getBytes()))
+ .setBody(ByteString.copyFrom("entrydata".getBytes())).build();
+ Request request = Request.newBuilder().setHeader(header).setAddRequest(addRequest).build();
+
+ WriteEntryProcessorV3 writeEntryProcessorV3 = new WriteEntryProcessorV3(request, null, null);
+ String toString = writeEntryProcessorV3.toString();
+ assertFalse("writeEntryProcessorV3's toString should have filtered out body", toString.contains("body"));
+ assertFalse("writeEntryProcessorV3's toString should have filtered out masterKey",
+ toString.contains("masterKey"));
+ assertTrue("writeEntryProcessorV3's toString should contain ledgerId", toString.contains("ledgerId"));
+ assertTrue("writeEntryProcessorV3's toString should contain entryId", toString.contains("entryId"));
+ assertTrue("writeEntryProcessorV3's toString should contain version", toString.contains("version"));
+ assertTrue("writeEntryProcessorV3's toString should contain operation", toString.contains("operation"));
+ assertTrue("writeEntryProcessorV3's toString should contain txnId", toString.contains("txnId"));
+ assertFalse("writeEntryProcessorV3's toString shouldn't contain flag", toString.contains("flag"));
+ assertFalse("writeEntryProcessorV3's toString shouldn't contain writeFlags", toString.contains("writeFlags"));
+
+ addRequest = AddRequest.newBuilder().setLedgerId(10).setEntryId(1)
+ .setMasterKey(ByteString.copyFrom("masterKey".getBytes()))
+ .setBody(ByteString.copyFrom("entrydata".getBytes())).setFlag(Flag.RECOVERY_ADD).setWriteFlags(0)
+ .build();
+ request = Request.newBuilder().setHeader(header).setAddRequest(addRequest).build();
+ writeEntryProcessorV3 = new WriteEntryProcessorV3(request, null, null);
+ toString = writeEntryProcessorV3.toString();
+ assertFalse("writeEntryProcessorV3's toString should have filtered out body", toString.contains("body"));
+ assertFalse("writeEntryProcessorV3's toString should have filtered out masterKey",
+ toString.contains("masterKey"));
+ assertTrue("writeEntryProcessorV3's toString should contain flag", toString.contains("flag"));
+ assertTrue("writeEntryProcessorV3's toString should contain writeFlags", toString.contains("writeFlags"));
+
+ ReadRequest readRequest = ReadRequest.newBuilder().setLedgerId(10).setEntryId(23)
+ .setMasterKey(ByteString.copyFrom("masterKey".getBytes())).build();
+ request = Request.newBuilder().setHeader(header).setReadRequest(readRequest).build();
+ toString = RequestUtils.toSafeString(request);
+ assertFalse("ReadRequest's safeString should have filtered out masterKey", toString.contains("masterKey"));
+ assertTrue("ReadRequest's safeString should contain ledgerId", toString.contains("ledgerId"));
+ assertTrue("ReadRequest's safeString should contain entryId", toString.contains("entryId"));
+ assertTrue("ReadRequest's safeString should contain version", toString.contains("version"));
+ assertTrue("ReadRequest's safeString should contain operation", toString.contains("operation"));
+ assertTrue("ReadRequest's safeString should contain txnId", toString.contains("txnId"));
+ assertFalse("ReadRequest's safeString shouldn't contain flag", toString.contains("flag"));
+ assertFalse("ReadRequest's safeString shouldn't contain previousLAC", toString.contains("previousLAC"));
+ assertFalse("ReadRequest's safeString shouldn't contain timeOut", toString.contains("timeOut"));
+
+ readRequest = ReadRequest.newBuilder().setLedgerId(10).setEntryId(23).setPreviousLAC(2).setTimeOut(100)
+ .setMasterKey(ByteString.copyFrom("masterKey".getBytes())).setFlag(ReadRequest.Flag.ENTRY_PIGGYBACK)
+ .build();
+ request = Request.newBuilder().setHeader(header).setReadRequest(readRequest).build();
+ toString = RequestUtils.toSafeString(request);
+ assertFalse("ReadRequest's safeString should have filtered out masterKey", toString.contains("masterKey"));
+ assertTrue("ReadRequest's safeString shouldn contain flag", toString.contains("flag"));
+ assertTrue("ReadRequest's safeString shouldn contain previousLAC", toString.contains("previousLAC"));
+ assertTrue("ReadRequest's safeString shouldn contain timeOut", toString.contains("timeOut"));
+
+ WriteLacRequest writeLacRequest = WriteLacRequest.newBuilder().setLedgerId(10).setLac(23)
+ .setMasterKey(ByteString.copyFrom("masterKey".getBytes()))
+ .setBody(ByteString.copyFrom("entrydata".getBytes())).build();
+ request = Request.newBuilder().setHeader(header).setWriteLacRequest(writeLacRequest).build();
+ WriteLacProcessorV3 writeLacProcessorV3 = new WriteLacProcessorV3(request, null, null);
+ toString = writeLacProcessorV3.toString();
+ assertFalse("writeLacProcessorV3's toString should have filtered out body", toString.contains("body"));
+ assertFalse("writeLacProcessorV3's toString should have filtered out masterKey",
+ toString.contains("masterKey"));
+ assertTrue("writeLacProcessorV3's toString should contain ledgerId", toString.contains("ledgerId"));
+ assertTrue("writeLacProcessorV3's toString should contain lac", toString.contains("lac"));
+ assertTrue("writeLacProcessorV3's toString should contain version", toString.contains("version"));
+ assertTrue("writeLacProcessorV3's toString should contain operation", toString.contains("operation"));
+ assertTrue("writeLacProcessorV3's toString should contain txnId", toString.contains("txnId"));
+ }
}
--
To stop receiving notification emails like this one, please contact
sijie@apache.org.