From 9515ed7bcdb32c7985ca83d309beda7155d02136 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Mon, 20 Jun 2016 13:49:31 +0200 Subject: [PATCH] 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. --- apt-pkg/contrib/proxy.cc | 19 +++++++---- cmdline/apt-helper.cc | 3 +- doc/apt.conf.5.xml | 19 +++++++---- test/integration/test-apt-helper | 55 ++++++++++++++++---------------- 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. Acquire::http::Proxy-Auto-Detect 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 - http://proxy:port/. This will override the - generic Acquire::http::Proxy but not any specific - host proxy configuration set via - Acquire::http::Proxy::$HOST. + 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 http://proxy:port/ + or the word DIRECT 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 Acquire::http::Proxy::HOST. 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 ProxyAutoDetect. 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/' -- 2.45.2