address comments

Signed-off-by: Ali Beyad <abeyad@google.com>

contrib/vcl/source/vcl_io_handle.cc

@@ -166,7 +166,7 @@ Api::IoCallUint64Result VclIoHandle::close() {
 
 bool VclIoHandle::isOpen() const { return VCL_SH_VALID(sh_); }
 
-bool VclIoHandle::isConnected() const { return false; }
+bool VclIoHandle::wasConnected() const { return false; }
 
 Api::IoCallUint64Result VclIoHandle::readv(uint64_t max_length, Buffer::RawSlice* slices,
                                            uint64_t num_slice) {

contrib/vcl/source/vcl_io_handle.h

@@ -31,7 +31,7 @@ class VclIoHandle : public Envoy::Network::IoHandle,
   os_fd_t fdDoNotUse() const override { return fd_; }
   Api::IoCallUint64Result close() override;
   bool isOpen() const override;
-  bool isConnected() const override;
+  bool wasConnected() const override;
   Api::IoCallUint64Result readv(uint64_t max_length, Buffer::RawSlice* slices,
                                 uint64_t num_slice) override;
   Api::IoCallUint64Result read(Buffer::Instance& buffer,

envoy/network/io_handle.h

@@ -66,10 +66,10 @@ class IoHandle {
   virtual bool isOpen() const PURE;
 
   /**
-   * Return true if connect() has successfully been called on the socket.
+   * Return true if the socket has had connect() successfully called on it.
    * Use isOpen() to check if the socket is still connected or not.
    */
-  virtual bool isConnected() const PURE;
+  virtual bool wasConnected() const PURE;
 
   /**
    * Read data into given slices.

source/common/network/io_socket_handle_base_impl.cc

@@ -29,7 +29,7 @@ IoSocketHandleBaseImpl::~IoSocketHandleBaseImpl() {
 
 bool IoSocketHandleBaseImpl::isOpen() const { return SOCKET_VALID(fd_); }
 
-bool IoSocketHandleBaseImpl::isConnected() const { return is_connected_; }
+bool IoSocketHandleBaseImpl::wasConnected() const { return was_connected_; }
 
 bool IoSocketHandleBaseImpl::supportsMmsg() const {
   return Api::OsSysCallsSingleton::get().supportsMmsg();

source/common/network/io_socket_handle_base_impl.h

@@ -19,7 +19,7 @@ class IoSocketHandleBaseImpl : public IoHandle, protected Logger::Loggable<Logge
   // TODO(sbelair2)  To be removed when the fd is fully abstracted from clients.
   os_fd_t fdDoNotUse() const override { return fd_; }
   bool isOpen() const override;
-  bool isConnected() const override;
+  bool wasConnected() const override;
   bool supportsMmsg() const override;
   bool supportsUdpGro() const override;
   Api::SysCallIntResult setOption(int level, int optname, const void* optval,
@@ -39,7 +39,7 @@ class IoSocketHandleBaseImpl : public IoHandle, protected Logger::Loggable<Logge
   os_fd_t fd_;
   int socket_v6only_;
   const absl::optional<int> domain_;
-  bool is_connected_ = false;
+  bool was_connected_ = false;
 };
 
 } // namespace Network

source/common/network/io_socket_handle_impl.cc

@@ -541,7 +541,7 @@ Api::SysCallIntResult IoSocketHandleImpl::connect(Address::InstanceConstSharedPt
 
   auto result = Api::OsSysCallsSingleton::get().connect(fd_, sockaddr_to_use, sockaddr_len_to_use);
   if (result.return_value_ != -1) {
-    is_connected_ = true;
+    was_connected_ = true;
   }
   return result;
 }

source/common/network/utility.cc

@@ -516,11 +516,11 @@ Api::IoCallUint64Result Utility::writeToSocket(IoHandle& handle, Buffer::RawSlic
   Api::IoCallUint64Result send_result(
       /*rc=*/0, /*err=*/Api::IoError::none());
 
-  const bool is_connected = handle.isConnected();
+  const bool is_connected = handle.wasConnected();
   do {
     if (is_connected) {
       // The socket is already connected, so the local and peer addresses should not be specified.
-      // Instead, a writev/send is called.
+      // Instead, a writev is called.
       send_result = handle.writev(slices, num_slices);
     } else {
       // For non-connected sockets(), calling sendmsg with the peer address specified ensures the

source/common/quic/quic_io_handle_wrapper.h

@@ -24,7 +24,7 @@ class QuicIoHandleWrapper : public Network::IoHandle {
     return Api::ioCallUint64ResultNoError();
   }
   bool isOpen() const override { return !closed_; }
-  bool isConnected() const override { return io_handle_.isConnected(); }
+  bool wasConnected() const override { return io_handle_.wasConnected(); }
   Api::IoCallUint64Result readv(uint64_t max_length, Buffer::RawSlice* slices,
                                 uint64_t num_slice) override {
     if (closed_) {

source/extensions/io_socket/user_space/io_handle_impl.cc

@@ -90,7 +90,7 @@ Api::IoCallUint64Result IoHandleImpl::close() {
 
 bool IoHandleImpl::isOpen() const { return !closed_; }
 
-bool IoHandleImpl::isConnected() const { return false; }
+bool IoHandleImpl::wasConnected() const { return false; }
 
 Api::IoCallUint64Result IoHandleImpl::readv(uint64_t max_length, Buffer::RawSlice* slices,
                                             uint64_t num_slice) {

source/extensions/io_socket/user_space/io_handle_impl.h

@@ -49,7 +49,7 @@ class IoHandleImpl final : public Network::IoHandle,
   }
   Api::IoCallUint64Result close() override;
   bool isOpen() const override;
-  bool isConnected() const override;
+  bool wasConnected() const override;
   Api::IoCallUint64Result readv(uint64_t max_length, Buffer::RawSlice* slices,
                                 uint64_t num_slice) override;
   Api::IoCallUint64Result read(Buffer::Instance& buffer,

test/common/quic/client_connection_factory_impl_test.cc

@@ -23,6 +23,8 @@ using testing::Return;
 namespace Envoy {
 namespace Quic {
 
+constexpr int PEER_PORT = 54321;
+
 class QuicNetworkConnectionTest : public Event::TestUsingSimulatedTime,
                                   public testing::TestWithParam<Network::Address::IpVersion> {
 protected:
@@ -60,8 +62,8 @@ class QuicNetworkConnectionTest : public Event::TestUsingSimulatedTime,
     }
     EXPECT_EQ(quic_ccopts, "6RTOAKD4");
 
-    test_address_ = *Network::Utility::resolveUrl(
-        absl::StrCat("tcp://", Network::Test::getLoopbackAddressUrlString(GetParam()), ":30"));
+    test_address_ = *Network::Utility::resolveUrl(absl::StrCat(
+        "tcp://", Network::Test::getLoopbackAddressUrlString(GetParam()), ":", PEER_PORT));
     Ssl::ClientContextSharedPtr context{new Ssl::MockClientContext()};
     EXPECT_CALL(context_.context_manager_, createSslClientContext(_, _)).WillOnce(Return(context));
     factory_ = *Quic::QuicClientTransportSocketFactory::create(
@@ -94,10 +96,9 @@ class QuicNetworkConnectionTest : public Event::TestUsingSimulatedTime,
 
 TEST_P(QuicNetworkConnectionTest, BufferLimits) {
   initialize();
-  const int port = 30;
   std::unique_ptr<Network::ClientConnection> client_connection = createQuicNetworkConnection(
       *quic_info_, crypto_config_,
-      quic::QuicServerId{factory_->clientContextConfig()->serverNameIndication(), port, false},
+      quic::QuicServerId{factory_->clientContextConfig()->serverNameIndication(), PEER_PORT, false},
       dispatcher_, test_address_, test_address_, quic_stat_names_, {}, *store_.rootScope(), nullptr,
       nullptr, connection_id_generator_, *factory_);
   EnvoyQuicClientSession* session = static_cast<EnvoyQuicClientSession*>(client_connection.get());
@@ -118,14 +119,13 @@ TEST_P(QuicNetworkConnectionTest, SocketOptions) {
   auto socket_option = std::make_shared<Network::MockSocketOption>();
   auto socket_options = std::make_shared<Network::ConnectionSocket::Options>();
   socket_options->push_back(socket_option);
-  const int port = 30;
   EXPECT_CALL(*socket_option, setOption(_, envoy::config::core::v3::SocketOption::STATE_PREBIND));
   EXPECT_CALL(*socket_option, setOption(_, envoy::config::core::v3::SocketOption::STATE_BOUND));
   EXPECT_CALL(*socket_option, setOption(_, envoy::config::core::v3::SocketOption::STATE_LISTENING));
 
   std::unique_ptr<Network::ClientConnection> client_connection = createQuicNetworkConnection(
       *quic_info_, crypto_config_,
-      quic::QuicServerId{factory_->clientContextConfig()->serverNameIndication(), port, false},
+      quic::QuicServerId{factory_->clientContextConfig()->serverNameIndication(), PEER_PORT, false},
       dispatcher_, test_address_, test_address_, quic_stat_names_, {}, *store_.rootScope(),
       socket_options, nullptr, connection_id_generator_, *factory_);
   EnvoyQuicClientSession* session = static_cast<EnvoyQuicClientSession*>(client_connection.get());
@@ -136,14 +136,13 @@ TEST_P(QuicNetworkConnectionTest, SocketOptions) {
 
 TEST_P(QuicNetworkConnectionTest, LocalAddress) {
   initialize();
-  const int port = 30;
   Network::Address::InstanceConstSharedPtr local_addr =
       (GetParam() == Network::Address::IpVersion::v6)
           ? Network::Utility::getIpv6LoopbackAddress()
           : Network::Utility::getCanonicalIpv4LoopbackAddress();
   std::unique_ptr<Network::ClientConnection> client_connection = createQuicNetworkConnection(
       *quic_info_, crypto_config_,
-      quic::QuicServerId{factory_->clientContextConfig()->serverNameIndication(), port, false},
+      quic::QuicServerId{factory_->clientContextConfig()->serverNameIndication(), PEER_PORT, false},
       dispatcher_, test_address_, local_addr, quic_stat_names_, {}, *store_.rootScope(), nullptr,
       nullptr, connection_id_generator_, *factory_);
   EnvoyQuicClientSession* session = static_cast<EnvoyQuicClientSession*>(client_connection.get());
@@ -163,10 +162,9 @@ TEST_P(QuicNetworkConnectionTest, Srtt) {
 
   EXPECT_CALL(rtt_cache, getSrtt).WillOnce(Return(std::chrono::microseconds(5)));
 
-  const int port = 30;
   std::unique_ptr<Network::ClientConnection> client_connection = createQuicNetworkConnection(
       info, crypto_config_,
-      quic::QuicServerId{factory_->clientContextConfig()->serverNameIndication(), port, false},
+      quic::QuicServerId{factory_->clientContextConfig()->serverNameIndication(), PEER_PORT, false},
       dispatcher_, test_address_, test_address_, quic_stat_names_, rtt_cache, *store_.rootScope(),
       nullptr, nullptr, connection_id_generator_, *factory_);
 

test/common/quic/envoy_quic_utils_test.cc

@@ -289,13 +289,13 @@ TEST(EnvoyQuicUtilsTest, CreateConnectionSocket) {
       std::make_shared<Network::Address::Ipv4Instance>("127.0.0.1", 54321, nullptr);
   auto connection_socket = createConnectionSocket(peer_addr, local_addr, nullptr);
   EXPECT_TRUE(connection_socket->isOpen());
-  EXPECT_TRUE(connection_socket->ioHandle().isConnected());
+  EXPECT_TRUE(connection_socket->ioHandle().wasConnected());
   connection_socket->close();
 
   Network::Address::InstanceConstSharedPtr no_local_addr = nullptr;
   connection_socket = createConnectionSocket(peer_addr, no_local_addr, nullptr);
   EXPECT_TRUE(connection_socket->isOpen());
-  EXPECT_TRUE(connection_socket->ioHandle().isConnected());
+  EXPECT_TRUE(connection_socket->ioHandle().wasConnected());
   EXPECT_EQ("127.0.0.1", no_local_addr->ip()->addressAsString());
   connection_socket->close();
 
@@ -305,13 +305,13 @@ TEST(EnvoyQuicUtilsTest, CreateConnectionSocket) {
       std::make_shared<Network::Address::Ipv6Instance>("::1", 54321, nullptr);
   connection_socket = createConnectionSocket(peer_addr_v6, local_addr_v6, nullptr);
   EXPECT_TRUE(connection_socket->isOpen());
-  EXPECT_TRUE(connection_socket->ioHandle().isConnected());
+  EXPECT_TRUE(connection_socket->ioHandle().wasConnected());
   connection_socket->close();
 
   Network::Address::InstanceConstSharedPtr no_local_addr_v6 = nullptr;
   connection_socket = createConnectionSocket(peer_addr_v6, no_local_addr_v6, nullptr);
   EXPECT_TRUE(connection_socket->isOpen());
-  EXPECT_TRUE(connection_socket->ioHandle().isConnected());
+  EXPECT_TRUE(connection_socket->ioHandle().wasConnected());
   EXPECT_EQ("::1", no_local_addr_v6->ip()->addressAsString());
   connection_socket->close();
 }

test/extensions/filters/udp/udp_proxy/udp_proxy_filter_test.cc

@@ -117,7 +117,7 @@ class UdpProxyFilterTest : public UdpProxyFilterBase {
       }
       if (!connect_sys_errno) {
         if (expect_connect) {
-          EXPECT_CALL(*socket_->io_handle_, isConnected()).WillOnce(Return(true));
+          EXPECT_CALL(*socket_->io_handle_, wasConnected()).WillOnce(Return(true));
           EXPECT_CALL(*socket_->io_handle_, writev(_, 1))
               .WillOnce(Invoke([data, sys_errno](const Buffer::RawSlice* slices,
                                                  uint64_t) -> Api::IoCallUint64Result {
@@ -132,7 +132,7 @@ class UdpProxyFilterTest : public UdpProxyFilterBase {
                 }
               }));
         } else {
-          EXPECT_CALL(*socket_->io_handle_, isConnected()).WillOnce(Return(false));
+          EXPECT_CALL(*socket_->io_handle_, wasConnected()).WillOnce(Return(false));
           EXPECT_CALL(*socket_->io_handle_, sendmsg(_, 1, 0, _, _))
               .WillOnce(Invoke(
                   [this, data, local_ip, sys_errno](

test/extensions/upstreams/http/udp/upstream_request_test.cc

@@ -83,7 +83,7 @@ TEST_F(UdpUpstreamTest, ExchangeCapsules) {
                              "a1a2a3a4a5a6a7" // UDP Proxying Payload
       );
   Buffer::OwnedImpl sent_capsule(sent_capsule_fragment);
-  EXPECT_CALL(*mock_socket_->io_handle_, isConnected()).WillOnce(Return(true));
+  EXPECT_CALL(*mock_socket_->io_handle_, wasConnected()).WillOnce(Return(true));
   EXPECT_CALL(*mock_socket_->io_handle_, writev(_, _))
       .WillOnce([](const Buffer::RawSlice* slices, uint64_t num_slice) {
         Buffer::OwnedImpl buffer(absl::HexStringToBytes("a1a2a3a4a5a6a7"));

test/mocks/network/io_handle.h

@@ -22,7 +22,7 @@ class MockIoHandle : public IoHandle {
   MOCK_METHOD(os_fd_t, fdDoNotUse, (), (const));
   MOCK_METHOD(Api::IoCallUint64Result, close, ());
   MOCK_METHOD(bool, isOpen, (), (const));
-  MOCK_METHOD(bool, isConnected, (), (const));
+  MOCK_METHOD(bool, wasConnected, (), (const));
   MOCK_METHOD(Api::IoCallUint64Result, readv,
               (uint64_t max_length, Buffer::RawSlice* slices, uint64_t num_slice));
   MOCK_METHOD(Api::IoCallUint64Result, read,