From adcb3411903abe98b8a4accb373576e1f0c6785f Mon Sep 17 00:00:00 2001
From: Janusz Sobczak <jsobczak@google.com>
Date: Wed, 22 Mar 2023 18:02:32 -0700
Subject: [PATCH] Verify that IO on closed socket returns an error

The test implementation of BluetoothSocket::GetInputStream() would crash if
the socket was closed. That's not ideal because the caller can't verify if the
socket is open or closed.

New test verifies that GetOutputStream() and GetInputStream() always return a
valid reference.

PiperOrigin-RevId: 518722540
---
 internal/platform/bluetooth_classic_test.cc   | 44 +++++++++++++++++++
 .../implementation/g3/bluetooth_classic.cc    |  8 +++-
 .../implementation/g3/bluetooth_classic.h     | 13 ++++++
 3 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/internal/platform/bluetooth_classic_test.cc b/internal/platform/bluetooth_classic_test.cc
index 210b2a23..a5e1c591 100644
--- a/internal/platform/bluetooth_classic_test.cc
+++ b/internal/platform/bluetooth_classic_test.cc
@@ -231,6 +231,50 @@ TEST_F(BluetoothClassicMediumTest, SendData) {
   server_socket.Close();
 }
 
+TEST_F(BluetoothClassicMediumTest, IoOnClosedSocketReturnsError) {
+  adapter_a_->SetScanMode(BluetoothAdapter::ScanMode::kConnectable);
+  CountDownLatch found_latch(1);
+  BluetoothDevice* discovered_device = nullptr;
+  bt_a_->StartDiscovery(DiscoveryCallback{
+      .device_discovered_cb =
+          [this, &found_latch, &discovered_device](BluetoothDevice& device) {
+            NEARBY_LOG(INFO, "Device discovered: %s", device.GetName().c_str());
+            EXPECT_EQ(device.GetName(), adapter_b_->GetName());
+            discovered_device = &device;
+            found_latch.CountDown();
+          },
+  });
+  adapter_b_->SetScanMode(BluetoothAdapter::ScanMode::kConnectableDiscoverable);
+  EXPECT_EQ(adapter_b_->GetScanMode(),
+            BluetoothAdapter::ScanMode::kConnectableDiscoverable);
+  ASSERT_TRUE(found_latch.Await().Ok());
+  std::string service_name{"service"};
+  std::string service_uuid("service-uuid");
+  BluetoothServerSocket server_socket =
+      bt_b_->ListenForService(service_name, service_uuid);
+  ASSERT_TRUE(server_socket.IsValid());
+  {
+    ByteArray data("data");
+    CancellationFlag flag;
+    SingleThreadExecutor server_executor;
+    SingleThreadExecutor client_executor;
+    client_executor.Execute([&, this]() {
+      BluetoothSocket socket_a =
+          bt_a_->ConnectToService(*discovered_device, service_uuid, &flag);
+      ASSERT_TRUE(socket_a.IsValid());
+      socket_a.Close();
+      EXPECT_FALSE(socket_a.GetOutputStream().Write(data).Ok());
+    });
+    server_executor.Execute([&]() {
+      BluetoothSocket socket_b = server_socket.Accept();
+      ASSERT_TRUE(socket_b.IsValid());
+      socket_b.Close();
+      EXPECT_FALSE(socket_b.GetInputStream().Read(data.size()).ok());
+    });
+  }
+  server_socket.Close();
+}
+
 TEST_F(BluetoothClassicMediumTest, ConstructorDestructorWorks) {
   // Make sure we can create functional adapters.
   ASSERT_TRUE(adapter_a_->IsValid());
diff --git a/internal/platform/implementation/g3/bluetooth_classic.cc b/internal/platform/implementation/g3/bluetooth_classic.cc
index 5ffd2ba5..a8d2c93b 100644
--- a/internal/platform/implementation/g3/bluetooth_classic.cc
+++ b/internal/platform/implementation/g3/bluetooth_classic.cc
@@ -21,6 +21,7 @@
 #include "internal/platform/cancellation_flag_listener.h"
 #include "internal/platform/implementation/bluetooth_classic.h"
 #include "internal/platform/implementation/g3/bluetooth_adapter.h"
+#include "internal/platform/input_stream.h"
 #include "internal/platform/logging.h"
 #include "internal/platform/medium_environment.h"
 
@@ -52,8 +53,11 @@ bool BluetoothSocket::IsConnectedLocked() const { return input_ != nullptr; }
 
 InputStream& BluetoothSocket::GetInputStream() {
   auto* remote_socket = GetRemoteSocket();
-  CHECK(remote_socket != nullptr);
-  return remote_socket->GetLocalInputStream();
+  if (remote_socket != nullptr) {
+    return remote_socket->GetLocalInputStream();
+  } else {
+    return invalid_input_stream_;
+  }
 }
 
 OutputStream& BluetoothSocket::GetOutputStream() {
diff --git a/internal/platform/implementation/g3/bluetooth_classic.h b/internal/platform/implementation/g3/bluetooth_classic.h
index f08ff6a4..668f8cec 100644
--- a/internal/platform/implementation/g3/bluetooth_classic.h
+++ b/internal/platform/implementation/g3/bluetooth_classic.h
@@ -92,6 +92,19 @@ class BluetoothSocket : public api::BluetoothSocket {
   // This is a helper for GetOutputStream() method.
   OutputStream& GetLocalOutputStream() ABSL_LOCKS_EXCLUDED(mutex_);
 
+  class InvalidInputStream : public InputStream {
+   public:
+    ExceptionOr<ByteArray> Read(std::int64_t size) override {
+      return ExceptionOr<ByteArray>(Exception::kIo);
+    }
+    ExceptionOr<size_t> Skip(size_t offset) override {
+      return ExceptionOr<size_t>(Exception::kIo);
+    }
+    Exception Close() override { return {Exception::kIo}; }
+  };
+  // Returned to the caller if the remote socket is destroyed.
+  InvalidInputStream invalid_input_stream_;
+
   // Output pipe is initialized by constructor, it remains always valid, until
   // it is closed. it represents output part of a local socket. Input part of a
   // local socket comes from the peer socket, after connection.