Empty output if compact fails.

There must be equal number of start/end calls with right token given in
order, if used in wrong order, ProtoOutputStream will have empty output.

Also refactor Android.bp, so the gtest is standalone unit test, it used
to require compile/push libprotoutil.so to device.

Bug: 77342154
Test: atest libprotoutil_test
Change-Id: I0011bbab34c04cb38164d2ed21cd818d52a2ecf9

4 files changed, 57 insertions, 13 deletions

diff --git a/libs/protoutil/Android.bp b/libs/protoutil/Android.bp
index 14b2e4131acb..44bc97adb946 100644
--- a/libs/protoutil/Android.bp
+++ b/libs/protoutil/Android.bp
@@ -12,9 +12,8 @@
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
-
-cc_library {
-    name: "libprotoutil",
+cc_defaults {
+    name: "libprotoutil_defaults",
 
     cflags: [
         "-Wall",
@@ -30,8 +29,6 @@ cc_library {
         "src/protobuf.cpp",
     ],
 
-    export_include_dirs: ["include"],
-
     shared_libs: [
         "libbase",
         "libcutils",
@@ -39,15 +36,24 @@ cc_library {
     ],
 }
 
+cc_library {
+    name: "libprotoutil",
+
+    defaults: ["libprotoutil_defaults"],
+
+    export_include_dirs: ["include"],
+}
+
 cc_test {
     name: "libprotoutil_test",
 
+    defaults: ["libprotoutil_defaults"],
+
+    local_include_dirs: ["include"],
+
     srcs: ["tests/*"],
 
     shared_libs: [
-        "libbase",
-        "libcutils",
-        "libprotoutil",
         "libprotobuf-cpp-full",
     ],
 
diff --git a/libs/protoutil/include/android/util/ProtoOutputStream.h b/libs/protoutil/include/android/util/ProtoOutputStream.h
index ad90893f8cd6..ad765592eb55 100644
--- a/libs/protoutil/include/android/util/ProtoOutputStream.h
+++ b/libs/protoutil/include/android/util/ProtoOutputStream.h
@@ -105,7 +105,7 @@ public:
     /**
      * Starts a sub-message write session.
      * Returns a token of this write session.
-     * Must call end(token) when finish write this sub-message.
+     * Must call end(token) exactly once when finish write this sub-message.
      */
     uint64_t start(uint64_t fieldId);
     void end(uint64_t token);
diff --git a/libs/protoutil/src/ProtoOutputStream.cpp b/libs/protoutil/src/ProtoOutputStream.cpp
index 0d429e0ba6c8..ff3fad6055e1 100644
--- a/libs/protoutil/src/ProtoOutputStream.cpp
+++ b/libs/protoutil/src/ProtoOutputStream.cpp
@@ -244,12 +244,14 @@ ProtoOutputStream::end(uint64_t token)
 {
     if (token != mExpectedObjectToken) {
         ALOGE("Unexpected token: 0x%" PRIx64 ", should be 0x%" PRIx64, token, mExpectedObjectToken);
+        mDepth = UINT32_C(-1); // make depth invalid
         return;
     }
 
     uint32_t depth = getDepthFromToken(token);
     if (depth != (mDepth & 0x01ff)) {
         ALOGE("Unexpected depth: %" PRIu32 ", should be %" PRIu32, depth, mDepth);
+        mDepth = UINT32_C(-1); // make depth invalid
         return;
     }
     mDepth--;
@@ -282,7 +284,7 @@ bool
 ProtoOutputStream::compact() {
     if (mCompact) return true;
     if (mDepth != 0) {
-        ALOGE("Can't compact when depth(%" PRIu32 ") is not zero. Missing calls to end.", mDepth);
+        ALOGE("Can't compact when depth(%" PRIu32 ") is not zero. Missing or extra calls to end.", mDepth);
         return false;
     }
     // record the size of the original buffer.
@@ -425,7 +427,7 @@ ProtoOutputStream::size()
 {
     if (!compact()) {
         ALOGE("compact failed, the ProtoOutputStream data is corrupted!");
-        // TODO: handle this error
+        return 0;
     }
     return mBuffer.size();
 }
@@ -449,7 +451,7 @@ ProtoOutputStream::data()
 {
     if (!compact()) {
         ALOGE("compact failed, the ProtoOutputStream data is corrupted!");
-        // TODO: handle this error
+        mBuffer.clear();
     }
     return mBuffer.begin();
 }
diff --git a/libs/protoutil/tests/ProtoOutputStream_test.cpp b/libs/protoutil/tests/ProtoOutputStream_test.cpp
index 5c25787e8555..27ee13d4dfe1 100644
--- a/libs/protoutil/tests/ProtoOutputStream_test.cpp
+++ b/libs/protoutil/tests/ProtoOutputStream_test.cpp
@@ -189,4 +189,40 @@ TEST(ProtoOutputStreamTest, InvalidTypes) {
     EXPECT_FALSE(proto.write(FIELD_TYPE_ENUM | PrimitiveProto::kValEnumFieldNumber, 234.34));
     EXPECT_FALSE(proto.write(FIELD_TYPE_BOOL | PrimitiveProto::kValBoolFieldNumber, 18.73f));
     EXPECT_EQ(proto.size(), 0);
-}
\ No newline at end of file
+}
+
+TEST(ProtoOutputStreamTest, NoEndCalled) {
+    ProtoOutputStream proto;
+    proto.start(FIELD_TYPE_MESSAGE | ComplexProto::kLogsFieldNumber);
+    proto.write(FIELD_TYPE_INT32 | ComplexProto::Log::kIdFieldNumber, 53);
+    // no proto.end called
+    EXPECT_NE(proto.bytesWritten(), 0);
+    EXPECT_EQ(proto.size(), 0);
+    EXPECT_EQ(proto.data().size(), 0);
+    EXPECT_FALSE(proto.flush(STDOUT_FILENO));
+}
+
+
+TEST(ProtoOutputStreamTest, TwoEndCalled) {
+    ProtoOutputStream proto;
+    uint64_t token = proto.start(FIELD_TYPE_MESSAGE | ComplexProto::kLogsFieldNumber);
+    proto.write(FIELD_TYPE_INT32 | ComplexProto::Log::kIdFieldNumber, 53);
+    proto.end(token);
+    proto.end(token);
+    EXPECT_NE(proto.bytesWritten(), 0);
+    EXPECT_EQ(proto.size(), 0);
+    EXPECT_EQ(proto.data().size(), 0);
+    EXPECT_FALSE(proto.flush(STDOUT_FILENO));
+}
+
+TEST(ProtoOutputStreamTest, NoStartCalled) {
+    ProtoOutputStream proto;
+    uint64_t wrongToken = UINT64_C(324536345);
+    // no proto.start called
+    proto.write(FIELD_TYPE_INT32 | ComplexProto::Log::kIdFieldNumber, 53);
+    proto.end(wrongToken);
+    EXPECT_NE(proto.bytesWritten(), 0);
+    EXPECT_EQ(proto.size(), 0);
+    EXPECT_EQ(proto.data().size(), 0);
+    EXPECT_FALSE(proto.flush(STDOUT_FILENO));
+}