avoid 416 response teardown binding to null pointer

methods/http.cc:640:13: runtime error: reference binding to null pointer
of type 'struct FileFd'

This reference is never used in the cases it has a nullptr, so the
practical difference is non-existent, but its a bug still.

Reported-By: gcc -fsanitize=undefined

diff --git a/methods/http.cc b/methods/http.cc
index a283162a2e2905700fe5793fa78d9811cf87826d..64cfe0f85def9e3b5714811c885f59c82a208f54 100644 (file)
--- a/methods/http.cc
+++ b/methods/http.cc
@@ -497,20 +497,22 @@ APT_PURE Hashes * HttpServerState::GetHashes()                            /*{{{*/
 }
                                                                        /*}}}*/
 // HttpServerState::Die - The server has closed the connection.                /*{{{*/
 }
                                                                        /*}}}*/
 // HttpServerState::Die - The server has closed the connection.                /*{{{*/

-bool HttpServerState::Die(FileFd &File)
+bool HttpServerState::Die(FileFd * const File)

 {
    unsigned int LErrno = errno;
 
    // Dump the buffer to the file
    if (State == ServerState::Data)
    {
 {
    unsigned int LErrno = errno;
 
    // Dump the buffer to the file
    if (State == ServerState::Data)
    {

+      if (File == nullptr)
+        return true;

       // on GNU/kFreeBSD, apt dies on /dev/null because non-blocking
       // can't be set
       // on GNU/kFreeBSD, apt dies on /dev/null because non-blocking
       // can't be set

-      if (File.Name() != "/dev/null")
-        SetNonBlock(File.Fd(),false);
+      if (File->Name() != "/dev/null")
+        SetNonBlock(File->Fd(),false);

       while (In.WriteSpace() == true)
       {
       while (In.WriteSpace() == true)
       {

-        if (In.Write(File.Fd()) == false)
+        if (In.Write(File->Fd()) == false)

            return _error->Errno("write",_("Error writing to the file"));
 
         // Done
            return _error->Errno("write",_("Error writing to the file"));
 
         // Done


@@ -629,7 +631,7 @@ bool HttpServerState::Go(bool ToFile, FileFd * const File)
    if (Res == 0)
    {
       _error->Error(_("Connection timed out"));
    if (Res == 0)
    {
       _error->Error(_("Connection timed out"));

-      return Die(*File);
+      return Die(File);

    }
    
    // Handle server IO
    }
    
    // Handle server IO


@@ -637,14 +639,14 @@ bool HttpServerState::Go(bool ToFile, FileFd * const File)
    {
       errno = 0;
       if (In.Read(ServerFd) == false)
    {
       errno = 0;
       if (In.Read(ServerFd) == false)

-        return Die(*File);
+        return Die(File);

    }
         
    if (ServerFd != -1 && FD_ISSET(ServerFd,&wfds))
    {
       errno = 0;
       if (Out.Write(ServerFd) == false)
    }
         
    if (ServerFd != -1 && FD_ISSET(ServerFd,&wfds))
    {
       errno = 0;
       if (Out.Write(ServerFd) == false)

-        return Die(*File);
+        return Die(File);

    }
 
    // Send data to the file
    }
 
    // Send data to the file

diff --git a/methods/http.h b/methods/http.h
index 5567de715d1abc17d75fac06736679dfeb43444a..ac5f52314c46d7fb3c824c6d109534d5e4503087 100644 (file)
--- a/methods/http.h
+++ b/methods/http.h
@@ -112,7 +112,7 @@ struct HttpServerState: public ServerState
    virtual bool Close() APT_OVERRIDE;
    virtual bool InitHashes(HashStringList const &ExpectedHashes) APT_OVERRIDE;
    virtual Hashes * GetHashes() APT_OVERRIDE;
    virtual bool Close() APT_OVERRIDE;
    virtual bool InitHashes(HashStringList const &ExpectedHashes) APT_OVERRIDE;
    virtual Hashes * GetHashes() APT_OVERRIDE;

-   virtual bool Die(FileFd &File) APT_OVERRIDE;
+   virtual bool Die(FileFd * const File) APT_OVERRIDE;

    virtual bool Flush(FileFd * const File) APT_OVERRIDE;
    virtual bool Go(bool ToFile, FileFd * const File) APT_OVERRIDE;
 
    virtual bool Flush(FileFd * const File) APT_OVERRIDE;
    virtual bool Go(bool ToFile, FileFd * const File) APT_OVERRIDE;
 

diff --git a/methods/https.h b/methods/https.h
index 74b86a24f8016c72ce4e00b87cff8003b3e62b2a..2fa714c7b6a86b143a9b069e91bef0d345dcc68b 100644 (file)
--- a/methods/https.h
+++ b/methods/https.h
@@ -45,7 +45,7 @@ class HttpsServerState : public ServerState
    virtual bool Close() APT_OVERRIDE { return false; }
    virtual bool InitHashes(HashStringList const &ExpectedHashes) APT_OVERRIDE;
    virtual Hashes * GetHashes() APT_OVERRIDE;
    virtual bool Close() APT_OVERRIDE { return false; }
    virtual bool InitHashes(HashStringList const &ExpectedHashes) APT_OVERRIDE;
    virtual Hashes * GetHashes() APT_OVERRIDE;

-   virtual bool Die(FileFd &/*File*/) APT_OVERRIDE { return false; }
+   virtual bool Die(FileFd * const /*File*/) APT_OVERRIDE { return false; }

    virtual bool Flush(FileFd * const /*File*/) APT_OVERRIDE { return false; }
    virtual bool Go(bool /*ToFile*/, FileFd * const /*File*/) APT_OVERRIDE { return false; }
 
    virtual bool Flush(FileFd * const /*File*/) APT_OVERRIDE { return false; }
    virtual bool Go(bool /*ToFile*/, FileFd * const /*File*/) APT_OVERRIDE { return false; }
 

diff --git a/methods/server.h b/methods/server.h
index 1f90f6bd4789364c5c3dac8fe75c11f14634f1d6..af0914b9c6ea9ce25847893d3a6db4f7c1913a2b 100644 (file)
--- a/methods/server.h
+++ b/methods/server.h
@@ -97,7 +97,7 @@ struct ServerState
    virtual bool Close() = 0;
    virtual bool InitHashes(HashStringList const &ExpectedHashes) = 0;
    virtual Hashes * GetHashes() = 0;
    virtual bool Close() = 0;
    virtual bool InitHashes(HashStringList const &ExpectedHashes) = 0;
    virtual Hashes * GetHashes() = 0;

-   virtual bool Die(FileFd &File) = 0;
+   virtual bool Die(FileFd * const File) = 0;

    virtual bool Flush(FileFd * const File) = 0;
    virtual bool Go(bool ToFile, FileFd * const File) = 0;
 
    virtual bool Flush(FileFd * const File) = 0;
    virtual bool Go(bool ToFile, FileFd * const File) = 0;