OSDN Git Service

Use ProtoOutputStream to strip Pii sensetive fields in incidentd
authorYi Jin <jinyithu@google.com>
Thu, 12 Oct 2017 01:20:24 +0000 (18:20 -0700)
committerYi Jin <jinyithu@google.com>
Fri, 13 Oct 2017 22:44:26 +0000 (15:44 -0700)
The in-place algorithm in ProtoOutputStream will have at most 2 copies
of possible nested message values incidentd is getting which makes it
more ram-efficient, and minimize allocations.

Bug: 65641021
Test: the units pass
Change-Id: Ic3fe99b7e27895bd613a129ba0f12ccfca4af317

cmds/incidentd/src/Privacy.cpp
cmds/incidentd/src/Privacy.h
cmds/incidentd/src/PrivacyBuffer.cpp
cmds/incidentd/src/PrivacyBuffer.h
libs/protoutil/include/android/util/ProtoOutputStream.h
libs/protoutil/src/ProtoOutputStream.cpp

index e7969e7..140b12c 100644 (file)
@@ -30,6 +30,9 @@ const uint8_t TYPE_MESSAGE = 11;
 bool
 Privacy::IsMessageType() const { return type == TYPE_MESSAGE; }
 
+uint64_t
+Privacy::EncodedFieldId() const { return (uint64_t)type << 32 | field_id; }
+
 bool
 Privacy::IsStringType() const { return type == TYPE_STRING; }
 
index 7f1977e..f514f19 100644 (file)
@@ -40,6 +40,8 @@ struct Privacy {
     bool IsMessageType() const;
     bool IsStringType() const;
     bool HasChildren() const;
+    uint64_t EncodedFieldId() const;
+
     const Privacy* lookup(uint32_t fieldId) const;
 };
 
index 37f6ed7..a095afc 100644 (file)
@@ -28,37 +28,41 @@ using namespace android::util;
  * Write the field to buf based on the wire type, iterator will point to next field.
  * If skip is set to true, no data will be written to buf. Return number of bytes written.
  */
-static size_t
-write_field_or_skip(EncodedBuffer::iterator* iter, EncodedBuffer* buf, uint8_t wireType, bool skip)
+void
+PrivacyBuffer::writeFieldOrSkip(uint32_t fieldTag, bool skip)
 {
-    EncodedBuffer::Pointer snapshot = iter->rp()->copy();
+    uint8_t wireType = read_wire_type(fieldTag);
     size_t bytesToWrite = 0;
-    uint64_t varint = 0;
+    uint32_t varint = 0;
+
     switch (wireType) {
         case WIRE_TYPE_VARINT:
-            varint = iter->readRawVarint();
-            if(!skip) return buf->writeRawVarint64(varint);
-            break;
+            varint = mData.readRawVarint();
+            if (!skip) {
+                mProto.writeRawVarint(fieldTag);
+                mProto.writeRawVarint(varint);
+            }
+            return;
         case WIRE_TYPE_FIXED64:
+            if (!skip) mProto.writeRawVarint(fieldTag);
             bytesToWrite = 8;
             break;
         case WIRE_TYPE_LENGTH_DELIMITED:
-            bytesToWrite = iter->readRawVarint();
-            if(!skip) buf->writeRawVarint32(bytesToWrite);
+            bytesToWrite = mData.readRawVarint();
+            if(!skip) mProto.writeLengthDelimitedHeader(read_field_id(fieldTag), bytesToWrite);
             break;
         case WIRE_TYPE_FIXED32:
+            if (!skip) mProto.writeRawVarint(fieldTag);
             bytesToWrite = 4;
             break;
     }
     if (skip) {
-        iter->rp()->move(bytesToWrite);
+        mData.rp()->move(bytesToWrite);
     } else {
         for (size_t i=0; i<bytesToWrite; i++) {
-            *buf->writeBuffer() = iter->next();
-            buf->wp()->move();
+            mProto.writeRawByte(mData.next());
         }
     }
-    return skip ? 0 : iter->rp()->pos() - snapshot.pos();
 }
 
 /**
@@ -68,46 +72,27 @@ write_field_or_skip(EncodedBuffer::iterator* iter, EncodedBuffer* buf, uint8_t w
  * The iterator must point to the head of a protobuf formatted field for successful operation.
  * After exit with NO_ERROR, iterator points to the next protobuf field's head.
  */
