You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2018/04/05 07:01:34 UTC
[GitHub] sijie closed pull request #1311: Issue #1310: Filter out body and masterKey from ProcessorV3s
sijie closed pull request #1311: Issue #1310: Filter out body and masterKey from ProcessorV3s
URL: https://github.com/apache/bookkeeper/pull/1311
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
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 eef18476d..3d2a1eafe 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.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 @@ protected void registerEvent(boolean failed, OpStatsLogger statsLogger, Stopwatc
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 87d40c7b6..fb6116529 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 @@ static boolean hasFlag(BookkeeperProtocol.AddRequest request, BookkeeperProtocol
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 0854bf538..c0e097a66 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 @@ public void safeRun() {
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 dd4dc3c1c..c0866f391 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.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 @@ public void safeRun() {
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 b212e49e4..354fcaf4b 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 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 void testFlagsV3() {
.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"));
+ }
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services