Browse Source

Do not read stderr from proxy autodetection scripts

This fixes a regression introduced in
  commit 8f858d560e

  don't leak FD in AutoProxyDetect command return parsing

which accidentally made the proxy autodetection code also read
the scripts output on stderr, not only on stdout when it switched
the code from popen() to Popen().

Reported-By: Tim Small <tim@seoss.co.uk>
debian/1.8.y
Julian Andres Klode 6 years ago
parent
commit
0ecceb5bb9
  1. 8
      apt-pkg/contrib/fileutl.cc
  2. 3
      apt-pkg/contrib/fileutl.h
  3. 2
      apt-pkg/contrib/proxy.cc
  4. 9
      test/libapt/apt-proxy-script
  5. 26
      test/libapt/uri_test.cc

8
apt-pkg/contrib/fileutl.cc

@ -2860,6 +2860,11 @@ bool Rename(std::string From, std::string To) /*{{{*/
}
/*}}}*/
bool Popen(const char* Args[], FileFd &Fd, pid_t &Child, FileFd::OpenMode Mode)/*{{{*/
{
return Popen(Args, Fd, Child, Mode, true);
}
/*}}}*/
bool Popen(const char* Args[], FileFd &Fd, pid_t &Child, FileFd::OpenMode Mode, bool CaptureStderr)/*{{{*/
{
int fd;
if (Mode != FileFd::ReadOnly && Mode != FileFd::WriteOnly)
@ -2891,7 +2896,8 @@ bool Popen(const char* Args[], FileFd &Fd, pid_t &Child, FileFd::OpenMode Mode)/
if(Mode == FileFd::ReadOnly)
{
dup2(fd, 1);
dup2(fd, 2);
if (CaptureStderr == true)
dup2(fd, 2);
} else if(Mode == FileFd::WriteOnly)
dup2(fd, 0);

3
apt-pkg/contrib/fileutl.h

@ -248,8 +248,11 @@ std::vector<std::string> Glob(std::string const &pattern, int flags=0);
* \param Child a reference to the integer that stores the child pid
* Note that you must call ExecWait() or similar to cleanup
* \param Mode is either FileFd::ReadOnly or FileFd::WriteOnly
* \param CaptureStderr True if we should capture stderr in addition to stdout.
* (default: True).
* \return true on success, false on failure with _error set
*/
bool Popen(const char* Args[], FileFd &Fd, pid_t &Child, FileFd::OpenMode Mode, bool CaptureStderr);
bool Popen(const char* Args[], FileFd &Fd, pid_t &Child, FileFd::OpenMode Mode);

2
apt-pkg/contrib/proxy.cc

@ -48,7 +48,7 @@ bool AutoDetectProxy(URI &URL)
Args.push_back(nullptr);
FileFd PipeFd;
pid_t Child;
if(Popen(&Args[0], PipeFd, Child, FileFd::ReadOnly) == false)
if(Popen(&Args[0], PipeFd, Child, FileFd::ReadOnly, false) == false)
return _error->Error("ProxyAutoDetect command '%s' failed!", AutoDetectProxyCmd.c_str());
char buf[512];
bool const goodread = PipeFd.ReadLine(buf, sizeof(buf)) != nullptr;

9
test/libapt/apt-proxy-script

@ -0,0 +1,9 @@
#!/bin/sh
if [ $1 = "http://www.debian.org:90/temp/test" ]; then
echo "http://example.com"
fi
if [ $1 = "http://www.debian.org:91/temp/test" ]; then
echo "This works" >&2
echo "http://example.com/foo"
fi

26
test/libapt/uri_test.cc

@ -1,4 +1,6 @@
#include <config.h>
#include <apt-pkg/configuration.h>
#include <apt-pkg/proxy.h>
#include <apt-pkg/strutl.h>
#include <string>
#include <gtest/gtest.h>
@ -188,3 +190,27 @@ TEST(URITest, RFC2732)
EXPECT_EQ("ftp://example.org", URI::ArchiveOnly(U));
EXPECT_EQ("ftp://example.org/", URI::NoUserPassword(U));
}
TEST(URITest, AutoProxyTest)
{
URI u0("http://www.debian.org:90/temp/test");
URI u1("http://www.debian.org:91/temp/test");
_config->Set("Acquire::http::Proxy-Auto-Detect", "./apt-proxy-script");
// Scenario 0: Autodetecting a simple proxy
AutoDetectProxy(u0);
EXPECT_EQ(_config->Find("Acquire::http::proxy::www.debian.org", ""), "http://example.com");
// Scenario 1: Proxy stays the same if it is already set
AutoDetectProxy(u1);
EXPECT_EQ(_config->Find("Acquire::http::proxy::www.debian.org", ""), "http://example.com");
// Scenario 2: Reading with stderr output works fine
_config->Clear("Acquire::http::proxy::www.debian.org");
AutoDetectProxy(u1);
EXPECT_EQ(_config->Find("Acquire::http::proxy::www.debian.org", ""), "http://example.com/foo");
// Scenario 1 again: Proxy stays the same if it is already set
AutoDetectProxy(u0);
EXPECT_EQ(_config->Find("Acquire::http::proxy::www.debian.org", ""), "http://example.com/foo");
}

Loading…
Cancel
Save