Remove limit of system property name length
This change introduces new __system_property_read_callback
method to use in place of deprecated __system_property_read
__system_property_set() and get() should just work but now
do not have limit on system property names.
Bug: http://b/33926793
Test: boot device, run adb shell propget
Test: boot device with old version of init (protocol v1)
Test: run bionic-unit-tests --gtest_filter=prop*
Change-Id: I619fb5a7e27a272aac30011579665f6160888bc7
diff --git a/tests/Android.bp b/tests/Android.bp
index 85ae26e..64a5c30 100644
--- a/tests/Android.bp
+++ b/tests/Android.bp
@@ -132,6 +132,7 @@
"sys_vfs_test.cpp",
"sys_xattr_test.cpp",
"system_properties_test.cpp",
+ "system_properties_test2.cpp",
"time_test.cpp",
"uchar_test.cpp",
"unistd_nofortify_test.cpp",
diff --git a/tests/system_properties_test.cpp b/tests/system_properties_test.cpp
index 70482f0..6b037d8 100644
--- a/tests/system_properties_test.cpp
+++ b/tests/system_properties_test.cpp
@@ -69,7 +69,7 @@
static void foreach_test_callback(const prop_info *pi, void* cookie) {
size_t *count = static_cast<size_t *>(cookie);
- ASSERT_NE((prop_info *)NULL, pi);
+ ASSERT_TRUE(pi != nullptr);
(*count)++;
}
@@ -98,16 +98,15 @@
ok[name_i][name_j][name_k] = true;
}
-static void *PropertyWaitHelperFn(void *arg) {
- int *flag = (int *)arg;
- prop_info *pi;
- pi = (prop_info *)__system_property_find("property");
+static void* PropertyWaitHelperFn(void* arg) {
+ int* flag = static_cast<int*>(arg);
+ prop_info* pi = const_cast<prop_info*>(__system_property_find("property"));
usleep(100000);
*flag = 1;
__system_property_update(pi, "value3", 6);
- return NULL;
+ return nullptr;
}
#endif // __BIONIC__
@@ -123,6 +122,16 @@
ASSERT_EQ(0, __system_property_add("other_property", 14, "value2", 6));
ASSERT_EQ(0, __system_property_add("property_other", 14, "value3", 6));
+ // check that there is no limit on property name length
+ char name[PROP_NAME_MAX + 11];
+ name[0] = 'p';
+ for (size_t i = 1; i < sizeof(name); i++) {
+ name[i] = 'x';
+ }
+
+ name[sizeof(name)-1] = '\0';
+ ASSERT_EQ(0, __system_property_add(name, strlen(name), "value", 5));
+
ASSERT_EQ(6, __system_property_get("property", propvalue));
ASSERT_STREQ(propvalue, "value1");
@@ -131,6 +140,9 @@
ASSERT_EQ(6, __system_property_get("property_other", propvalue));
ASSERT_STREQ(propvalue, "value3");
+
+ ASSERT_EQ(5, __system_property_get(name, propvalue));
+ ASSERT_STREQ(propvalue, "value");
#else // __BIONIC__
GTEST_LOG_(INFO) << "This test does nothing.\n";
#endif // __BIONIC__
@@ -323,7 +335,6 @@
ASSERT_EQ(0, __system_property_find("property1"));
ASSERT_EQ(0, __system_property_get("property1", prop_value));
- ASSERT_EQ(-1, __system_property_add("name", PROP_NAME_MAX, "value", 5));
ASSERT_EQ(-1, __system_property_add("name", 4, "value", PROP_VALUE_MAX));
ASSERT_EQ(-1, __system_property_update(NULL, "value", PROP_VALUE_MAX));
#else // __BIONIC__
@@ -359,12 +370,13 @@
ASSERT_EQ(0, __system_property_add("property", 8, "value1", 6));
serial = __system_property_wait_any(0);
- pi = (prop_info *)__system_property_find("property");
- ASSERT_NE((prop_info *)NULL, pi);
+
+ pi = const_cast<prop_info*>(__system_property_find("property"));
+ ASSERT_TRUE(pi != nullptr);
__system_property_update(pi, "value2", 6);
serial = __system_property_wait_any(serial);
- ASSERT_EQ(0, pthread_create(&t, NULL, PropertyWaitHelperFn, &flag));
+ ASSERT_EQ(0, pthread_create(&t, nullptr, PropertyWaitHelperFn, &flag));
ASSERT_EQ(flag, 0);
serial = __system_property_wait_any(serial);
ASSERT_EQ(flag, 1);
@@ -396,9 +408,7 @@
// This test only makes sense if we're talking to the real system property service.
struct stat sb;
- if (stat(PROP_FILENAME, &sb) == -1 && errno == ENOENT) {
- return;
- }
+ ASSERT_FALSE(stat(PROP_FILENAME, &sb) == -1 && errno == ENOENT);
ASSERT_EXIT(__system_property_add("property", 8, "value", 5), KilledByFault(), "");
#else // __BIONIC__
diff --git a/tests/system_properties_test2.cpp b/tests/system_properties_test2.cpp
new file mode 100644
index 0000000..858bc17
--- /dev/null
+++ b/tests/system_properties_test2.cpp
@@ -0,0 +1,128 @@
+/*
+ * 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 <gtest/gtest.h>
+
+#include <errno.h>
+#include <sys/wait.h>
+#include <unistd.h>
+#include <sstream>
+#include <string>
+
+#if defined(__BIONIC__)
+#include <sys/system_properties.h>
+
+static uint64_t NanoTime() {
+ timespec now;
+ clock_gettime(CLOCK_MONOTONIC, &now);
+ return static_cast<uint64_t>(now.tv_sec) * UINT64_C(1000000000) + now.tv_nsec;
+}
+#endif
+
+// Note that this test affects global state of the system
+// this tests tries to mitigate this by using utime+pid
+// prefix for the property name. It is still results in
+// pollution of property service since properties cannot
+// be removed.
+//
+// Note that there is also possibility to run into "out-of-memory"
+// if this test if it is executed often enough without reboot.
+TEST(properties, smoke) {
+#if defined(__BIONIC__)
+ char propvalue[PROP_VALUE_MAX];
+
+ std::stringstream ss;
+ ss << "debug.test." << getpid() << "." << NanoTime() << ".";
+ const std::string property_prefix = ss.str();
+ const std::string property_name = property_prefix + "property1";
+
+ // Set brand new property
+ ASSERT_EQ(0, __system_property_set(property_name.c_str(), "value1"));
+ ASSERT_EQ(6, __system_property_get(property_name.c_str(), propvalue));
+ ASSERT_STREQ("value1", propvalue);
+
+ std::string long_value = "property-";
+ for (size_t i = 0; i < PROP_VALUE_MAX; i++) {
+ long_value += "y";
+ }
+
+ // Make sure that attempts to set invalid property value fails and preserves
+ // previous value.
+ propvalue[0] = '\0';
+ ASSERT_EQ(-1, __system_property_set(property_name.c_str(), long_value.c_str()));
+ ASSERT_EQ(6, __system_property_get(property_name.c_str(), propvalue));
+ ASSERT_STREQ("value1", propvalue);
+
+ // Update property
+ ASSERT_EQ(0, __system_property_set(property_name.c_str(), "value1-1"));
+ ASSERT_EQ(8, __system_property_get(property_name.c_str(), propvalue));
+ ASSERT_STREQ("value1-1", propvalue);
+
+
+ // check that there is no limit on property name length
+ char suffix[1024];
+ for (size_t i = 0; i < sizeof(suffix); i++) {
+ suffix[i] = 'x';
+ }
+
+ suffix[sizeof(suffix)-1] = '\0';
+ const std::string long_property_name = property_prefix + suffix;
+
+ ASSERT_EQ(0, __system_property_set(long_property_name.c_str(), "value2"));
+ ASSERT_EQ(6, __system_property_get(long_property_name.c_str(), propvalue));
+ ASSERT_STREQ("value2", propvalue);
+
+ // test find and read_callback
+ const prop_info* pi = __system_property_find(property_name.c_str());
+ ASSERT_TRUE(pi != nullptr);
+
+ std::string expected_name = property_name;
+ __system_property_read_callback(pi, [](void* cookie, const char *name, const char *value) {
+ const std::string* expected_name = static_cast<const std::string*>(cookie);
+ ASSERT_EQ(*expected_name, name);
+ ASSERT_STREQ("value1-1", value);
+ }, &expected_name);
+
+ pi = __system_property_find(long_property_name.c_str());
+ ASSERT_TRUE(pi != nullptr);
+
+ expected_name = long_property_name;
+ __system_property_read_callback(pi, [](void* cookie, const char *name, const char *value) {
+ const std::string* expected_name = static_cast<const std::string*>(cookie);
+ ASSERT_EQ(*expected_name, name);
+ ASSERT_STREQ("value2", value);
+ }, &expected_name);
+
+ // Check that read() for long names still works but returns truncated version of the name
+ pi = __system_property_find(property_name.c_str());
+ ASSERT_TRUE(pi != nullptr);
+ char legacy_name[PROP_NAME_MAX];
+ expected_name = std::string(property_name.c_str(), PROP_NAME_MAX-1);
+ ASSERT_EQ(8, __system_property_read(pi, &legacy_name[0], propvalue));
+ ASSERT_EQ(expected_name, legacy_name);
+ ASSERT_STREQ("value1-1", propvalue);
+
+ const prop_info* pi_long = __system_property_find(long_property_name.c_str());
+ ASSERT_TRUE(pi != nullptr);
+ expected_name = std::string(long_property_name.c_str(), PROP_NAME_MAX-1);
+ ASSERT_EQ(6, __system_property_read(pi_long, &legacy_name[0], propvalue));
+ ASSERT_EQ(expected_name, legacy_name);
+ ASSERT_STREQ("value2", propvalue);
+#else // __BIONIC__
+ GTEST_LOG_(INFO) << "This test does nothing.\n";
+#endif // __BIONIC__
+}
+