Revert "libbinder_rs: Treat previously associated Binder as remote"

This reverts commit 29422bf9421560bf283c063ad13a82fa30d9a141.

Reason for revert: b/174694961

Change-Id: I3043d1c9b7b317c9bf9a0ebeedf0ef1e16025827

9 files changed, 18 insertions, 358 deletions

diff --git a/libs/binder/TEST_MAPPING b/libs/binder/TEST_MAPPING
index 437581884f..1cfb560f82 100644
--- a/libs/binder/TEST_MAPPING
+++ b/libs/binder/TEST_MAPPING
@@ -56,9 +56,6 @@
     },
     {
       "name": "rustBinderTest"
-    },
-    {
-      "name": "binderRustNdkInteropTest"
     }
   ]
 }
diff --git a/libs/binder/rust/src/binder.rs b/libs/binder/rust/src/binder.rs
index b558f27e06..037ee95683 100644
--- a/libs/binder/rust/src/binder.rs
+++ b/libs/binder/rust/src/binder.rs
@@ -21,8 +21,7 @@ use crate::parcel::Parcel;
 use crate::proxy::{DeathRecipient, SpIBinder};
 use crate::sys;
 
-use std::ffi::{c_void, CStr, CString};
-use std::os::raw::c_char;
+use std::ffi::{c_void, CString};
 use std::os::unix::io::AsRawFd;
 use std::ptr;
 
@@ -206,22 +205,6 @@ impl InterfaceClass {
     pub(crate) unsafe fn from_ptr(ptr: *const sys::AIBinder_Class) -> InterfaceClass {
         InterfaceClass(ptr)
     }
-
-    /// Get the interface descriptor string of this class.
-    pub fn get_descriptor(&self) -> String {
-        unsafe {
-            // SAFETY: The descriptor returned by AIBinder_Class_getDescriptor
-            // is always a two-byte null terminated sequence of u16s. Thus, we
-            // can continue reading from the pointer until we hit a null value,
-            // and this pointer can be a valid slice if the slice length is <=
-            // the number of u16 elements before the null terminator.
-
-            let raw_descriptor: *const c_char = sys::AIBinder_Class_getDescriptor(self.0);
-            CStr::from_ptr(raw_descriptor).to_str()
-                .expect("Expected valid UTF-8 string from AIBinder_Class_getDescriptor")
-                .into()
-        }
-    }
 }
 
 impl From<InterfaceClass> for *const sys::AIBinder_Class {
@@ -524,7 +507,12 @@ macro_rules! declare_binder_interface {
             }
 
             fn from_binder(mut binder: $crate::SpIBinder) -> $crate::Result<Self> {
-                Ok(Self { binder, $($fname: $finit),* })
+                use $crate::AssociateClass;
+                if binder.associate_class(<$native as $crate::Remotable>::get_class()) {
+                    Ok(Self { binder, $($fname: $finit),* })
+                } else {
+                    Err($crate::StatusCode::BAD_TYPE)
+                }
             }
         }
 
