methods: connect: Switch from int fds to new MethodFd

Use std::unique_ptr<MethodFd> everywhere we used an
integer-based file descriptor before. This allows us
to implement stuff like TLS support easily.

Gbp-Dch: ignore

6 files changed, 126 insertions, 72 deletions

diff --git a/methods/connect.cc b/methods/connect.cc
index d5e40fbab..9e9e09ac4 100644
--- a/methods/connect.cc
+++ b/methods/connect.cc
@@ -77,16 +77,42 @@ static bool ConnectionAllowed(char const * const Service, std::string const &Hos
    return true;
 }
 									/*}}}*/
+
+// File Descriptor based Fd /*{{{*/
+struct FdFd : public MethodFd
+{
+   int fd = -1;
+   int Fd() APT_OVERRIDE { return fd; }
+   ssize_t Read(void *buf, size_t count) APT_OVERRIDE { return ::read(fd, buf, count); }
+   ssize_t Write(void *buf, size_t count) APT_OVERRIDE { return ::write(fd, buf, count); }
+   int Close() APT_OVERRIDE
+   {
+      int result = 0;
+      if (fd != -1)
+	 result = ::close(fd);
+      fd = -1;
+      return result;
+   }
+};
+
+std::unique_ptr<MethodFd> MethodFd::FromFd(int iFd)
+{
+   FdFd *fd = new FdFd();
+   fd->fd = iFd;
+   return std::unique_ptr<MethodFd>(fd);
+}
+									/*}}}*/
 // DoConnect - Attempt a connect operation				/*{{{*/
 // ---------------------------------------------------------------------
 /* This helper function attempts a connection to a single address. */
 static bool DoConnect(struct addrinfo *Addr, std::string const &Host,
-		      unsigned long TimeOut, int &Fd, aptMethod *Owner)
+		      unsigned long TimeOut, std::unique_ptr<MethodFd> &Fd, aptMethod *Owner)
 {
    // Show a status indicator
    char Name[NI_MAXHOST];
    char Service[NI_MAXSERV];
-   
+   Fd.reset(new FdFd());
+
    Name[0] = 0;   
    Service[0] = 0;
    getnameinfo(Addr->ai_addr,Addr->ai_addrlen,
@@ -108,20 +134,21 @@ static bool DoConnect(struct addrinfo *Addr, std::string const &Host,
    }
       
    // Get a socket
-   if ((Fd = socket(Addr->ai_family,Addr->ai_socktype,
-		    Addr->ai_protocol)) < 0)
+   if ((static_cast<FdFd *>(Fd.get())->fd = socket(Addr->ai_family, Addr->ai_socktype,
+						   Addr->ai_protocol)) < 0)
       return _error->Errno("socket",_("Could not create a socket for %s (f=%u t=%u p=%u)"),
 			   Name,Addr->ai_family,Addr->ai_socktype,Addr->ai_protocol);
-   
-   SetNonBlock(Fd,true);
-   if (connect(Fd,Addr->ai_addr,Addr->ai_addrlen) < 0 &&
+
+   SetNonBlock(Fd->Fd(), true);
+   if (connect(Fd->Fd(), Addr->ai_addr, Addr->ai_addrlen) < 0 &&
        errno != EINPROGRESS)
       return _error->Errno("connect",_("Cannot initiate the connection "
 			   "to %s:%s (%s)."),Host.c_str(),Service,Name);
    
    /* This implements a timeout for connect by opening the connection
       nonblocking */
-   if (WaitFd(Fd,true,TimeOut) == false) {
+   if (WaitFd(Fd->Fd(), true, TimeOut) == false)
+   {
       bad_addr.insert(bad_addr.begin(), std::string(Name));
       Owner->SetFailReason("Timeout");
       return _error->Error(_("Could not connect to %s:%s (%s), "
@@ -131,7 +158,7 @@ static bool DoConnect(struct addrinfo *Addr, std::string const &Host,
    // Check the socket for an error condition
    unsigned int Err;
    unsigned int Len = sizeof(Err);
-   if (getsockopt(Fd,SOL_SOCKET,SO_ERROR,&Err,&Len) != 0)
+   if (getsockopt(Fd->Fd(), SOL_SOCKET, SO_ERROR, &Err, &Len) != 0)
       return _error->Errno("getsockopt",_("Failed"));
    
    if (Err != 0)
@@ -151,7 +178,7 @@ static bool DoConnect(struct addrinfo *Addr, std::string const &Host,
 									/*}}}*/
 // Connect to a given Hostname						/*{{{*/
 static bool ConnectToHostname(std::string const &Host, int const Port,
-			      const char *const Service, int DefPort, int &Fd,
+			      const char *const Service, int DefPort, std::unique_ptr<MethodFd> &Fd,
 			      unsigned long const TimeOut, aptMethod *const Owner)
 {
    if (ConnectionAllowed(Service, Host) == false)
@@ -255,10 +282,9 @@ static bool ConnectToHostname(std::string const &Host, int const Port,
       {
 	 LastUsed = CurHost;
 	 return true;
-      }      
-      close(Fd);
-      Fd = -1;
-      
+      }
+      Fd->Close();
+
       // Ignore UNIX domain sockets
       do
       {
@@ -288,7 +314,7 @@ static bool ConnectToHostname(std::string const &Host, int const Port,
 // ---------------------------------------------------------------------
 /* Performs a connection to the server (including SRV record lookup) */
 bool Connect(std::string Host, int Port, const char *Service,
-	     int DefPort, int &Fd,
+	     int DefPort, std::unique_ptr<MethodFd> &Fd,
 	     unsigned long TimeOut, aptMethod *Owner)
 {
    if (_error->PendingError() == true)
diff --git a/methods/connect.h b/methods/connect.h
index 44473a07c..5eae77d09 100644
--- a/methods/connect.h
+++ b/methods/connect.h
@@ -10,12 +10,35 @@
 #ifndef CONNECT_H
 #define CONNECT_H
 
+#include <memory>
+#include <stddef.h>
 #include <string>
 
 class aptMethod;
 
+/**
+ * \brief Small representation of a file descriptor for network traffic.
+ *
+ * This provides support for TLS, SOCKS, and HTTP CONNECT proxies.
+ */
+struct MethodFd
+{
+   /// \brief Returns -1 for unusable, or an fd to select() on otherwise
+   virtual int Fd() = 0;
+   /// \brief Should behave like read(2)
+   virtual ssize_t Read(void *buf, size_t count) = 0;
+   /// \brief Should behave like write(2)
+   virtual ssize_t Write(void *buf, size_t count) = 0;
+   /// \brief Closes the file descriptor. Can be called multiple times.
+   virtual int Close() = 0;
+   /// \brief Destructor
+   virtual ~MethodFd(){};
+   /// \brief Construct a MethodFd from a UNIX file descriptor
+   static std::unique_ptr<MethodFd> FromFd(int iFd);
+};
+
 bool Connect(std::string To, int Port, const char *Service, int DefPort,
-	     int &Fd, unsigned long TimeOut, aptMethod *Owner);
+	     std::unique_ptr<MethodFd> &Fd, unsigned long TimeOut, aptMethod *Owner);
 
 void RotateDNS();
 
diff --git a/methods/ftp.cc b/methods/ftp.cc
index 5b30b5486..f4986f648 100644
--- a/methods/ftp.cc
+++ b/methods/ftp.cc
@@ -73,8 +73,8 @@ time_t FtpMethod::FailTime = 0;
 // FTPConn::FTPConn - Constructor					/*{{{*/
 // ---------------------------------------------------------------------
 /* */
-FTPConn::FTPConn(URI Srv) : Len(0), ServerFd(-1), DataFd(-1),
-                            DataListenFd(-1), ServerName(Srv),
+FTPConn::FTPConn(URI Srv) : Len(0), ServerFd(MethodFd::FromFd(-1)), DataFd(-1),
+			    DataListenFd(-1), ServerName(Srv),
 			    ForceExtended(false), TryPassive(true),
 			    PeerAddrLen(0), ServerAddrLen(0)
 {
@@ -96,8 +96,7 @@ FTPConn::~FTPConn()
 /* Just tear down the socket and data socket */
 void FTPConn::Close()
 {
-   close(ServerFd);
-   ServerFd = -1;
+   ServerFd->Close();
    close(DataFd);
    DataFd = -1;
    close(DataListenFd);
@@ -115,7 +114,7 @@ void FTPConn::Close()
 bool FTPConn::Open(aptMethod *Owner)
 {
    // Use the already open connection if possible.
-   if (ServerFd != -1)
+   if (ServerFd->Fd() != -1)
       return true;
    
    Close();
@@ -178,12 +177,12 @@ bool FTPConn::Open(aptMethod *Owner)
    
    // Get the remote server's address
    PeerAddrLen = sizeof(PeerAddr);
-   if (getpeername(ServerFd,(sockaddr *)&PeerAddr,&PeerAddrLen) != 0)
+   if (getpeername(ServerFd->Fd(), (sockaddr *)&PeerAddr, &PeerAddrLen) != 0)
       return _error->Errno("getpeername",_("Unable to determine the peer name"));
    
    // Get the local machine's address
    ServerAddrLen = sizeof(ServerAddr);
-   if (getsockname(ServerFd,(sockaddr *)&ServerAddr,&ServerAddrLen) != 0)
+   if (getsockname(ServerFd->Fd(), (sockaddr *)&ServerAddr, &ServerAddrLen) != 0)
       return _error->Errno("getsockname",_("Unable to determine the local name"));
    
    return Res;
@@ -314,7 +313,7 @@ bool FTPConn::Login()
 /* This performs a very simple buffered read. */
 bool FTPConn::ReadLine(string &Text)
 {
-   if (ServerFd == -1)
+   if (ServerFd->Fd() == -1)
       return false;
    
    // Suck in a line
@@ -339,14 +338,14 @@ bool FTPConn::ReadLine(string &Text)
       }
 
       // Wait for some data..
-      if (WaitFd(ServerFd,false,TimeOut) == false)
+      if (WaitFd(ServerFd->Fd(), false, TimeOut) == false)
       {
 	 Close();
 	 return _error->Error(_("Connection timeout"));
       }
       
       // Suck it back
-      int Res = read(ServerFd,Buffer + Len,sizeof(Buffer) - Len);
+      int Res = ServerFd->Read(Buffer + Len, sizeof(Buffer) - Len);
       if (Res == 0)
 	 _error->Error(_("Server closed the connection"));
       if (Res <= 0)
@@ -451,13 +450,13 @@ bool FTPConn::WriteMsg(unsigned int &Ret,string &Text,const char *Fmt,...)
    unsigned long Start = 0;
    while (Len != 0)
    {
-      if (WaitFd(ServerFd,true,TimeOut) == false)
+      if (WaitFd(ServerFd->Fd(), true, TimeOut) == false)
       {
 	 Close();
 	 return _error->Error(_("Connection timeout"));
       }
-      
-      int Res = write(ServerFd,S + Start,Len);
+
+      int Res = ServerFd->Write(S + Start, Len);
       if (Res <= 0)
       {
 	 _error->Errno("write",_("Write error"));
diff --git a/methods/ftp.h b/methods/ftp.h
index 3a482fa42..5b23dba41 100644
--- a/methods/ftp.h
+++ b/methods/ftp.h
@@ -10,8 +10,9 @@
 #ifndef APT_FTP_H
 #define APT_FTP_H
 
-#include <apt-pkg/strutl.h>
 #include "aptmethod.h"
+#include "connect.h"
+#include <apt-pkg/strutl.h>
 
 #include <sys/socket.h>
 #include <sys/types.h>
@@ -22,7 +23,7 @@ class FTPConn
 {
    char Buffer[1024*10];
    unsigned long Len;
-   int ServerFd;
+   std::unique_ptr<MethodFd> ServerFd;
    int DataFd;
    int DataListenFd;
    URI ServerName;
diff --git a/methods/http.cc b/methods/http.cc
index 9f5959548..d62f97be3 100644
--- a/methods/http.cc
+++ b/methods/http.cc
@@ -95,7 +95,7 @@ void CircleBuf::Reset()
 // ---------------------------------------------------------------------
 /* This fills up the buffer with as much data as is in the FD, assuming it
    is non-blocking.. */
-bool CircleBuf::Read(int Fd)
+bool CircleBuf::Read(std::unique_ptr<MethodFd> const &Fd)
 {
    while (1)
    {
@@ -126,11 +126,11 @@ bool CircleBuf::Read(int Fd)
       // Write the buffer segment
       ssize_t Res;
       if(CircleBuf::BwReadLimit) {
-	 Res = read(Fd,Buf + (InP%Size), 
-		    BwReadMax > LeftRead() ? LeftRead() : BwReadMax);
+	 Res = Fd->Read(Buf + (InP % Size),
+			BwReadMax > LeftRead() ? LeftRead() : BwReadMax);
       } else
-	 Res = read(Fd,Buf + (InP%Size),LeftRead());
-      
+	 Res = Fd->Read(Buf + (InP % Size), LeftRead());
+
       if(Res > 0 && BwReadLimit > 0) 
 	 CircleBuf::BwTickReadData += Res;
     
@@ -193,7 +193,7 @@ void CircleBuf::FillOut()
 // CircleBuf::Write - Write from the buffer into a FD			/*{{{*/
 // ---------------------------------------------------------------------
 /* This empties the buffer into the FD. */
-bool CircleBuf::Write(int Fd)
+bool CircleBuf::Write(std::unique_ptr<MethodFd> const &Fd)
 {
    while (1)
    {
@@ -208,7 +208,7 @@ bool CircleBuf::Write(int Fd)
       
       // Write the buffer segment
       ssize_t Res;
-      Res = write(Fd,Buf + (OutP%Size),LeftWrite());
+      Res = Fd->Write(Buf + (OutP % Size), LeftWrite());
 
       if (Res == 0)
 	 return false;
@@ -291,6 +291,7 @@ CircleBuf::~CircleBuf()
 HttpServerState::HttpServerState(URI Srv,HttpMethod *Owner) : ServerState(Srv, Owner), In(Owner, 64*1024), Out(Owner, 4*1024)
 {
    TimeOut = Owner->ConfigFindI("Timeout", TimeOut);
+   ServerFd = MethodFd::FromFd(-1);
    Reset();
 }
 									/*}}}*/
@@ -318,7 +319,7 @@ static bool TalkToSocksProxy(int const ServerFd, std::string const &Proxy,
 bool HttpServerState::Open()
 {
    // Use the already open connection if possible.
-   if (ServerFd != -1)
+   if (ServerFd->Fd() != -1)
       return true;
    
    Close();
@@ -371,8 +372,12 @@ bool HttpServerState::Open()
       std::string const ProxyInfo = URI::SiteOnly(Proxy);
       Owner->Status(_("Connecting to %s (%s)"),"SOCKS5h proxy",ProxyInfo.c_str());
       auto const Timeout = Owner->ConfigFindI("TimeOut", 120);
-      #define APT_WriteOrFail(TYPE, DATA, LENGTH) if (TalkToSocksProxy(ServerFd, ProxyInfo, TYPE, true, DATA, LENGTH, Timeout) == false) return false
-      #define APT_ReadOrFail(TYPE, DATA, LENGTH) if (TalkToSocksProxy(ServerFd, ProxyInfo, TYPE, false, DATA, LENGTH, Timeout) == false) return false
+#define APT_WriteOrFail(TYPE, DATA, LENGTH)                                                     \
+   if (TalkToSocksProxy(ServerFd->Fd(), ProxyInfo, TYPE, true, DATA, LENGTH, Timeout) == false) \
+   return false
+#define APT_ReadOrFail(TYPE, DATA, LENGTH)                                                       \
+   if (TalkToSocksProxy(ServerFd->Fd(), ProxyInfo, TYPE, false, DATA, LENGTH, Timeout) == false) \
+   return false
       if (ServerName.Host.length() > 255)
 	 return _error->Error("Can't use SOCKS5h as hostname %s is too long!", ServerName.Host.c_str());
       if (Proxy.User.length() > 255 || Proxy.Password.length() > 255)
@@ -514,7 +519,7 @@ bool HttpServerState::Open()
 	 ioprintf(std::clog, "http: SOCKS proxy %s connection established to %s (%s)\n",
 	       ProxyInfo.c_str(), ServerName.Host.c_str(), bindaddr.c_str());
 
-      if (WaitFd(ServerFd, true, Timeout) == false)
+      if (WaitFd(ServerFd->Fd(), true, Timeout) == false)
 	 return _error->Error("SOCKS proxy %s reported connection to %s (%s), but timed out",
 	       ProxyInfo.c_str(), ServerName.Host.c_str(), bindaddr.c_str());
       #undef APT_ReadOrFail
@@ -549,8 +554,7 @@ bool HttpServerState::Open()
 /* */
 bool HttpServerState::Close()
 {
-   close(ServerFd);
-   ServerFd = -1;
+   ServerFd->Close();
    return true;
 }
 									/*}}}*/
@@ -672,7 +676,7 @@ bool HttpServerState::WriteResponse(const std::string &Data)		/*{{{*/
 									/*}}}*/
 APT_PURE bool HttpServerState::IsOpen()					/*{{{*/
 {
-   return (ServerFd != -1);
+   return (ServerFd->Fd() != -1);
 }
 									/*}}}*/
 bool HttpServerState::InitHashes(HashStringList const &ExpectedHashes)	/*{{{*/
@@ -685,7 +689,7 @@ bool HttpServerState::InitHashes(HashStringList const &ExpectedHashes)	/*{{{*/
 void HttpServerState::Reset()						/*{{{*/
 {
    ServerState::Reset();
-   ServerFd = -1;
+   ServerFd->Close();
 }
 									/*}}}*/
 
@@ -710,7 +714,7 @@ bool HttpServerState::Die(RequestState &Req)
 	 SetNonBlock(Req.File.Fd(),false);
       while (In.WriteSpace() == true)
       {
-	 if (In.Write(Req.File.Fd()) == false)
+	 if (In.Write(MethodFd::FromFd(Req.File.Fd())) == false)
 	    return _error->Errno("write",_("Error writing to the file"));
 
 	 // Done
@@ -762,7 +766,7 @@ bool HttpServerState::Flush(FileFd * const File)
       
       while (In.WriteSpace() == true)
       {
-	 if (In.Write(File->Fd()) == false)
+	 if (In.Write(MethodFd::FromFd(File->Fd())) == false)
 	    return _error->Errno("write",_("Error writing to file"));
 	 if (In.IsLimit() == true)
 	    return true;
@@ -781,8 +785,8 @@ bool HttpServerState::Flush(FileFd * const File)
 bool HttpServerState::Go(bool ToFile, RequestState &Req)
 {
    // Server has closed the connection
-   if (ServerFd == -1 && (In.WriteSpace() == false || 
-			       ToFile == false))
+   if (ServerFd->Fd() == -1 && (In.WriteSpace() == false ||
+				ToFile == false))
       return false;
    
    fd_set rfds,wfds;
@@ -791,28 +795,27 @@ bool HttpServerState::Go(bool ToFile, RequestState &Req)
    
    /* Add the server. We only send more requests if the connection will 
       be persisting */
-   if (Out.WriteSpace() == true && ServerFd != -1 
-       && Persistent == true)
-      FD_SET(ServerFd,&wfds);
-   if (In.ReadSpace() == true && ServerFd != -1)
-      FD_SET(ServerFd,&rfds);
-   
+   if (Out.WriteSpace() == true && ServerFd->Fd() != -1 && Persistent == true)
+      FD_SET(ServerFd->Fd(), &wfds);
+   if (In.ReadSpace() == true && ServerFd->Fd() != -1)
+      FD_SET(ServerFd->Fd(), &rfds);
+
    // Add the file
-   int FileFD = -1;
+   auto FileFD = MethodFd::FromFd(-1);
    if (Req.File.IsOpen())
-      FileFD = Req.File.Fd();
-   
-   if (In.WriteSpace() == true && ToFile == true && FileFD != -1)
-      FD_SET(FileFD,&wfds);
+      FileFD = MethodFd::FromFd(Req.File.Fd());
+
+   if (In.WriteSpace() == true && ToFile == true && FileFD->Fd() != -1)
+      FD_SET(FileFD->Fd(), &wfds);
 
    // Add stdin
    if (Owner->ConfigFindB("DependOnSTDIN", true) == true)
       FD_SET(STDIN_FILENO,&rfds);
 	  
    // Figure out the max fd
-   int MaxFd = FileFD;
-   if (MaxFd < ServerFd)
-      MaxFd = ServerFd;
+   int MaxFd = FileFD->Fd();
+   if (MaxFd < ServerFd->Fd())
+      MaxFd = ServerFd->Fd();
 
    // Select
    struct timeval tv;
@@ -833,14 +836,14 @@ bool HttpServerState::Go(bool ToFile, RequestState &Req)
    }
    
    // Handle server IO
-   if (ServerFd != -1 && FD_ISSET(ServerFd,&rfds))
+   if (ServerFd->Fd() != -1 && FD_ISSET(ServerFd->Fd(), &rfds))
    {
       errno = 0;
       if (In.Read(ServerFd) == false)
 	 return Die(Req);
    }
-	 
-   if (ServerFd != -1 && FD_ISSET(ServerFd,&wfds))
+
+   if (ServerFd->Fd() != -1 && FD_ISSET(ServerFd->Fd(), &wfds))
    {
       errno = 0;
       if (Out.Write(ServerFd) == false)
@@ -848,7 +851,7 @@ bool HttpServerState::Go(bool ToFile, RequestState &Req)
    }
 
    // Send data to the file
-   if (FileFD != -1 && FD_ISSET(FileFD,&wfds))
+   if (FileFD->Fd() != -1 && FD_ISSET(FileFD->Fd(), &wfds))
    {
       if (In.Write(FileFD) == false)
 	 return _error->Errno("write",_("Error writing to output file"));
diff --git a/methods/http.h b/methods/http.h
index c79a6454e..3336fb780 100644
--- a/methods/http.h
+++ b/methods/http.h
@@ -13,11 +13,13 @@
 
 #include <apt-pkg/strutl.h>
 
+#include <iostream>
+#include <memory>
 #include <string>
 #include <sys/time.h>
-#include <iostream>
 
 #include "basehttp.h"
+#include "connect.h"
 
 using std::cout;
 using std::endl;
@@ -66,11 +68,11 @@ class CircleBuf
    unsigned long long TotalWriten;
 
    // Read data in
-   bool Read(int Fd);
+   bool Read(std::unique_ptr<MethodFd> const &Fd);
    bool Read(std::string const &Data);
 
    // Write data out
-   bool Write(int Fd);
+   bool Write(std::unique_ptr<MethodFd> const &Fd);
    bool WriteTillEl(std::string &Data,bool Single = false);
 
    // Control the write limit
@@ -95,7 +97,7 @@ struct HttpServerState: public ServerState
    // This is the connection itself. Output is data FROM the server
    CircleBuf In;
    CircleBuf Out;
-   int ServerFd;
+   std::unique_ptr<MethodFd> ServerFd;
 
    protected:
    virtual bool ReadHeaderLines(std::string &Data) APT_OVERRIDE;