implement and document DIRECT for auto-detect-proxy

There is a subtile difference between an empty setting and "DIRECT" in
the configuration as the later overrides the generic settings while the
earlier does not. Also, non-zero exitcodes should really be reported as
an error rather than silently discarded.

4 files changed, 54 insertions, 42 deletions

diff --git a/apt-pkg/contrib/proxy.cc b/apt-pkg/contrib/proxy.cc
index 84d802dcb..4529cf230 100644
--- a/apt-pkg/contrib/proxy.cc
+++ b/apt-pkg/contrib/proxy.cc
@@ -51,19 +51,26 @@ bool AutoDetectProxy(URI &URL)
    if(Popen(&Args[0], PipeFd, Child, FileFd::ReadOnly) == false)
       return _error->Error("ProxyAutoDetect command '%s' failed!", AutoDetectProxyCmd.c_str());
    char buf[512];
-   if (PipeFd.ReadLine(buf, sizeof(buf)) == nullptr)
-      return true;
+   bool const goodread = PipeFd.ReadLine(buf, sizeof(buf)) != nullptr;
    PipeFd.Close();
-   ExecWait(Child, "ProxyAutoDetect", true);
+   if (ExecWait(Child, "ProxyAutoDetect") == false)
+      return false;
+   // no output means the detector has no idea which proxy to use
+   // and apt will use the generic proxy settings
+   if (goodread == false)
+      return true;
    auto const cleanedbuf = _strstrip(buf);
-
+   // We warn about this as the implementor probably meant to use DIRECT instead
    if (cleanedbuf[0] == '\0')
-      return _error->Warning("ProxyAutoDetect returned no data");
+   {
+      _error->Warning("ProxyAutoDetect command returned an empty line");
+      return true;
+   }
 
    if (Debug)
       std::clog << "auto detect command returned: '" << cleanedbuf << "'" << std::endl;
 
-   if (strstr(cleanedbuf, URL.Access.c_str()) == cleanedbuf)
+   if (strstr(cleanedbuf, URL.Access.c_str()) == cleanedbuf || strcmp(cleanedbuf, "DIRECT") == 0)
       _config->Set("Acquire::"+URL.Access+"::proxy::"+URL.Host, cleanedbuf);
 
    return true;
diff --git a/cmdline/apt-helper.cc b/cmdline/apt-helper.cc
index f3b8c326e..fd99fba8b 100644
--- a/cmdline/apt-helper.cc
+++ b/cmdline/apt-helper.cc
@@ -39,7 +39,8 @@ static bool DoAutoDetectProxy(CommandLine &CmdL)			/*{{{*/
    if (CmdL.FileSize() != 2)
       return _error->Error(_("Need one URL as argument"));
    URI ServerURL(CmdL.FileList[1]);
-   AutoDetectProxy(ServerURL);
+   if (AutoDetectProxy(ServerURL) == false)
+      return false;
    std::string SpecificProxy = _config->Find("Acquire::"+ServerURL.Access+"::Proxy::" + ServerURL.Host);
    ioprintf(std::cout, "Using proxy '%s' for URL '%s'\n",
             SpecificProxy.c_str(), std::string(ServerURL).c_str());
diff --git a/doc/apt.conf.5.xml b/doc/apt.conf.5.xml
index 6aa0c9629..d71f99c0a 100644
--- a/doc/apt.conf.5.xml
+++ b/doc/apt.conf.5.xml
@@ -444,15 +444,20 @@ APT::Compressor::rev {
      only if the client uses a known identifier.</para>
 
      <para><literal>Acquire::http::Proxy-Auto-Detect</literal> can be used to
-     specify an external command to discover the http proxy to use. Apt expects
-     the command to output the proxy on stdout in the style
-     <literal>http://proxy:port/</literal>. This will override the
-     generic <literal>Acquire::http::Proxy</literal> but not any specific
-     host proxy configuration set via 
-     <literal>Acquire::http::Proxy::$HOST</literal>.
+     specify an external command to discover the http proxy to use. The first
+     and only parameter is an URI denoting the host to be contacted to allow
+     for host-specific configuration. APT expects the command to output the
+     proxy on stdout as a single line in the style <literal>http://proxy:port/</literal>
+     or the word <literal>DIRECT</literal> if no proxy should be used. No output
+     indicates that the generic proxy settings should be used.
+
+     Note that auto-detection will not be used for a host if a host-specific proxy
+     configuration is already set via <literal>Acquire::http::Proxy::<replaceable>HOST</replaceable></literal>.
 
      See the &squid-deb-proxy-client; package for an example implementation that
-     uses avahi. This option takes precedence over the legacy option name 
+     uses avahi.
+
+     This option takes precedence over the legacy option name
      <literal>ProxyAutoDetect</literal>.
      </para>
 
diff --git a/test/integration/test-apt-helper b/test/integration/test-apt-helper
index c1c193c5c..1c163b3ae 100755
--- a/test/integration/test-apt-helper
+++ b/test/integration/test-apt-helper
@@ -71,44 +71,43 @@ E: Download Failed"
     testfileequal './downloaded/foo10' 'bar'
 }
 