@@ -579,33 +567,16 @@ macro_rules! declare_binder_interface {
         impl $crate::FromIBinder for dyn $interface {
             fn try_from(mut ibinder: $crate::SpIBinder) -> $crate::Result<Box<dyn $interface>> {
                 use $crate::AssociateClass;
-
-                let existing_class = ibinder.get_class();
-                if let Some(class) = existing_class {
-                    if class != <$native as $crate::Remotable>::get_class() &&
-                        class.get_descriptor() == <$native as $crate::Remotable>::get_descriptor()
-                    {
-                        // The binder object's descriptor string matches what we
-                        // expect. We still need to treat this local or already
-                        // associated object as remote, because we can't cast it
-                        // into a Rust service object without a matching class
-                        // pointer.
-                        return Ok(Box::new(<$proxy as $crate::Proxy>::from_binder(ibinder)?));
-                    }
-                } else if ibinder.associate_class(<$native as $crate::Remotable>::get_class()) {
-                    let service: $crate::Result<$crate::Binder<$native>> =
-                        std::convert::TryFrom::try_from(ibinder.clone());
-                    if let Ok(service) = service {
-                        // We were able to associate with our expected class and
-                        // the service is local.
-                        return Ok(Box::new(service));
-                    } else {
-                        // Service is remote
-                        return Ok(Box::new(<$proxy as $crate::Proxy>::from_binder(ibinder)?));
-                    }
+                if !ibinder.associate_class(<$native as $crate::Remotable>::get_class()) {
+                    return Err($crate::StatusCode::BAD_TYPE.into());
                 }
 
-                Err($crate::StatusCode::BAD_TYPE.into())
+                let service: $crate::Result<$crate::Binder<$native>> = std::convert::TryFrom::try_from(ibinder.clone());
+                if let Ok(service) = service {
+                    Ok(Box::new(service))
+                } else {
+                    Ok(Box::new(<$proxy as $crate::Proxy>::from_binder(ibinder)?))
+                }
             }
         }
 
diff --git a/libs/binder/rust/src/proxy.rs b/libs/binder/rust/src/proxy.rs
index 485bb422fa..5002fc6d83 100644
--- a/libs/binder/rust/src/proxy.rs
+++ b/libs/binder/rust/src/proxy.rs
@@ -91,7 +91,7 @@ impl SpIBinder {
 
     /// Return the interface class of this binder object, if associated with
     /// one.
-    pub fn get_class(&mut self) -> Option<InterfaceClass> {
+    pub(crate) fn get_class(&mut self) -> Option<InterfaceClass> {
         unsafe {
             // Safety: `SpIBinder` guarantees that it always contains a valid
             // `AIBinder` pointer. `AIBinder_getClass` returns either a null
diff --git a/libs/binder/rust/tests/Android.bp b/libs/binder/rust/tests/Android.bp
index 5ae9c53077..3db40ba92d 100644
--- a/libs/binder/rust/tests/Android.bp
+++ b/libs/binder/rust/tests/Android.bp
@@ -30,52 +30,3 @@ rust_test {
     auto_gen_config: false,
     test_suites: ["general-tests"],
 }
-
-cc_test {
-    name: "binderRustNdkInteropTest",
-    srcs: [
-        "binderRustNdkInteropTest.cpp",
-    ],
-    shared_libs: [
-        "libbinder",
-        "libbinder_ndk",
-    ],
-    static_libs: [
-        "IBinderRustNdkInteropTest-ndk_platform",
-        "libbinder_ndk_rust_interop",
-    ],
-    test_suites: ["general-tests"],
-    require_root: true,
-
-    // rustBinderTestService uses a custom config
-    auto_gen_config: true,
-}
-
-aidl_interface {
-    name: "IBinderRustNdkInteropTest",
-    unstable: true,
-    srcs: [
-        "IBinderRustNdkInteropTest.aidl",
-        "IBinderRustNdkInteropTestOther.aidl",
-    ],
-    backend: {
-        ndk: {
-            enabled: true,
-        },
-        rust: {
-            enabled: true,
-        },
-    },
-}
-
-rust_ffi_static {
-    name: "libbinder_ndk_rust_interop",
-    crate_name: "binder_ndk_rust_interop",
-    srcs: [
-        "ndk_rust_interop.rs",
-    ],
-    rustlibs: [
-        "libbinder_rs",
-        "IBinderRustNdkInteropTest-rust",
-    ],
-}
diff --git a/libs/binder/rust/tests/IBinderRustNdkInteropTest.aidl b/libs/binder/rust/tests/IBinderRustNdkInteropTest.aidl
deleted file mode 100644
index 7f5e837d28..0000000000
--- a/libs/binder/rust/tests/IBinderRustNdkInteropTest.aidl
+++ /dev/null
@@ -1,19 +0,0 @@
-/*
- * Copyright (C) 2020 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.
- */
-
-interface IBinderRustNdkInteropTest {
-    @utf8InCpp String echo(@utf8InCpp String str);
-}
diff --git a/libs/binder/rust/tests/IBinderRustNdkInteropTestOther.aidl b/libs/binder/rust/tests/IBinderRustNdkInteropTestOther.aidl
deleted file mode 100644
index 82a03237a4..0000000000
--- a/libs/binder/rust/tests/IBinderRustNdkInteropTestOther.aidl
+++ /dev/null
@@ -1,19 +0,0 @@
-/*
- * Copyright (C) 2020 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.
- */
-
-interface IBinderRustNdkInteropTestOther {
-    @utf8InCpp String echo(@utf8InCpp String str);
-}
diff --git a/libs/binder/rust/tests/binderRustNdkInteropTest.cpp b/libs/binder/rust/tests/binderRustNdkInteropTest.cpp
deleted file mode 100644
index 59ca6edf12..0000000000
--- a/libs/binder/rust/tests/binderRustNdkInteropTest.cpp
+++ /dev/null
@@ -1,78 +0,0 @@
-/*
- * Copyright (C) 2020 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 <aidl/BnBinderRustNdkInteropTest.h>
-#include <aidl/IBinderRustNdkInteropTest.h>
-#include <android/binder_ibinder.h>
-#include <android/binder_manager.h>
-#include <android/binder_process.h>
-#include <binder/Status.h>
-#include <gtest/gtest.h>
-
-using namespace android;
-using ::ndk::ScopedAStatus;
-using ::ndk::SharedRefBase;
-using ::ndk::SpAIBinder;
-
-static const char* kNdkServerName = "NdkServer-BinderRustNdkInteropTest";
-static const char* kRustServerName = "RustServer-BinderRustNdkInteropTest";
-
-extern "C" {
-int rust_call_ndk(const char* service_name);
-int rust_start_service(const char* service_name);
-}
-
-class NdkServer : public aidl::BnBinderRustNdkInteropTest {
-    ScopedAStatus echo(const std::string& in, std::string* out) override {
-        *out = in;
-        return ScopedAStatus::ok();
-    }
-};
-
-TEST(RustNdkInterop, RustCanCallNdk) {
-    ASSERT_EQ(STATUS_OK, rust_call_ndk(kNdkServerName));
-}
-
-TEST(RustNdkInterop, NdkCanCallRust) {
-    ASSERT_EQ(STATUS_OK, rust_start_service(kRustServerName));
-
-    SpAIBinder binder = SpAIBinder(AServiceManager_checkService(kRustServerName));
-    ASSERT_NE(nullptr, binder.get());
-    EXPECT_EQ(STATUS_OK, AIBinder_ping(binder.get()));
-
-    auto interface = aidl::IBinderRustNdkInteropTest::fromBinder(binder);
-    // TODO(b/167723746): this test requires that fromBinder allow association
-    // with an already associated local binder by treating it as remote.
-    EXPECT_EQ(interface, nullptr);
-
-    // std::string in("testing");
-    // std::string out;
-    // EXPECT_TRUE(interface->echo(in, &out).isOk());
-    // EXPECT_EQ(in, out);
-}
-
-int main(int argc, char** argv) {
-    ::testing::InitGoogleTest(&argc, argv);
-
-    // so we can host a client and service concurrently
-    ABinderProcess_setThreadPoolMaxThreadCount(1);
-    ABinderProcess_startThreadPool();
-
-    std::shared_ptr<NdkServer> ndkServer = SharedRefBase::make<NdkServer>();
-    EXPECT_EQ(STATUS_OK, AServiceManager_addService(ndkServer->asBinder().get(), kNdkServerName));
-
-    return RUN_ALL_TESTS();
-}
diff --git a/libs/binder/rust/tests/integration.rs b/libs/binder/rust/tests/integration.rs
index 497361a0ed..953d328d23 100644
--- a/libs/binder/rust/tests/integration.rs
+++ b/libs/binder/rust/tests/integration.rs
@@ -173,30 +173,6 @@ impl ITest for Binder<BnTest> {
     }
 }
 
-/// Trivial testing binder interface
-pub trait ITestSameDescriptor: Interface {}
-
-declare_binder_interface! {
-    ITestSameDescriptor["android.os.ITest"] {
-        native: BnTestSameDescriptor(on_transact_same_descriptor),
-        proxy: BpTestSameDescriptor,
-    }
-}
-
-fn on_transact_same_descriptor(
-    _service: &dyn ITestSameDescriptor,
-    _code: TransactionCode,
-    _data: &Parcel,
-    _reply: &mut Parcel,
-) -> binder::Result<()> {
-    Ok(())
-}
-
-impl ITestSameDescriptor for BpTestSameDescriptor {}
-
-impl ITestSameDescriptor for Binder<BnTestSameDescriptor> {}
-
-
 #[cfg(test)]
 mod tests {
     use selinux_bindgen as selinux_sys;
@@ -209,9 +185,9 @@ mod tests {
     use std::thread;
     use std::time::Duration;
 
-    use binder::{Binder, DeathRecipient, FromIBinder, IBinder, Interface, SpIBinder, StatusCode};
+    use binder::{DeathRecipient, FromIBinder, IBinder, SpIBinder, StatusCode};
 
-    use super::{BnTest, ITest, ITestSameDescriptor, RUST_SERVICE_BINARY, TestService};
+    use super::{ITest, RUST_SERVICE_BINARY};
 
     pub struct ScopedServiceProcess(Child);
 
@@ -459,27 +435,4 @@ mod tests {
             assert_eq!(extension.test().unwrap(), extension_name);
         }
     }
-
-    /// Test re-associating a local binder object with a different class.
-    ///
-    /// This is needed because different binder service (e.g. NDK vs Rust)
-    /// implementations are incompatible and must not be interchanged. A local
-    /// service with the same descriptor string but a different class pointer
-    /// may have been created by an NDK service and is therefore incompatible
-    /// with the Rust service implementation. It must be treated as remote and
-    /// all API calls parceled and sent through transactions.
-    ///
-    /// Further tests of this behavior with the C NDK and Rust API are in
-    /// rust_ndk_interop.rs
-    #[test]
-    fn associate_existing_class() {
-        let service = Binder::new(BnTest(Box::new(TestService {
-            s: "testing_service".to_string(),
-        })));
-
-        // This should succeed although we will have to treat the service as
-        // remote.
-        let _interface: Box<dyn ITestSameDescriptor> = FromIBinder::try_from(service.as_binder())
-            .expect("Could not re-interpret service as the ITestSameDescriptor interface");
-    }
 }
diff --git a/libs/binder/rust/tests/ndk_rust_interop.rs b/libs/binder/rust/tests/ndk_rust_interop.rs
deleted file mode 100644
index 70a6dc0788..0000000000
--- a/libs/binder/rust/tests/ndk_rust_interop.rs
+++ /dev/null
@@ -1,96 +0,0 @@
-/*
- * Copyright (C) 2020 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.
- */
-
-//! Rust Binder NDK interop tests
-
-use std::ffi::CStr;
-use std::os::raw::{c_char, c_int};
-use ::IBinderRustNdkInteropTest::binder::{self, Interface, StatusCode};
-use ::IBinderRustNdkInteropTest::aidl::IBinderRustNdkInteropTest::{
-    BnBinderRustNdkInteropTest, IBinderRustNdkInteropTest,
-};
-use ::IBinderRustNdkInteropTest::aidl::IBinderRustNdkInteropTestOther::{
-    IBinderRustNdkInteropTestOther,
-};
-
-/// Look up the provided AIDL service and call its echo method.
-///
-/// # Safety
-///
-/// service_name must be a valid, non-null C-style string (null-terminated).
-#[no_mangle]
-pub unsafe extern "C" fn rust_call_ndk(service_name: *const c_char) -> c_int {
-    let service_name = CStr::from_ptr(service_name).to_str().unwrap();
-
-    // The Rust class descriptor pointer will not match the NDK one, but the
-    // descriptor strings match so this needs to still associate.
-    let service: Box<dyn IBinderRustNdkInteropTest> = match binder::get_interface(service_name) {
-        Err(e) => {
-            eprintln!("Could not find Ndk service {}: {:?}", service_name, e);
-            return StatusCode::NAME_NOT_FOUND as c_int;
-        }
-        Ok(service) => service,
-    };
-
-    match service.echo("testing") {
-        Ok(s) => if s != "testing" {
-            return StatusCode::BAD_VALUE as c_int;
-        },
-        Err(e) => return e.into(),
-    }
-
-    // Try using the binder service through the wrong interface type
-    let wrong_service: Result<Box<dyn IBinderRustNdkInteropTestOther>, StatusCode> =
-        binder::get_interface(service_name);
-    match wrong_service {
-        Err(e) if e == StatusCode::BAD_TYPE => {}
-        Err(e) => {
-            eprintln!("Trying to use a service via the wrong interface errored with unexpected error {:?}", e);
-            return e as c_int;
-        }
-        Ok(_) => {
-            eprintln!("We should not be allowed to use a service via the wrong interface");
-            return StatusCode::BAD_TYPE as c_int;
-        }
-    }
-
-    StatusCode::OK as c_int
-}
-
-struct Service;
-
-impl Interface for Service {}
-
-impl IBinderRustNdkInteropTest for Service {
-    fn echo(&self, s: &str) -> binder::Result<String> {
-        Ok(s.to_string())
-    }
-}
-
-/// Start the interop Echo test service with the given service name.
-///
-/// # Safety
-///
-/// service_name must be a valid, non-null C-style string (null-terminated).
-#[no_mangle]
-pub unsafe extern "C" fn rust_start_service(service_name: *const c_char) -> c_int {
-    let service_name = CStr::from_ptr(service_name).to_str().unwrap();
-    let service = BnBinderRustNdkInteropTest::new_binder(Service);
-    match binder::add_service(&service_name, service.as_binder()) {
-        Ok(_) => StatusCode::OK as c_int,
-        Err(e) => e as c_int,
-    }
-}