commit 69fa769cde3ac606b4052c493b82f86b35b1211a
author Aaron Vaage <vaage@google.com> Tue Jun 11 12:37:19 2024 -0700
committer Aaron Vaage <vaage@google.com> Tue Jun 11 12:37:19 2024 -0700
tree b43b17ee34fe765a6b024bef0c3e3a85a1bd846f
parent 9132f3bfb0476bc0f2df5b0645260be6d4e18adb

Trace Redaction - Loosen final flush restriction

Final flush can be: success, unknown, fail.

Before the check was: fail if final_flush != success. This caused
unwanted failures, so this changes the check to be: fail if
final_flush = fail.

Change-Id: I448e138b677ee2c1a9f3945b1cb4c18bb7884a44

src/trace_redaction/verify_integrity.cc

diff --git a/src/trace_redaction/verify_integrity.cc b/src/trace_redaction/verify_integrity.cc
index 19619de..0927432 100644
--- a/src/trace_redaction/verify_integrity.cc
+++ b/src/trace_redaction/verify_integrity.cc
@@ -133,10 +133,11 @@
   }
 
   if (trace_stats.has_final_flush_outcome() &&
-      trace_stats.final_flush_outcome() !=
-          protos::pbzero::TraceStats::FINAL_FLUSH_SUCCEEDED) {
+      trace_stats.final_flush_outcome() ==
+          protos::pbzero::TraceStats::FINAL_FLUSH_FAILED) {
     return base::ErrStatus(
-        "VerifyIntegrity: detected TraceStats final flush failed.");
+        "VerifyIntegrity: TraceStats final_flush_outcome is "
+        "FINAL_FLUSH_FAILED.");
   }
 
   for (auto it = trace_stats.buffer_stats(); it; ++it) {

src/trace_redaction/verify_integrity_unittest.cc

diff --git a/src/trace_redaction/verify_integrity_unittest.cc b/src/trace_redaction/verify_integrity_unittest.cc
index 1c372eb..9ca8bfe 100644
--- a/src/trace_redaction/verify_integrity_unittest.cc
+++ b/src/trace_redaction/verify_integrity_unittest.cc
@@ -232,8 +232,38 @@
   ASSERT_OK(Verify(packet));
 }
 
-// TODO(vaage): Write tests for final_flush_outcome. Type-compile errors were
-// getting in the way of moving forward.
+TEST_F(VerifyIntegrityUnitTest, ValidPacketFinalFlushSucceeded) {
+  protos::gen::TracePacket packet;
+
+  packet.set_trusted_uid(kValid);
+
+  packet.mutable_trace_stats()->set_final_flush_outcome(
+      protos::gen::TraceStats::FINAL_FLUSH_SUCCEEDED);
+
+  ASSERT_OK(Verify(packet));
+}
+
+TEST_F(VerifyIntegrityUnitTest, ValidPacketFinalFlushUnspecified) {
+  protos::gen::TracePacket packet;
+
+  packet.set_trusted_uid(kValid);
+
+  packet.mutable_trace_stats()->set_final_flush_outcome(
+      protos::gen::TraceStats::FINAL_FLUSH_UNSPECIFIED);
+
+  ASSERT_OK(Verify(packet));
+}
+
+TEST_F(VerifyIntegrityUnitTest, InvalidPacketFinalFlushFailed) {
+  protos::gen::TracePacket packet;
+
+  packet.set_trusted_uid(kValid);
+
+  packet.mutable_trace_stats()->set_final_flush_outcome(
+      protos::gen::TraceStats::FINAL_FLUSH_FAILED);
+
+  ASSERT_FALSE(Verify(packet).ok());
+}
 
 TEST_F(VerifyIntegrityUnitTest, InvalidPacketBufferStatsPatchesFailed) {
   protos::gen::TracePacket packet;