Get incidentd cts working again.
- The buffer size increased, and the CTS test that checked that
was triggering.
- Privacy filtering wasn't working for the stack trace sections
- The incident command was not handling the default arguments correctly
- The throttler was throttling streaming reports, which made the
test flaky.
Bug: 126253679
Test: atest CtsIncidentHostTestCases
Change-Id: I342cd7d0421ea8c22b7796fc99e779f21855af73
diff --git a/cmds/incidentd/src/IncidentService.cpp b/cmds/incidentd/src/IncidentService.cpp
index b4021d1b..71b89a2 100644
--- a/cmds/incidentd/src/IncidentService.cpp
+++ b/cmds/incidentd/src/IncidentService.cpp
@@ -174,12 +174,11 @@
}
void ReportHandler::take_report() {
- // Cycle the batch
+ // Cycle the batch and throttle.
sp<ReportBatch> batch;
{
unique_lock<mutex> lock(mLock);
- batch = mBatch;
- mBatch = new ReportBatch();
+ batch = mThrottler->filterBatch(mBatch);
}
if (batch->empty()) {
@@ -189,13 +188,6 @@
sp<Reporter> reporter = new Reporter(mWorkDirectory, batch);
- // TODO: Do we really want to clear the reports if we throttle? Should we only throttle
- // requests going to dropbox? How do we reconcile throttling with testing?
- if (mThrottler->shouldThrottle()) {
- ALOGW("RunReport got throttled.");
- return;
- }
-
// Take the report, which might take a while. More requests might queue
// up while we're doing this, and we'll handle them in their next batch.
// TODO: We should further rate-limit the reports to no more than N per time-period.
@@ -203,7 +195,13 @@
size_t reportByteSize = 0;
reporter->runReport(&reportByteSize);
- mThrottler->addReportSize(reportByteSize);
+ // Tell the throttler how big it was, for the next throttling.
+ // TODO: This still isn't ideal. The throttler really should just track the
+ // persisted reqeusts, but changing Reporter::runReport() to track that individually
+ // will be a big change.
+ if (batch->hasPersistedReports()) {
+ mThrottler->addReportSize(reportByteSize);
+ }
// Kick off the next steps, one of which is to send any new or otherwise remaining
// approvals, and one of which is to send any new or remaining broadcasts.
@@ -247,11 +245,11 @@
IncidentService::~IncidentService() {}
Status IncidentService::reportIncident(const IncidentReportArgs& args) {
- // TODO: Validate that the privacy policy is one of the real ones.
- // If it isn't, clamp it to the next more restrictive real one.
+ IncidentReportArgs argsCopy(args);
- // TODO: This function should reject the LOCAL privacy policy.
- // Those have to stream.
+ // Validate that the privacy policy is one of the real ones.
+ // If it isn't, clamp it to the next more restrictive real one.
+ argsCopy.setPrivacyPolicy(cleanup_privacy_policy(args.getPrivacyPolicy()));
// TODO: Check that the broadcast recevier has the proper permissions
// TODO: Maybe we should consider relaxing the permissions if it's going to
@@ -261,8 +259,15 @@
return status;
}
+ // If they asked for the LOCAL privacy policy, give them EXPLICT. LOCAL has to
+ // be streamed. (This only applies to shell/root, because everyone else would have
+ // been rejected by checkIncidentPermissions()).
+ if (argsCopy.getPrivacyPolicy() < PRIVACY_POLICY_EXPLICIT) {
+ ALOGI("Demoting privacy policy to EXPLICT for persisted report.");
+ argsCopy.setPrivacyPolicy(PRIVACY_POLICY_EXPLICIT);
+ }
+
// If they didn't specify a component, use dropbox.
- IncidentReportArgs argsCopy(args);
if (argsCopy.receiverPkg().length() == 0 && argsCopy.receiverCls().length() == 0) {
argsCopy.setReceiverPkg(DROPBOX_SENTINEL.getPackageName());
argsCopy.setReceiverCls(DROPBOX_SENTINEL.getClassName());
@@ -276,22 +281,21 @@
Status IncidentService::reportIncidentToStream(const IncidentReportArgs& args,
const sp<IIncidentReportStatusListener>& listener,
const unique_fd& stream) {
- // TODO: Validate that the privacy policy is one of the real ones.
- // If it isn't, clamp it to the next more restrictive real one.
-
- // TODO: Only shell should be able to do a LOCAL privacy policy report.
+ IncidentReportArgs argsCopy(args);
// Streaming reports can not also be broadcast.
- IncidentReportArgs argsCopy(args);
argsCopy.setReceiverPkg("");
argsCopy.setReceiverCls("");
+ // Validate that the privacy policy is one of the real ones.
+ // If it isn't, clamp it to the next more restrictive real one.
+ argsCopy.setPrivacyPolicy(cleanup_privacy_policy(args.getPrivacyPolicy()));
+
Status status = checkIncidentPermissions(argsCopy);
if (!status.isOk()) {
return status;
}
-
// The ReportRequest takes ownership of the fd, so we need to dup it.
int fd = dup(stream.get());
if (fd < 0) {