Merge "Allow statsd to be given empty config." into pi-dev

cmds/statsd/Android.mk

@@ -177,6 +177,7 @@ LOCAL_SRC_FILES := \
     tests/LogEvent_test.cpp \
     tests/MetricsManager_test.cpp \
     tests/StatsLogProcessor_test.cpp \
+    tests/StatsService_test.cpp \
     tests/UidMap_test.cpp \
     tests/FieldValue_test.cpp \
     tests/condition/CombinationConditionTracker_test.cpp \

cmds/statsd/src/StatsService.cpp

@@ -867,14 +867,11 @@ Status StatsService::addConfiguration(int64_t key,
                                       bool* success) {
     IPCThreadState* ipc = IPCThreadState::self();
     if (checkCallingPermission(String16(kPermissionDump))) {
-        ConfigKey configKey(ipc->getCallingUid(), key);
-        StatsdConfig cfg;
-        if (config.empty() || !cfg.ParseFromArray(&config[0], config.size())) {
+        if (addConfigurationChecked(ipc->getCallingUid(), key, config)) {
+            *success = true;
+        } else {
             *success = false;
-            return Status::ok();
         }
-        mConfigManager->UpdateConfig(configKey, cfg);
-        *success = true;
         return Status::ok();
     } else {
         *success = false;
@@ -882,6 +879,18 @@ Status StatsService::addConfiguration(int64_t key,
     }
 }
 
+bool StatsService::addConfigurationChecked(int uid, int64_t key, const vector<uint8_t>& config) {
+    ConfigKey configKey(uid, key);
+    StatsdConfig cfg;
+    if (config.size() > 0) {  // If the config is empty, skip parsing.
+        if (!cfg.ParseFromArray(&config[0], config.size())) {
+            return false;
+        }
+    }
+    mConfigManager->UpdateConfig(configKey, cfg);
+    return true;
+}
+
 Status StatsService::removeDataFetchOperation(int64_t key, bool* success) {
     IPCThreadState* ipc = IPCThreadState::self();
     if (checkCallingPermission(String16(kPermissionDump))) {

cmds/statsd/src/StatsService.h

@@ -17,6 +17,7 @@
 #ifndef STATS_SERVICE_H
 #define STATS_SERVICE_H
 
+#include <gtest/gtest_prod.h>
 #include "StatsLogProcessor.h"
 #include "anomaly/AlarmMonitor.h"
 #include "config/ConfigManager.h"
@@ -215,6 +216,11 @@ private:
      */
     status_t cmd_clear_puller_cache(FILE* out);
 
+    /**
+     * Adds a configuration after checking permissions and obtaining UID from binder call.
+     */
+    bool addConfigurationChecked(int uid, int64_t key, const vector<uint8_t>& config);
+
     /**
      * Update a configuration.
      */
@@ -254,6 +260,10 @@ private:
      * Whether this is an eng build.
      */
     bool mEngBuild;
+
+    FRIEND_TEST(StatsServiceTest, TestAddConfig_simple);
+    FRIEND_TEST(StatsServiceTest, TestAddConfig_empty);
+    FRIEND_TEST(StatsServiceTest, TestAddConfig_invalid);
 };
 
 }  // namespace statsd

cmds/statsd/tests/StatsService_test.cpp

@@ -0,0 +1,67 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// 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.
+
+#include "StatsService.h"
+#include "config/ConfigKey.h"
+#include "frameworks/base/cmds/statsd/src/statsd_config.pb.h"
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include <stdio.h>
+
+using namespace android;
+using namespace testing;
+
+namespace android {
+namespace os {
+namespace statsd {
+
+using android::util::ProtoOutputStream;
+
+#ifdef __ANDROID__
+
+TEST(StatsServiceTest, TestAddConfig_simple) {
+    StatsService service(nullptr);
+    StatsdConfig config;
+    config.set_id(12345);
+    string serialized = config.SerializeAsString();
+
+    EXPECT_TRUE(
+            service.addConfigurationChecked(123, 12345, {serialized.begin(), serialized.end()}));
+}
+
+TEST(StatsServiceTest, TestAddConfig_empty) {
+    StatsService service(nullptr);
+    string serialized = "";
+
+    EXPECT_TRUE(
+            service.addConfigurationChecked(123, 12345, {serialized.begin(), serialized.end()}));
+}
+
+TEST(StatsServiceTest, TestAddConfig_invalid) {
+    StatsService service(nullptr);
+    string serialized = "Invalid config!";
+
+    EXPECT_FALSE(
+            service.addConfigurationChecked(123, 12345, {serialized.begin(), serialized.end()}));
+}
+
+#else
+GTEST_LOG_(INFO) << "This test does nothing.\n";
+#endif
+
+}  // namespace statsd
+}  // namespace os
+}  // namespace android