Implement Pii Stripper Part 3
The incident request args sets privacy spec. Strip action is optimized
to run once for each type of spec and ready for flush multiple times.
Incident command is updated to take -p option to specify privacy spec.
Bug: 64687253
Test: unit tests written, manually run incident command to test as well
Change-Id: I6753df117f76dc1a5f4d2152baa3fbbf56b490e4
diff --git a/cmds/incidentd/src/EncodedBuffer.cpp b/cmds/incidentd/src/EncodedBuffer.cpp
index d1872f9..3d20548 100644
--- a/cmds/incidentd/src/EncodedBuffer.cpp
+++ b/cmds/incidentd/src/EncodedBuffer.cpp
@@ -70,7 +70,6 @@
if (skip) {
iterator += bytesToWrite;
} else {
- buf.reserve(bytesToWrite);
for (size_t i=0; i<bytesToWrite; i++) {
buf.push_back(*iterator);
iterator++;
@@ -192,4 +191,5 @@
if (err != NO_ERROR) return err;
}
return NO_ERROR;
-}
\ No newline at end of file
+}
+
diff --git a/cmds/incidentd/src/FdBuffer.cpp b/cmds/incidentd/src/FdBuffer.cpp
index 23a611a..bb399b5 100644
--- a/cmds/incidentd/src/FdBuffer.cpp
+++ b/cmds/incidentd/src/FdBuffer.cpp
@@ -259,6 +259,12 @@
}
FdBuffer::iterator
+FdBuffer::begin() const
+{
+ return iterator(*this, 0, 0);
+}
+
+FdBuffer::iterator
FdBuffer::end() const
{
if (mBuffers.empty() || mCurrentWritten < 0) return begin();
@@ -268,6 +274,17 @@
return FdBuffer::iterator(*this, mBuffers.size() - 1, mCurrentWritten);
}
+// ===============================================================================
+FdBuffer::iterator::iterator(const FdBuffer& buffer, ssize_t index, ssize_t offset)
+ : mFdBuffer(buffer),
+ mIndex(index),
+ mOffset(offset)
+{
+}
+
+FdBuffer::iterator&
+FdBuffer::iterator::operator=(iterator& other) const { return other; }
+
FdBuffer::iterator&
FdBuffer::iterator::operator+(size_t offset)
{
@@ -280,8 +297,50 @@
return *this;
}
+FdBuffer::iterator&
+FdBuffer::iterator::operator+=(size_t offset) { return *this + offset; }
+
+FdBuffer::iterator&
+FdBuffer::iterator::operator++() { return *this + 1; }
+
+FdBuffer::iterator
+FdBuffer::iterator::operator++(int) { return *this + 1; }
+
+bool
+FdBuffer::iterator::operator==(iterator other) const
+{
+ return mIndex == other.mIndex && mOffset == other.mOffset;
+}
+
+bool
+FdBuffer::iterator::operator!=(iterator other) const { return !(*this == other); }
+
+int
+FdBuffer::iterator::operator-(iterator other) const
+{
+ return (int)bytesRead() - (int)other.bytesRead();
+}
+
+FdBuffer::iterator::reference
+FdBuffer::iterator::operator*() const
+{
+ return mFdBuffer.mBuffers[mIndex][mOffset];
+}
+
+FdBuffer::iterator
+FdBuffer::iterator::snapshot() const
+{
+ return FdBuffer::iterator(mFdBuffer, mIndex, mOffset);
+}
+
size_t
FdBuffer::iterator::bytesRead() const
{
return mIndex * BUFFER_SIZE + mOffset;
}
+
+bool
+FdBuffer::iterator::outOfBound() const
+{
+ return bytesRead() > mFdBuffer.size();
+}
diff --git a/cmds/incidentd/src/FdBuffer.h b/cmds/incidentd/src/FdBuffer.h
index 4c4823e..dfe39c6 100644
--- a/cmds/incidentd/src/FdBuffer.h
+++ b/cmds/incidentd/src/FdBuffer.h
@@ -87,32 +87,29 @@
friend class iterator;
class iterator : public std::iterator<std::random_access_iterator_tag, uint8_t> {
public:
- iterator(const FdBuffer& buffer, ssize_t index, ssize_t offset)
- : mFdBuffer(buffer), mIndex(index), mOffset(offset) {}
- iterator& operator=(iterator& other) const { return other; }
- iterator& operator+(size_t offset); // this is implemented in .cpp
- iterator& operator+=(size_t offset) { return *this + offset; }
- iterator& operator++() { return *this + 1; }
- iterator operator++(int) { return *this + 1; }
- bool operator==(iterator other) const {
- return mIndex == other.mIndex && mOffset == other.mOffset;
- }
- bool operator!=(iterator other) const { return !(*this == other); }
- int operator-(iterator other) const { return (int)bytesRead() - (int)other.bytesRead(); }
- reference operator*() const { return mFdBuffer.mBuffers[mIndex][mOffset]; }
+ iterator(const FdBuffer& buffer, ssize_t index, ssize_t offset);
+ iterator& operator=(iterator& other) const;
+ iterator& operator+(size_t offset);
+ iterator& operator+=(size_t offset);
+ iterator& operator++();
+ iterator operator++(int);
+ bool operator==(iterator other) const;
+ bool operator!=(iterator other) const;
+ int operator-(iterator other) const;
+ reference operator*() const;
// return the snapshot of the current iterator
- iterator snapshot() const { return iterator(mFdBuffer, mIndex, mOffset); }
+ iterator snapshot() const;
// how many bytes are read
size_t bytesRead() const;
// random access could make the iterator out of bound
- bool outOfBound() const { return bytesRead() > mFdBuffer.size(); }
+ bool outOfBound() const;
private:
const FdBuffer& mFdBuffer;
size_t mIndex;
size_t mOffset;
};
- iterator begin() const { return iterator(*this, 0, 0); }
+ iterator begin() const;
iterator end() const;
private:
diff --git a/cmds/incidentd/src/Privacy.cpp b/cmds/incidentd/src/Privacy.cpp
index a790efa..dbab548 100644
--- a/cmds/incidentd/src/Privacy.cpp
+++ b/cmds/incidentd/src/Privacy.cpp
@@ -88,6 +88,12 @@
}
bool
+PrivacySpec::operator<(const PrivacySpec& other) const
+{
+ return dest < other.dest;
+}
+
+bool
PrivacySpec::CheckPremission(const Privacy* privacy) const
{
uint8_t policy = privacy == NULL ? DEST_DEFAULT_VALUE : privacy->dest;
@@ -97,4 +103,9 @@
bool
PrivacySpec::RequireAll() const { return dest == DEST_LOCAL; }
+PrivacySpec new_spec_from_args(int dest) {
+ if (dest < 0) return PrivacySpec();
+ return PrivacySpec(dest);
+}
+
PrivacySpec get_default_dropbox_spec() { return PrivacySpec(DEST_AUTOMATIC); }
\ No newline at end of file
diff --git a/cmds/incidentd/src/Privacy.h b/cmds/incidentd/src/Privacy.h
index 53b0325..c56ba9b8 100644
--- a/cmds/incidentd/src/Privacy.h
+++ b/cmds/incidentd/src/Privacy.h
@@ -58,10 +58,13 @@
PrivacySpec() : dest(DEST_DEFAULT_VALUE) {}
PrivacySpec(uint8_t dest) : dest(dest) {}
+ bool operator<(const PrivacySpec& other) const;
+
bool CheckPremission(const Privacy* privacy) const;
bool RequireAll() const;
};
+PrivacySpec new_spec_from_args(int dest);
PrivacySpec get_default_dropbox_spec();
#endif // PRIVACY_H
diff --git a/cmds/incidentd/src/Section.cpp b/cmds/incidentd/src/Section.cpp
index a04804c..08959263 100644
--- a/cmds/incidentd/src/Section.cpp
+++ b/cmds/incidentd/src/Section.cpp
@@ -27,6 +27,7 @@
#include <private/android_filesystem_config.h>
#include <binder/IServiceManager.h>
+#include <map>
#include <mutex>
#include <wait.h>
#include <unistd.h>
@@ -109,20 +110,16 @@
}
static status_t
-WriteToRequest(const int id, const int fd, EncodedBuffer& buffer, const PrivacySpec& spec)
+WriteToRequest(const int id, const int fd, EncodedBuffer& buffer)
{
- if (fd < 0) return EBADF;
+ if (buffer.size() == 0) return NO_ERROR;
+
status_t err = NO_ERROR;
uint8_t buf[20];
-
- buffer.clear(); // clear before strip
- err = buffer.strip(spec); // TODO: don't have to strip again if the spec is the same.
- if (err != NO_ERROR || buffer.size() == 0) return err;
uint8_t *p = write_length_delimited_tag_header(buf, id, buffer.size());
err = write_all(fd, buf, p-buf);
if (err == NO_ERROR) {
err = buffer.flush(fd);
- ALOGD("Section %d flushed %zu bytes to fd %d with spec %d", id, buffer.size(), fd, spec.dest);
}
return err;
}
@@ -130,25 +127,46 @@
static status_t
WriteToReportRequests(const int id, const FdBuffer& buffer, ReportRequestSet* requests)
{
- status_t err = EBADF;
+ status_t err = -EBADF;
EncodedBuffer encodedBuffer(buffer, GetPrivacyOfSection(id));
int writeable = 0;
- // The streaming ones
+ // The streaming ones, group requests by spec in order to save unnecessary strip operations
+ map<PrivacySpec, vector<sp<ReportRequest>>> requestsBySpec;
for (ReportRequestSet::iterator it = requests->begin(); it != requests->end(); it++) {
sp<ReportRequest> request = *it;
- PrivacySpec spec; // TODO: this should be derived from each request.
- err = WriteToRequest(id, request->fd, encodedBuffer, spec);
- if (err != NO_ERROR) {
- request->err = err;
- } else {
- writeable++;
+ if (!request->args.containsSection(id) || request->fd < 0 || request->err != NO_ERROR) {
+ continue; // skip invalid request
}
+ PrivacySpec spec = new_spec_from_args(request->args.dest());
+ requestsBySpec[spec].push_back(request);
+ }
+
+ for (map<PrivacySpec, vector<sp<ReportRequest>>>::iterator mit = requestsBySpec.begin(); mit != requestsBySpec.end(); mit++) {
+ PrivacySpec spec = mit->first;
+ err = encodedBuffer.strip(spec);
+ if (err != NO_ERROR) return err; // it means the encodedBuffer data is corrupted.
+ if (encodedBuffer.size() == 0) continue;
+
+ for (vector<sp<ReportRequest>>::iterator it = mit->second.begin(); it != mit->second.end(); it++) {
+ sp<ReportRequest> request = *it;
+ err = WriteToRequest(id, request->fd, encodedBuffer);
+ if (err != NO_ERROR) {
+ request->err = err;
+ } else {
+ writeable++;
+ }
+ ALOGD("Section %d flushed %zu bytes to fd %d with spec %d", id, encodedBuffer.size(), request->fd, spec.dest);
+ }
+ encodedBuffer.clear();
}
// The dropbox file
if (requests->mainFd() >= 0) {
- err = WriteToRequest(id, requests->mainFd(), encodedBuffer, get_default_dropbox_spec());
+ err = encodedBuffer.strip(get_default_dropbox_spec());
+ if (err != NO_ERROR) return err; // the buffer data is corrupted.
+
+ err = WriteToRequest(id, requests->mainFd(), encodedBuffer);
if (err != NO_ERROR) {
requests->setMainFd(-1);
} else {