+setupproxydetect() {
+	local METH="$1"
+	shift
+	{
+		echo '#!/bin/sh -e'
+		echo "$@"
+	} > "${TMPWORKINGDIRECTORY}/apt-proxy-detect"
+	chmod 755 "${TMPWORKINGDIRECTORY}/apt-proxy-detect"
+	echo "Acquire::${METH}::Proxy-Auto-Detect \"${TMPWORKINGDIRECTORY}/apt-proxy-detect\";" > rootdir/etc/apt/apt.conf.d/02proxy-detect
+}
+
 test_apt_helper_detect_proxy() {
-    # no proxy
+    msgmsg 'apt-helper auto-detect-proxy' 'no proxy'
     testsuccessequal "Using proxy '' for URL 'http://example.com/'" apthelper auto-detect-proxy http://example.com/
-    cat > apt-proxy-detect <<'EOF'
-#!/bin/sh -e
-exit 0
-EOF
-    chmod 755 apt-proxy-detect
-    echo "Acquire::http::Proxy-Auto-Detect \"$(pwd)/apt-proxy-detect\";" > rootdir/etc/apt/apt.conf.d/02proxy-detect
-    testsuccessequal "Using proxy '' for URL 'http://www.example.com/'" apthelper auto-detect-proxy http://www.example.com
-
-    # http auto detect proxy script
-    cat > apt-proxy-detect <<'EOF'
-#!/bin/sh -e
-echo "http://some-proxy"
-EOF
-    chmod 755 apt-proxy-detect
-    echo "Acquire::http::Proxy-Auto-Detect \"$(pwd)/apt-proxy-detect\";" > rootdir/etc/apt/apt.conf.d/02proxy-detect
-
+    setupproxydetect 'http' 'exit 0'
+    testsuccessequal "Using proxy '' for URL 'http://example.com/'" apthelper auto-detect-proxy http://example.com/
+    setupproxydetect 'http' 'exit 1'
+    testfailureequal 'E: Sub-process ProxyAutoDetect returned an error code (1)' apthelper auto-detect-proxy http://example.com/
+    setupproxydetect 'http' 'echo'
+    testwarningequal "Using proxy '' for URL 'http://example.com/'
+W: ProxyAutoDetect command returned an empty line" apthelper auto-detect-proxy http://example.com/
+    setupproxydetect 'http' 'echo DIRECT'
+    testsuccessequal "Using proxy 'DIRECT' for URL 'http://example.com/'" apthelper auto-detect-proxy http://example.com/
+
+    msgmsg 'apt-helper auto-detect-proxy' 'http proxy'
+    setupproxydetect 'http' 'echo "http://some-proxy"'
     testsuccessequal "Using proxy 'http://some-proxy' for URL 'http://www.example.com/'" apthelper auto-detect-proxy http://www.example.com
 
-
-    # https auto detect proxy script
-    cat > apt-proxy-detect <<'EOF'
-#!/bin/sh -e
-echo "https://https-proxy"
-EOF
-    chmod 755 apt-proxy-detect
-    echo "Acquire::https::Proxy-Auto-Detect \"$(pwd)/apt-proxy-detect\";" > rootdir/etc/apt/apt.conf.d/02proxy-detect
-
+    msgmsg 'apt-helper auto-detect-proxy' 'https proxy'
+    setupproxydetect 'https' 'echo "https://https-proxy"'
     testsuccessequal "Using proxy 'https://https-proxy' for URL 'https://ssl.example.com/'" apthelper auto-detect-proxy https://ssl.example.com
 }
 
 test_apt_helper_download "http://localhost:${APTHTTPPORT}"
 test_apt_helper_download "https://localhost:${APTHTTPSPORT}"
 test_apt_helper_detect_proxy
-
-# test failure modes
+msgmsg 'test various failure modes'
 testfailureequal 'E: Invalid operation download' apthelper download
 testfailureequal 'E: Must specify at least one pair url/filename' apthelper download-file
 testfailureequal 'E: Must specify at least one pair url/filename' apthelper download-file 'http://example.org/'