-static status_t
-stripField(EncodedBuffer::iterator* iter, EncodedBuffer* buf, const Privacy* parentPolicy, const PrivacySpec& spec)
+status_t
+PrivacyBuffer::stripField(const Privacy* parentPolicy, const PrivacySpec& spec)
 {
-    if (!iter->hasNext() || parentPolicy == NULL) return BAD_VALUE;
-    uint32_t varint = iter->readRawVarint();
-    uint8_t wireType = read_wire_type(varint);
-    uint32_t fieldId = read_field_id(varint);
-    const Privacy* policy = parentPolicy->lookup(fieldId);
+    if (!mData.hasNext() || parentPolicy == NULL) return BAD_VALUE;
+    uint32_t fieldTag = mData.readRawVarint();
+    const Privacy* policy = parentPolicy->lookup(read_field_id(fieldTag));
+
     if (policy == NULL || !policy->IsMessageType() || !policy->HasChildren()) {
         bool skip = !spec.CheckPremission(policy);
-        size_t amt = buf->size();
-        if (!skip) amt += buf->writeHeader(fieldId, wireType);
-        amt += write_field_or_skip(iter, buf, wireType, skip); // point to head of next field
-        return buf->size() != amt ? BAD_VALUE : NO_ERROR;
+        // iterator will point to head of next field
+        writeFieldOrSkip(fieldTag, skip);
+        return NO_ERROR;
     }
     // current field is message type and its sub-fields have extra privacy policies
-    deque<EncodedBuffer*> q;
-    uint32_t msgSize = iter->readRawVarint();
-    size_t finalSize = 0;
-    EncodedBuffer::Pointer start = iter->rp()->copy();
-    while (iter->rp()->pos() - start.pos() != msgSize) {
-        EncodedBuffer* v = new EncodedBuffer();
-        status_t err = stripField(iter, v, policy, spec);
+    uint32_t msgSize = mData.readRawVarint();
+    EncodedBuffer::Pointer start = mData.rp()->copy();
+    while (mData.rp()->pos() - start.pos() != msgSize) {
+        long long token = mProto.start(policy->EncodedFieldId());
+        status_t err = stripField(policy, spec);
         if (err != NO_ERROR) return err;
-        if (v->size() == 0) continue;
-        q.push_back(v);
-        finalSize += v->size();
-    }
-
-    buf->writeHeader(fieldId, wireType);
-    buf->writeRawVarint32(finalSize);
-    while (!q.empty()) {
-        EncodedBuffer* subField = q.front();
-        EncodedBuffer::iterator it = subField->begin();
-        while (it.hasNext()) {
-            *buf->writeBuffer() = it.next();
-            buf->wp()->move();
-        }
-        q.pop_front();
-        delete subField;
+        mProto.end(token);
     }
     return NO_ERROR;
 }
@@ -116,7 +101,7 @@ stripField(EncodedBuffer::iterator* iter, EncodedBuffer* buf, const Privacy* par
 PrivacyBuffer::PrivacyBuffer(const Privacy* policy, EncodedBuffer::iterator& data)
         :mPolicy(policy),
          mData(data),
-         mBuffer(0),
+         mProto(),
          mSize(0)
 {
 }
@@ -134,11 +119,11 @@ PrivacyBuffer::strip(const PrivacySpec& spec)
         return NO_ERROR;
     }
     while (mData.hasNext()) {
-        status_t err = stripField(&mData, &mBuffer, mPolicy, spec);
+        status_t err = stripField(mPolicy, spec);
         if (err != NO_ERROR) return err;
     }
     if (mData.bytesRead() != mData.size()) return BAD_VALUE;
-    mSize = mBuffer.size();
+    mSize = mProto.size();
     mData.rp()->rewind(); // rewind the read pointer back to beginning after the strip.
     return NO_ERROR;
 }
@@ -147,7 +132,7 @@ void
 PrivacyBuffer::clear()
 {
     mSize = 0;
-    mBuffer.wp()->rewind();
+    mProto = ProtoOutputStream();
 }
 
 size_t
