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 <[email protected]>
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 <[email protected]>
Reviewers: Enrico Olivelli <[email protected]>, Sijie Guo
<[email protected]>, 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
[email protected].
