Forbid untyped list/map in every parcelable
Bug: 171373954
Test: aidl_unittests
Change-Id: I4f3cead27c4e05d7b0d4e531dd90b0e8f2e254cb
diff --git a/aidl.cpp b/aidl.cpp
index 2f00a12..94e8387 100644
--- a/aidl.cpp
+++ b/aidl.cpp
@@ -707,32 +707,41 @@
<< " does not have VINTF level stability, but this interface requires it.";
}
- // Ensure that untyped List/Map is not used in stable AIDL.
- if (options.IsStructured()) {
- const AidlInterface* iface = type.AsInterface();
- const AidlStructuredParcelable* parcelable = type.AsStructuredParcelable();
+ // Ensure that untyped List/Map is not used in a parcelable, a union and a stable interface.
- auto check = [&err](const AidlTypeSpecifier& type, const AidlNode* node) {
- if (!type.IsGeneric() && (type.GetName() == "List" || type.GetName() == "Map")) {
- err = AidlError::BAD_TYPE;
- AIDL_ERROR(node)
- << "Encountered an untyped List or Map. The use of untyped List/Map is prohibited "
- << "because it is not guaranteed that the objects in the list are recognizable in "
- << "the receiving side. Consider switching to an array or a generic List/Map.";
- }
- };
-
- if (iface != nullptr) {
- for (const auto& method : iface->GetMethods()) {
- check(method->GetType(), method.get());
- for (const auto& arg : method->GetArguments()) {
- check(arg->GetType(), method.get());
+ std::function<void(const AidlTypeSpecifier&, const AidlNode*)> check_untyped_container =
+ [&err, &check_untyped_container](const AidlTypeSpecifier& type, const AidlNode* node) {
+ if (type.IsGeneric()) {
+ std::for_each(type.GetTypeParameters().begin(), type.GetTypeParameters().end(),
+ [&node, &check_untyped_container](auto& nested) {
+ check_untyped_container(*nested, node);
+ });
+ return;
}
+ if (type.GetName() == "List" || type.GetName() == "Map") {
+ err = AidlError::BAD_TYPE;
+ AIDL_ERROR(node)
+ << "Encountered an untyped List or Map. The use of untyped List/Map is prohibited "
+ << "because it is not guaranteed that the objects in the list are recognizable in "
+ << "the receiving side. Consider switching to an array or a generic List/Map.";
+ }
+ };
+ const AidlInterface* iface = type.AsInterface();
+ const AidlWithFields* data_structure = type.AsStructuredParcelable();
+ if (!data_structure) {
+ data_structure = type.AsUnionDeclaration();
+ }
+
+ if (iface != nullptr && options.IsStructured()) {
+ for (const auto& method : iface->GetMethods()) {
+ check_untyped_container(method->GetType(), method.get());
+ for (const auto& arg : method->GetArguments()) {
+ check_untyped_container(arg->GetType(), method.get());
}
- } else if (parcelable != nullptr) {
- for (const auto& field : parcelable->GetFields()) {
- check(field->GetType(), field.get());
- }
+ }
+ } else if (data_structure != nullptr) {
+ for (const auto& field : data_structure->GetFields()) {
+ check_untyped_container(field->GetType(), field.get());
}
}
});
diff --git a/aidl_unittest.cpp b/aidl_unittest.cpp
index 654e5e0..fa86626 100644
--- a/aidl_unittest.cpp
+++ b/aidl_unittest.cpp
@@ -3538,64 +3538,57 @@
EXPECT_EQ("", GetCapturedStderr());
}
-constexpr char kDescriptContentsWithUntypedListMapInJava[] = R"(
- @Override
- public int describeContents() {
- int _mask = 0;
- _mask |= describeContents(l);
- _mask |= describeContents(m);
- return _mask;
- }
- private int describeContents(Object _v) {
- if (_v == null) return 0;
- Class<?> _clazz = _v.getClass();
- if (_clazz.isArray() && _clazz.getComponentType() == Object.class) {
- int _mask = 0;
- for (Object o : (Object[]) _v) {
- _mask |= describeContents(o);
- }
- return _mask;
- }
- if (_v instanceof java.io.FileDescriptor) {
- return android.os.Parcelable.CONTENTS_FILE_DESCRIPTOR;
- }
- if (_v instanceof java.util.Collection) {
- int _mask = 0;
- for (Object o : (java.util.Collection) _v) {
- _mask |= describeContents(o);
- }
- return _mask;
- }
- if (_v instanceof java.util.Map) {
- return describeContents(((java.util.Map) _v).values());
- }
- if (_v instanceof android.os.Parcelable) {
- return ((android.os.Parcelable) _v).describeContents();
- }
- if (_v instanceof android.util.SparseArray) {
- android.util.SparseArray _sa = (android.util.SparseArray) _v;
- int _mask = 0;
- int _N = _sa.size();
- int _i = 0;
- while (_i < _N) {
- _mask |= describeContents(_sa.valueAt(_i));
- _i++;
- }
- return _mask;
- }
- return 0;
- }
-)";
-TEST_F(AidlTest, SupportUntypeListAndMap) {
+TEST_F(AidlTest, RejectUntypdeListAndMapInUnion) {
+ io_delegate_.SetFileContents("a/Foo.aidl", "package a; union Foo { List l; Map m; }");
+ Options options = Options::From("aidl a/Foo.aidl --lang=java -o out");
+ std::string expectedErr =
+ "ERROR: a/Foo.aidl:1.28-30: "
+ "Encountered an untyped List or Map. The use of untyped List/Map is "
+ "prohibited because it is not guaranteed that the objects in the list are recognizable in "
+ "the receiving side. Consider switching to an array or a generic List/Map.\n"
+ "ERROR: a/Foo.aidl:1.35-37: "
+ "Encountered an untyped List or Map. The use of untyped List/Map is "
+ "prohibited because it is not guaranteed that the objects in the list are recognizable in "
+ "the receiving side. Consider switching to an array or a generic List/Map.\n";
+ CaptureStderr();
+ EXPECT_NE(0, ::android::aidl::compile_aidl(options, io_delegate_));
+ EXPECT_EQ(expectedErr, GetCapturedStderr());
+}
+
+TEST_F(AidlTest, RejectUntypdeListAndMapInUnstructuredParcelable) {
io_delegate_.SetFileContents("a/Foo.aidl", "package a; parcelable Foo { List l; Map m; }");
Options options = Options::From("aidl a/Foo.aidl --lang=java -o out");
+ std::string expectedErr =
+ "ERROR: a/Foo.aidl:1.33-35: "
+ "Encountered an untyped List or Map. The use of untyped List/Map is "
+ "prohibited because it is not guaranteed that the objects in the list are recognizable in "
+ "the receiving side. Consider switching to an array or a generic List/Map.\n"
+ "ERROR: a/Foo.aidl:1.40-42: "
+ "Encountered an untyped List or Map. The use of untyped List/Map is "
+ "prohibited because it is not guaranteed that the objects in the list are recognizable in "
+ "the receiving side. Consider switching to an array or a generic List/Map.\n";
CaptureStderr();
- EXPECT_EQ(0, ::android::aidl::compile_aidl(options, io_delegate_));
- EXPECT_EQ("", GetCapturedStderr());
+ EXPECT_NE(0, ::android::aidl::compile_aidl(options, io_delegate_));
+ EXPECT_EQ(expectedErr, GetCapturedStderr());
+}
- string code;
- EXPECT_TRUE(io_delegate_.GetWrittenContents("out/a/Foo.java", &code));
- EXPECT_THAT(code, testing::HasSubstr(kDescriptContentsWithUntypedListMapInJava));
+TEST_F(AidlTest, RejectNestedUntypedListAndMap) {
+ io_delegate_.SetFileContents("a/Bar.aidl", "package a; parcelable Bar<T>;");
+ io_delegate_.SetFileContents(
+ "a/Foo.aidl", "package a; import a.Bar; parcelable Foo { Bar<List> a; Bar<Map> b; }");
+ Options options = Options::From("aidl a/Foo.aidl -I . --lang=java -o out");
+ std::string expectedErr =
+ "ERROR: a/Foo.aidl:1.52-54: "
+ "Encountered an untyped List or Map. The use of untyped List/Map is "
+ "prohibited because it is not guaranteed that the objects in the list are recognizable in "
+ "the receiving side. Consider switching to an array or a generic List/Map.\n"
+ "ERROR: a/Foo.aidl:1.64-66: "
+ "Encountered an untyped List or Map. The use of untyped List/Map is "
+ "prohibited because it is not guaranteed that the objects in the list are recognizable in "
+ "the receiving side. Consider switching to an array or a generic List/Map.\n";
+ CaptureStderr();
+ EXPECT_NE(0, ::android::aidl::compile_aidl(options, io_delegate_));
+ EXPECT_EQ(expectedErr, GetCapturedStderr());
}
} // namespace aidl
diff --git a/generate_java.cpp b/generate_java.cpp
index 7e24c06..3bff58c 100644
--- a/generate_java.cpp
+++ b/generate_java.cpp
@@ -69,17 +69,6 @@
}
return _mask;
})"},
- {"SparseArray", R"(if (_v instanceof android.util.SparseArray) {
- android.util.SparseArray _sa = (android.util.SparseArray) _v;
- int _mask = 0;
- int _N = _sa.size();
- int _i = 0;
- while (_i < _N) {
- _mask |= describeContents(_sa.valueAt(_i));
- _i++;
- }
- return _mask;
-})"},
{"Array", R"(Class<?> _clazz = _v.getClass();
if (_clazz.isArray() && _clazz.getComponentType() == Object.class) {
int _mask = 0;
@@ -107,14 +96,6 @@
// e.g. FileDescriptor, Parcelables, List<Parcelables> ...
bool CanDescribeContents(const AidlTypeSpecifier& type, const AidlTypenames& types,
set<string>* describers) {
- if ((type.GetName() == "List" || type.GetName() == "Map") && !type.IsGeneric()) {
- // needs all describers
- for (const auto d : contents_describers) {
- describers->insert(d.first);
- }
- return true;
- }
-
if (type.IsArray()) {
if (CanDescribeContents(type.ArrayBase(), types, describers)) {
describers->insert("Array");