@@ -157,7 +142,7 @@ status_t
 PrivacyBuffer::flush(int fd)
 {
     status_t err = NO_ERROR;
-    EncodedBuffer::iterator iter = size() == mData.size() ? mData : mBuffer.begin();
+    EncodedBuffer::iterator iter = size() == mData.size() ? mData : mProto.data();
     while (iter.readBuffer() != NULL) {
         err = write_all(fd, iter.readBuffer(), iter.currentToRead());
         iter.rp()->move(iter.currentToRead());
index 720b38e..c9ca9a7 100644 (file)
@@ -20,6 +20,7 @@
 #include "Privacy.h"
 
 #include <android/util/EncodedBuffer.h>
+#include <android/util/ProtoOutputStream.h>
 #include <stdint.h>
 #include <utils/Errors.h>
 
@@ -60,8 +61,11 @@ private:
     const Privacy* mPolicy;
     EncodedBuffer::iterator& mData;
 
-    EncodedBuffer mBuffer;
+    ProtoOutputStream mProto;
     size_t mSize;
+
+    status_t stripField(const Privacy* parentPolicy, const PrivacySpec& spec);
+    void writeFieldOrSkip(uint32_t fieldTag, bool skip);
 };
 
 #endif // PRIVACY_BUFFER_H
\ No newline at end of file
index 49ec169..0f1cced 100644 (file)
@@ -37,7 +37,7 @@ namespace util {
 class ProtoOutputStream
 {
 public:
-    ProtoOutputStream(int fd);
+    ProtoOutputStream();
     ~ProtoOutputStream();
 
     /**
@@ -60,13 +60,19 @@ public:
     void end(long long token);
 
     /**
-     * Flushes the protobuf data out.
+     * Flushes the protobuf data out to given fd.
      */
-    bool flush();
+    size_t size();
+    EncodedBuffer::iterator data();
+    bool flush(int fd);
+
+    // Please don't use the following functions to dump protos unless you are sure about it.
+    void writeRawVarint(uint64_t varint);
+    void writeLengthDelimitedHeader(uint32_t id, size_t size);
+    void writeRawByte(uint8_t byte);
 
 private:
     EncodedBuffer mBuffer;
-    int mFd;
     size_t mCopyBegin;
     bool mCompact;
     int mDepth;
index e9ca0dc..15144ac 100644 (file)
@@ -70,9 +70,8 @@ const uint64_t FIELD_COUNT_SINGLE = 1ULL << FIELD_COUNT_SHIFT;
 const uint64_t FIELD_COUNT_REPEATED = 2ULL << FIELD_COUNT_SHIFT;
 const uint64_t FIELD_COUNT_PACKED = 4ULL << FIELD_COUNT_SHIFT;
 
-ProtoOutputStream::ProtoOutputStream(int fd)
+ProtoOutputStream::ProtoOutputStream()
         :mBuffer(),
-         mFd(fd),
          mCopyBegin(0),
          mCompact(false),
          mDepth(0),
@@ -483,6 +482,13 @@ ProtoOutputStream::compactSize(size_t rawSize)
     return true;
 }
 
+size_t
+ProtoOutputStream::size()
+{
+    compact();
+    return mBuffer.size();
+}
+
 static bool write_all(int fd, uint8_t const* buf, size_t size)
 {
     while (size > 0) {
@@ -497,19 +503,47 @@ static bool write_all(int fd, uint8_t const* buf, size_t size)
 }
 
 bool
-ProtoOutputStream::flush()
+ProtoOutputStream::flush(int fd)
 {
-    if (mFd < 0) return false;
+    if (fd < 0) return false;
     if (!compact()) return false;
 
     EncodedBuffer::iterator it = mBuffer.begin();
     while (it.readBuffer() != NULL) {
-        if (!write_all(mFd, it.readBuffer(), it.currentToRead())) return false;
+        if (!write_all(fd, it.readBuffer(), it.currentToRead())) return false;
         it.rp()->move(it.currentToRead());
     }
     return true;
 }
 
+EncodedBuffer::iterator
+ProtoOutputStream::data()
+{
+    compact();
+    return mBuffer.begin();
+}
+
+void
+ProtoOutputStream::writeRawVarint(uint64_t varint)
+{
+    mBuffer.writeRawVarint64(varint);
+}
+
+void
+ProtoOutputStream::writeLengthDelimitedHeader(uint32_t id, size_t size)
+{
+    mBuffer.writeHeader(id, WIRE_TYPE_LENGTH_DELIMITED);
+    // reserves 64 bits for length delimited fields, if first field is negative, compact it.
+    mBuffer.writeRawFixed32(size);
+    mBuffer.writeRawFixed32(size);
+}
+
+void
+ProtoOutputStream::writeRawByte(uint8_t byte)
+{
+    mBuffer.writeRawByte(byte);
+}
+
 
 // =========================================================================
 // Private functions
@@ -639,9 +673,7 @@ inline void
 ProtoOutputStream::writeUtf8StringImpl(uint32_t id, const char* val, size_t size)
 {
     if (val == NULL || size == 0) return;
-    mBuffer.writeHeader(id, WIRE_TYPE_LENGTH_DELIMITED);
-    mBuffer.writeRawFixed32(size);
-    mBuffer.writeRawFixed32(size);
+    writeLengthDelimitedHeader(id, size);
     for (size_t i=0; i<size; i++) {
         mBuffer.writeRawByte((uint8_t)val[i]);
     }