From b5595da902e62af7c295f1603ae5b43ba4cef281 Mon Sep 17 00:00:00 2001 From: "bubulle@debian.org" <> Date: Wed, 10 Apr 2013 11:28:11 +0200 Subject: Fix English spelling error in a message ('A error'). Unfuzzy translations. Closes: #705087 --- methods/http.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'methods') diff --git a/methods/http.h b/methods/http.h index 7a3ccda54..7446119cd 100644 --- a/methods/http.h +++ b/methods/http.h @@ -158,7 +158,7 @@ class HttpMethod : public pkgAcqMethod ERROR_UNRECOVERABLE, /** \brief The server reported a error with a error content page */ ERROR_WITH_CONTENT_PAGE, - /** \brief A error on the client side */ + /** \brief An error on the client side */ ERROR_NOT_FROM_SERVER, /** \brief A redirect or retry request */ TRY_AGAIN_OR_REDIRECT -- cgit v1.2.3-70-g09d2 From 1dea08eb2e1115b8da14cc3da02d53f8e069ba14 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Wed, 8 May 2013 17:45:17 +0200 Subject: properly handle if-modfied-since with libcurl/https (closes: #705648) --- apt-pkg/acquire-worker.cc | 13 ++++++++++--- debian/changelog | 8 ++++++++ debian/control | 2 +- methods/https.cc | 7 ++++++- 4 files changed, 25 insertions(+), 5 deletions(-) (limited to 'methods') diff --git a/apt-pkg/acquire-worker.cc b/apt-pkg/acquire-worker.cc index 9d90b08bc..44a84216a 100644 --- a/apt-pkg/acquire-worker.cc +++ b/apt-pkg/acquire-worker.cc @@ -305,7 +305,15 @@ bool pkgAcquire::Worker::RunMessages() OwnerQ->ItemDone(Itm); unsigned long long const ServerSize = strtoull(LookupTag(Message,"Size","0").c_str(), NULL, 10); - if (TotalSize != 0 && ServerSize != TotalSize) + bool isHit = StringToBool(LookupTag(Message,"IMS-Hit"),false) || + StringToBool(LookupTag(Message,"Alt-IMS-Hit"),false); + // Using the https method the server might return 200, but the + // If-Modified-Since condition is not satsified, libcurl will + // discard the download. In this case, however, TotalSize will be + // set to the actual size of the file, while ServerSize will be set + // to 0. Therefore, if the item is marked as a hit and the + // downloaded size (ServerSize) is 0, we ignore TotalSize. + if (TotalSize != 0 && (!isHit || ServerSize != 0) && ServerSize != TotalSize) _error->Warning("Size of file %s is not what the server reported %s %llu", Owner->DestFile.c_str(), LookupTag(Message,"Size","0").c_str(),TotalSize); @@ -332,8 +340,7 @@ bool pkgAcquire::Worker::RunMessages() // Log that we are done if (Log != 0) { - if (StringToBool(LookupTag(Message,"IMS-Hit"),false) == true || - StringToBool(LookupTag(Message,"Alt-IMS-Hit"),false) == true) + if (isHit) { /* Hide 'hits' for local only sources - we also manage to hide gets */ diff --git a/debian/changelog b/debian/changelog index 9ed9b4d61..db7a8ffda 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,11 @@ +apt (0.9.7.9) UNRELEASED; urgency=low + + [ Ludovico Cavedon ] + * properly handle if-modfied-since with libcurl/https + (closes: #705648) + + -- Michael Vogt Wed, 08 May 2013 16:20:13 +0200 + apt (0.9.7.8) unstable; urgency=criticial * SECURITY UPDATE: InRelease verification bypass diff --git a/debian/control b/debian/control index 50b3599fc..4a73239f7 100644 --- a/debian/control +++ b/debian/control @@ -7,7 +7,7 @@ Uploaders: Michael Vogt , Otavio Salvador , Julian Andres Klode Standards-Version: 3.9.3 Build-Depends: dpkg-dev (>= 1.15.8), debhelper (>= 8.1.3~), libdb-dev, - gettext (>= 0.12), libcurl4-gnutls-dev (>= 7.19.0), + gettext (>= 0.12), libcurl4-gnutls-dev (>= 7.19.4~), zlib1g-dev, libbz2-dev, xsltproc, docbook-xsl, docbook-xml, po4a (>= 0.34-2), autotools-dev, autoconf, automake Build-Depends-Indep: doxygen, debiandoc-sgml diff --git a/methods/https.cc b/methods/https.cc index c1a49ba60..d85415b2f 100644 --- a/methods/https.cc +++ b/methods/https.cc @@ -285,6 +285,11 @@ bool HttpsMethod::Fetch(FetchItem *Itm) long curl_servdate; curl_easy_getinfo(curl, CURLINFO_FILETIME, &curl_servdate); + // If the server returns 200 OK but the If-Modified-Since condition is not + // met, CURLINFO_CONDITION_UNMET will be set to 1 + long curl_condition_unmet = 0; + curl_easy_getinfo(curl, CURLINFO_CONDITION_UNMET, &curl_condition_unmet); + File->Close(); // cleanup @@ -312,7 +317,7 @@ bool HttpsMethod::Fetch(FetchItem *Itm) Res.Filename = File->Name(); Res.LastModified = Buf.st_mtime; Res.IMSHit = false; - if (curl_responsecode == 304) + if (curl_responsecode == 304 || curl_condition_unmet) { unlink(File->Name().c_str()); Res.IMSHit = true; -- cgit v1.2.3-70-g09d2 From 5b63d2a9a2e088bb7df7c703e9452af7efc88210 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Wed, 8 May 2013 17:50:15 +0200 Subject: merged patch from Daniel Hartwig to fix URI and proxy releated issues --- apt-pkg/contrib/strutl.cc | 9 +++++---- debian/changelog | 17 +++++++++++++++++ methods/http.cc | 14 +++++++------- methods/https.cc | 20 +++++++++++++++++++- .../test-bug-595691-empty-and-broken-archive-files | 14 +++++++------- test/integration/test-releasefile-verification | 4 ++-- test/libapt/uri_test.cc | 8 ++++++++ 7 files changed, 65 insertions(+), 21 deletions(-) (limited to 'methods') diff --git a/apt-pkg/contrib/strutl.cc b/apt-pkg/contrib/strutl.cc index 03b98e93e..f4dd3407d 100644 --- a/apt-pkg/contrib/strutl.cc +++ b/apt-pkg/contrib/strutl.cc @@ -1483,9 +1483,12 @@ URI::operator string() if (User.empty() == false) { - Res += User; + // FIXME: Technically userinfo is permitted even less + // characters than these, but this is not conveniently + // expressed with a blacklist. + Res += QuoteString(User, ":/?#[]@"); if (Password.empty() == false) - Res += ":" + Password; + Res += ":" + QuoteString(Password, ":/?#[]@"); Res += "@"; } @@ -1524,7 +1527,6 @@ string URI::SiteOnly(const string &URI) U.User.clear(); U.Password.clear(); U.Path.clear(); - U.Port = 0; return U; } /*}}}*/ @@ -1536,7 +1538,6 @@ string URI::NoUserPassword(const string &URI) ::URI U(URI); U.User.clear(); U.Password.clear(); - U.Port = 0; return U; } /*}}}*/ diff --git a/debian/changelog b/debian/changelog index 182596b62..d5ae8448b 100644 --- a/debian/changelog +++ b/debian/changelog @@ -50,6 +50,23 @@ apt (0.9.8) UNRELEASED; urgency=low [ Manpages translations ] * French translation completed (Christian Perrier) + [ Daniel Hartwig ] + * apt-pkg/contrib/strutl.cc: + - include port in shortened URIs (e.g. with apt-cache policy, progress + display) thanks to James McCoy (Closes: #154868, #322074) + - percent-encode username and password when writing URIs + * methods/http.cc: + - properly escape IP-literals (e.g. IPv6 address) when building + Host headers and URIs (Closes: #620344) + * methods/https.cc: + - use https_proxy environment variable if present, falling back to + http_proxy otherwise + - use authentication credentials from proxy URI + (Closes: #651640, LP: #1087512) + - environment variables do not override an explicit no proxy + directive ("DIRECT") in apt.conf + - disregard all_proxy environment variable, like other methods + -- Michael Vogt Mon, 08 Apr 2013 08:43:21 +0200 apt (0.9.7.9~exp2) experimental; urgency=low diff --git a/methods/http.cc b/methods/http.cc index fddf8a78e..db1085a2d 100644 --- a/methods/http.cc +++ b/methods/http.cc @@ -667,7 +667,12 @@ void HttpMethod::SendReq(FetchItem *Itm,CircleBuf &Out) // The HTTP server expects a hostname with a trailing :port char Buf[1000]; - string ProperHost = Uri.Host; + string ProperHost; + + if (Uri.Host.find(':') != string::npos) + ProperHost = '[' + Uri.Host + ']'; + else + ProperHost = Uri.Host; if (Uri.Port != 0) { sprintf(Buf,":%u",Uri.Port); @@ -975,12 +980,7 @@ HttpMethod::DealWithHeaders(FetchResult &Res,ServerState *Srv) { URI Uri = Queue->Uri; if (Uri.Host.empty() == false) - { - if (Uri.Port != 0) - strprintf(NextURI, "http://%s:%u", Uri.Host.c_str(), Uri.Port); - else - NextURI = "http://" + Uri.Host; - } + NextURI = URI::SiteOnly(Uri); else NextURI.clear(); NextURI.append(DeQuoteString(Srv->Location)); diff --git a/methods/https.cc b/methods/https.cc index b44642ab2..84ce2d68f 100644 --- a/methods/https.cc +++ b/methods/https.cc @@ -63,6 +63,12 @@ void HttpsMethod::SetupProxy() /*{{{*/ { URI ServerName = Queue->Uri; + // Curl should never read proxy settings from the environment, as + // we determine which proxy to use. Do this for consistency among + // methods and prevent an environment variable overriding a + // no-proxy ("DIRECT") setting in apt.conf. + curl_easy_setopt(curl, CURLOPT_PROXY, ""); + // Determine the proxy setting - try https first, fallback to http and use env at last string UseProxy = _config->Find("Acquire::https::Proxy::" + ServerName.Host, _config->Find("Acquire::http::Proxy::" + ServerName.Host).c_str()); @@ -81,7 +87,14 @@ void HttpsMethod::SetupProxy() /*{{{*/ if (getenv("no_proxy") != 0 && CheckDomainList(ServerName.Host,getenv("no_proxy")) == true) return; } else { - const char* result = getenv("http_proxy"); + const char* result = getenv("https_proxy"); + // FIXME: Fall back to http_proxy is to remain compatible with + // existing setups and behaviour of apt.conf. This should be + // deprecated in the future (including apt.conf). Most other + // programs do not fall back to http proxy settings and neither + // should Apt. + if (result == NULL) + result = getenv("http_proxy"); UseProxy = result == NULL ? "" : result; } @@ -92,6 +105,11 @@ void HttpsMethod::SetupProxy() /*{{{*/ if (Proxy.Port != 1) curl_easy_setopt(curl, CURLOPT_PROXYPORT, Proxy.Port); curl_easy_setopt(curl, CURLOPT_PROXY, Proxy.Host.c_str()); + if (Proxy.User.empty() == false || Proxy.Password.empty() == false) + { + curl_easy_setopt(curl, CURLOPT_PROXYUSERNAME, Proxy.User.c_str()); + curl_easy_setopt(curl, CURLOPT_PROXYPASSWORD, Proxy.Password.c_str()); + } } } /*}}}*/ // HttpsMethod::Fetch - Fetch an item /*{{{*/ diff --git a/test/integration/test-bug-595691-empty-and-broken-archive-files b/test/integration/test-bug-595691-empty-and-broken-archive-files index 4611b8b8e..a05ed5fa6 100755 --- a/test/integration/test-bug-595691-empty-and-broken-archive-files +++ b/test/integration/test-bug-595691-empty-and-broken-archive-files @@ -103,23 +103,23 @@ testoverhttp() { setupcompressor "$1" createemptyfile 'en' - testaptgetupdate "Get: http://localhost Packages [] -Get: http://localhost Translation-en + testaptgetupdate "Get: http://localhost:8080 Packages [] +Get: http://localhost:8080 Translation-en Reading package lists..." "empty file en.$COMPRESS over http" createemptyarchive 'en' - testaptgetupdate "Get: http://localhost Packages [] -Get: http://localhost Translation-en [] + testaptgetupdate "Get: http://localhost:8080 Packages [] +Get: http://localhost:8080 Translation-en [] Reading package lists..." "empty archive en.$COMPRESS over http" createemptyarchive 'Packages' - testaptgetupdate "Get: http://localhost Packages [] + testaptgetupdate "Get: http://localhost:8080 Packages [] Reading package lists..." "empty archive Packages.$COMPRESS over http" createemptyfile 'Packages' #FIXME: we should response with a good error message instead - testaptgetupdate "Get: http://localhost Packages -Err http://localhost Packages + testaptgetupdate "Get: http://localhost:8080 Packages +Err http://localhost:8080 Packages Empty files can't be valid archives W: Failed to fetch ${COMPRESSOR}:$(readlink -f rootdir/var/lib/apt/lists/partial/localhost:8080_Packages) Empty files can't be valid archives diff --git a/test/integration/test-releasefile-verification b/test/integration/test-releasefile-verification index 01fb2e529..fba7ab290 100755 --- a/test/integration/test-releasefile-verification +++ b/test/integration/test-releasefile-verification @@ -37,7 +37,7 @@ The following NEW packages will be installed: apt 0 upgraded, 1 newly installed, 0 to remove and 0 not upgraded. After this operation, 5370 kB of additional disk space will be used. -Get:1 http://localhost/ apt 0.7.25.3 +Get:1 http://localhost:8080/ apt 0.7.25.3 Download complete and in download only mode' aptget install apt -dy } @@ -50,7 +50,7 @@ The following NEW packages will be installed: apt 0 upgraded, 1 newly installed, 0 to remove and 0 not upgraded. After this operation, 5808 kB of additional disk space will be used. -Get:1 http://localhost/ apt 0.8.0~pre1 +Get:1 http://localhost:8080/ apt 0.8.0~pre1 Download complete and in download only mode' aptget install apt -dy } diff --git a/test/libapt/uri_test.cc b/test/libapt/uri_test.cc index 99bb3067e..16fde503f 100644 --- a/test/libapt/uri_test.cc +++ b/test/libapt/uri_test.cc @@ -108,5 +108,13 @@ int main() { equals("/debian/", U.Path); } + // Percent-encoding. + { + URI U("ftp://foo:b%40r@example.org"); + equals("foo", U.User); + equals("b@r", U.Password); + equals("ftp://foo:b%40r@example.org", (std::string) U); + } + return 0; } -- cgit v1.2.3-70-g09d2 From 245ba2c306e663fb311b7796fdf13a7ae7073a4d Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 6 Jun 2013 18:20:35 +0200 Subject: Fix double free (closes: #711045) * Fix double free (closes: #711045) * Fix crash when the "mirror" method does not find any entry (closes: #699303) --- debian/changelog | 6 +++++- methods/mirror.cc | 3 +++ 2 files changed, 8 insertions(+), 1 deletion(-) (limited to 'methods') diff --git a/debian/changelog b/debian/changelog index f0eb0421d..e31070aec 100644 --- a/debian/changelog +++ b/debian/changelog @@ -11,7 +11,11 @@ apt (0.9.8.2) UNRELEASED; urgency=low - fix build failure when building without NLS (closes: #671587) [ Gregoire Menuel ] - * fix double free (closes: #711045) + * Fix double free (closes: #711045) + + [ Raphael Geissert ] + * Fix crash when the "mirror" method does not find any entry + (closes: #699303) -- Christian Perrier Thu, 16 May 2013 22:28:22 +0200 diff --git a/methods/mirror.cc b/methods/mirror.cc index d6c5ba955..854366318 100644 --- a/methods/mirror.cc +++ b/methods/mirror.cc @@ -311,6 +311,9 @@ bool MirrorMethod::InitMirrors() AllMirrors.push_back(s); } + if (AllMirrors.empty()) { + return _error->Error(_("No entry found in mirror file '%s'"), MirrorFile.c_str()); + } Mirror = AllMirrors[0]; UsedMirror = Mirror; return true; -- cgit v1.2.3-70-g09d2 From 3a61adbba8bfc9ba76d1262e0e8118f78920f9fe Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sun, 19 May 2013 18:53:19 +0200 Subject: remove -ldl from cdrom and -lutil from apt-get linkage Building src:apt shows: dpkg-shlibdeps: warning: package could avoid a useless dependency if debian/apt/usr/lib/apt/methods/cdrom was not linked against libdl.so.2 (it uses none of the library's symbols) dpkg-shlibdeps: warning: package could avoid a useless dependency if debian/apt/usr/bin/apt-get was not linked against libutil.so.1 (it uses none of the library's symbols) --- cmdline/makefile | 2 +- debian/changelog | 2 +- methods/makefile | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) (limited to 'methods') diff --git a/cmdline/makefile b/cmdline/makefile index f3712232a..460a71240 100644 --- a/cmdline/makefile +++ b/cmdline/makefile @@ -14,7 +14,7 @@ include $(PROGRAM_H) # The apt-get program PROGRAM=apt-get -SLIBS = -lapt-pkg -lutil $(INTLLIBS) +SLIBS = -lapt-pkg $(INTLLIBS) LIB_MAKES = apt-pkg/makefile SOURCE = apt-get.cc acqprogress.cc include $(PROGRAM_H) diff --git a/debian/changelog b/debian/changelog index 1a8604e09..662289fba 100644 --- a/debian/changelog +++ b/debian/changelog @@ -2,6 +2,7 @@ apt (0.9.8.3) UNRELEASED; urgency=low [ David Kalnischkies ] * build the en manpages in subdirectory doc/en + * remove -ldl from cdrom and -lutil from apt-get linkage -- David Kalnischkies Sun, 09 Jun 2013 15:06:24 +0200 @@ -24,7 +25,6 @@ apt (0.9.8.2) unstable; urgency=low * Fix crash when the "mirror" method does not find any entry (closes: #699303) - [ Johan Kiviniemi ] * cmdline/apt-key: - Create new keyrings with mode 0644 instead of 0600. diff --git a/methods/makefile b/methods/makefile index a271aff5e..294c55d23 100644 --- a/methods/makefile +++ b/methods/makefile @@ -39,7 +39,7 @@ include $(PROGRAM_H) # The cdrom method PROGRAM=cdrom -SLIBS = -lapt-pkg -ldl $(INTLLIBS) +SLIBS = -lapt-pkg $(INTLLIBS) LIB_MAKES = apt-pkg/makefile SOURCE = cdrom.cc include $(PROGRAM_H) -- cgit v1.2.3-70-g09d2 From ae99ce2e3cadb07c80b89ab2afc804875b1026c5 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Mon, 17 Jun 2013 11:23:13 +0200 Subject: trigger NODATA error for invalid InRelease files With the selfgrown splitting we got the problem of not recovering from networks which just reply with invalid data like those sending us login pages to authenticate with the network (e.g. hotels) back. The good thing about the InRelease file is that we know that it must be clearsigned (a Release file might or might not have a detached sig) so if we get a file but are unable to split it something is seriously wrong, so there is not much point in trying further. The Acquire system already looks out for a NODATA error from gpgv, so this adds a new error message sent to the acquire system in case the splitting we do now ourselves failed including this magic word. Closes: #712486 --- apt-pkg/contrib/gpgv.cc | 2 +- apt-pkg/contrib/gpgv.h | 15 ++++- debian/changelog | 1 + methods/gpgv.cc | 16 +++--- test/integration/framework | 3 + .../test-ubuntu-bug-346386-apt-get-update-paywall | 64 ++++++++++++++++++++++ test/interactive-helper/aptwebserver.cc | 26 +++++++++ 7 files changed, 114 insertions(+), 13 deletions(-) create mode 100755 test/integration/test-ubuntu-bug-346386-apt-get-update-paywall (limited to 'methods') diff --git a/apt-pkg/contrib/gpgv.cc b/apt-pkg/contrib/gpgv.cc index 31db7d5fe..f47e7ea48 100644 --- a/apt-pkg/contrib/gpgv.cc +++ b/apt-pkg/contrib/gpgv.cc @@ -154,7 +154,7 @@ void ExecGPGV(std::string const &File, std::string const &FileGPG, if (sigFd != -1) unlink(data); ioprintf(std::cerr, "Splitting up %s into data and signature failed", File.c_str()); - exit(EINTERNAL); + exit(112); } Args.push_back(sig); Args.push_back(data); diff --git a/apt-pkg/contrib/gpgv.h b/apt-pkg/contrib/gpgv.h index 08b10a97a..45f069058 100644 --- a/apt-pkg/contrib/gpgv.h +++ b/apt-pkg/contrib/gpgv.h @@ -23,9 +23,18 @@ /** \brief generates and run the command to verify a file with gpgv * * If File and FileSig specify the same file it is assumed that we - * deal with a clear-signed message. In that case the file will be - * rewritten to be in a good-known format without uneeded whitespaces - * and additional messages (unsigned or signed). + * deal with a clear-signed message. Note that the method will accept + * and validate files which include additional (unsigned) messages + * without complaining. Do NOT open files accepted by this method + * for reading. Use #OpenMaybeClearSignedFile to access the message + * instead to ensure you are only reading signed data. + * + * The method does not return, but has some noteable exit-codes: + * 111 signals an internal error like the inability to execute gpgv, + * 112 indicates a clear-signed file which doesn't include a message, + * which can happen if APT is run while on a network requiring + * authentication before usage (e.g. in hotels) + * All other exit-codes are passed-through from gpgv. * * @param File is the message (unsigned or clear-signed) * @param FileSig is the signature (detached or clear-signed) diff --git a/debian/changelog b/debian/changelog index 91d4ae536..bd25df1e2 100644 --- a/debian/changelog +++ b/debian/changelog @@ -20,6 +20,7 @@ apt (0.9.8.3) UNRELEASED; urgency=low * try defaults if auto-detection failed in apt-cdrom (Closes: #712433) * support \n and \r\n line endings in ReadMessages * do not redownload unchanged InRelease files + * trigger NODATA error for invalid InRelease files (Closes: #712486) -- David Kalnischkies Sun, 09 Jun 2013 15:06:24 +0200 diff --git a/methods/gpgv.cc b/methods/gpgv.cc index 3f814b9f0..fe8bac6c9 100644 --- a/methods/gpgv.cc +++ b/methods/gpgv.cc @@ -55,9 +55,6 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile, vector &NoPubKeySigners) { bool const Debug = _config->FindB("Debug::Acquire::gpgv", false); - // setup a (empty) stringstream for formating the return value - std::stringstream ret; - ret.str(""); if (Debug == true) std::clog << "inside VerifyGetSigners" << std::endl; @@ -170,18 +167,19 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile, return ""; } else if (WEXITSTATUS(status) == 1) - { return _("At least one invalid signature was encountered."); - } else if (WEXITSTATUS(status) == 111) + return _("Could not execute 'gpgv' to verify signature (is gpgv installed?)"); + else if (WEXITSTATUS(status) == 112) { - ioprintf(ret, _("Could not execute 'gpgv' to verify signature (is gpgv installed?)")); - return ret.str(); + // acquire system checks for "NODATA" to generate GPG errors (the others are only warnings) + std::string errmsg; + //TRANSLATORS: %s is a single techy word like 'NODATA' + strprintf(errmsg, _("Clearsigned file isn't valid, got '%s' (does the network require authentication?)"), "NODATA"); + return errmsg; } else - { return _("Unknown error executing gpgv"); - } } bool GPGVMethod::Fetch(FetchItem *Itm) diff --git a/test/integration/framework b/test/integration/framework index 3f11ac23b..3a02cfb76 100644 --- a/test/integration/framework +++ b/test/integration/framework @@ -118,6 +118,9 @@ gdb() { echo "gdb: run »$*«" APT_CONFIG=aptconfig.conf LD_LIBRARY_PATH=${BUILDDIRECTORY} $(which gdb) ${BUILDDIRECTORY}/$1 } +http() { + LD_LIBRARY_PATH=${BUILDDIRECTORY} ${BUILDDIRECTORY}/methods/http +} exitwithstatus() { # error if we about to overflow, but ... diff --git a/test/integration/test-ubuntu-bug-346386-apt-get-update-paywall b/test/integration/test-ubuntu-bug-346386-apt-get-update-paywall new file mode 100755 index 000000000..1576c396c --- /dev/null +++ b/test/integration/test-ubuntu-bug-346386-apt-get-update-paywall @@ -0,0 +1,64 @@ +#!/bin/sh +set -e + +TESTDIR=$(readlink -f $(dirname $0)) +. $TESTDIR/framework + +setupenvironment +configarchitecture 'native' + +insertpackage 'unstable' 'unrelated' 'all' '1.0' 'stable' +insertsource 'unstable' 'unrelated' 'all' '1.0' 'stable' + +echo 'ni ni ni' > aptarchive/knights + +setupaptarchive +changetowebserver -o 'aptwebserver::overwrite::.*::filename=/knights' + +msgtest 'Acquire test file from the webserver to check' 'overwrite' +echo '601 Configuration +Config-Item: Acquire::http::DependOnSTDIN=0 + +600 Acquire URI +URI: http://localhost:8080/holygrail +Filename: knights-talking +' | http >/dev/null 2>&1 && msgpass || msgfail +testfileequal knights-talking 'ni ni ni' + +ensure_n_canary_strings_in_dir() { + local DIR="$1" + local CANARY_STRING="$2" + local EXPECTED_N="$3" + + msgtest "Testing in $DIR for $EXPECTED_N canary" "$CANARY_STRING" + local N=$(grep "$CANARY_STRING" $DIR/* 2>/dev/null |wc -l ) + test "$N" = "$EXPECTED_N" && msgpass || msgfail "Expected $EXPECTED_N canaries, got $N" +} + +LISTS='rootdir/var/lib/apt/lists' +rm -rf rootdir/var/lib/apt/lists +msgtest 'Got expected NODATA failure in' 'apt-get update' +aptget update -qq 2>&1 | grep -q 'E: GPG error.*NODATA' && msgpass || msgfail + +ensure_n_canary_strings_in_dir $LISTS 'ni ni ni' 0 +testequal 'partial' ls $LISTS + +# and again with pre-existing files with "valid data" which should remain +for f in Release Release.gpg main_binary-amd64_Packages main_source_Sources; do + echo 'peng neee-wom' > $LISTS/localhost:8080_dists_stable_${f} +done + +msgtest 'Got expected NODATA failure in' 'apt-get update' +aptget update -qq 2>&1 | grep -q 'E: GPG error.*NODATA' && msgpass || msgfail + +ensure_n_canary_strings_in_dir $LISTS 'peng neee-wom' 4 +ensure_n_canary_strings_in_dir $LISTS 'ni ni ni' 0 + +# and now with a pre-existing InRelease file +echo 'peng neee-wom' > $LISTS/localhost:8080_dists_stable_InRelease +rm -f $LISTS/localhost:8080_dists_stable_Release $LISTS/localhost:8080_dists_stable_Release.gpg +msgtest 'excpected failure of' 'apt-get update' +aptget update -qq 2>&1 | grep -q 'E: GPG error.*NODATA' && msgpass || msgfail + +ensure_n_canary_strings_in_dir $LISTS 'peng neee-wom' 3 +ensure_n_canary_strings_in_dir $LISTS 'ni ni ni' 0 diff --git a/test/interactive-helper/aptwebserver.cc b/test/interactive-helper/aptwebserver.cc index de235fa05..05b875673 100644 --- a/test/interactive-helper/aptwebserver.cc +++ b/test/interactive-helper/aptwebserver.cc @@ -435,6 +435,32 @@ int main(int const argc, const char * argv[]) } } + ::Configuration::Item const *Overwrite = _config->Tree("aptwebserver::overwrite"); + if (Overwrite != NULL) + { + for (::Configuration::Item *I = Overwrite->Child; I != NULL; I = I->Next) + { + regex_t *pattern = new regex_t; + int const res = regcomp(pattern, I->Tag.c_str(), REG_EXTENDED | REG_ICASE | REG_NOSUB); + if (res != 0) + { + char error[300]; + regerror(res, pattern, error, sizeof(error)); + sendError(client, 500, *m, sendContent, error); + continue; + } + if (regexec(pattern, filename.c_str(), 0, 0, 0) == 0) + { + filename = _config->Find("aptwebserver::overwrite::" + I->Tag + "::filename", filename); + if (filename[0] == '/') + filename.erase(0,1); + regfree(pattern); + break; + } + regfree(pattern); + } + } + // deal with the request if (RealFileExists(filename) == true) { -- cgit v1.2.3-70-g09d2 From 2b9c9b7f28b18f6ae3e422020e8934872b06c9f3 Mon Sep 17 00:00:00 2001 From: Raphael Geissert Date: Sun, 14 Jul 2013 18:38:03 +0200 Subject: Do not send a connection: keep-alive, at all --- methods/http.cc | 29 ++++++++--------------------- 1 file changed, 8 insertions(+), 21 deletions(-) (limited to 'methods') diff --git a/methods/http.cc b/methods/http.cc index db1085a2d..6e03e9d63 100644 --- a/methods/http.cc +++ b/methods/http.cc @@ -683,27 +683,14 @@ void HttpMethod::SendReq(FetchItem *Itm,CircleBuf &Out) if (Itm->Uri.length() >= sizeof(Buf)) abort(); - /* Build the request. We include a keep-alive header only for non-proxy - requests. This is to tweak old http/1.0 servers that do support keep-alive - but not HTTP/1.1 automatic keep-alive. Doing this with a proxy server - will glitch HTTP/1.0 proxies because they do not filter it out and - pass it on, HTTP/1.1 says the connection should default to keep alive - and we expect the proxy to do this */ - if (Proxy.empty() == true || Proxy.Host.empty()) - { - // see LP bugs #1003633 and #1086997. The "+" is encoded as a workaround - // for a amazon S3 bug - sprintf(Buf,"GET %s HTTP/1.1\r\nHost: %s\r\nConnection: keep-alive\r\n", - QuoteString(Uri.Path,"+~ ").c_str(),ProperHost.c_str()); - } - else - { - /* Generate a cache control header if necessary. We place a max - cache age on index files, optionally set a no-cache directive - and a no-store directive for archives. */ - sprintf(Buf,"GET %s HTTP/1.1\r\nHost: %s\r\n", - Itm->Uri.c_str(),ProperHost.c_str()); - } + /* Build the request. No keep-alive is included as it is the default + in 1.1, can cause problems with proxies, and we are an HTTP/1.1 + client anyway. + C.f. https://tools.ietf.org/wg/httpbis/trac/ticket/158 */ + // see LP bugs #1003633 and #1086997. The "+" is encoded as a workaround + // for a amazon S3 bug + sprintf(Buf,"GET %s HTTP/1.1\r\nHost: %s\r\n", + QuoteString(Uri.Path,"+~ ").c_str(),ProperHost.c_str()); // generate a cache control header (if needed) if (_config->FindB("Acquire::http::No-Cache",false) == true) { -- cgit v1.2.3-70-g09d2 From c104200045ef19f5ee061c4a00b468482ac65dc4 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 25 Jul 2013 20:16:31 +0200 Subject: fix off-by-one error in HttpMethod::​AutoDetectProxy() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- methods/http.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'methods') diff --git a/methods/http.cc b/methods/http.cc index db1085a2d..ec5b1ff52 100644 --- a/methods/http.cc +++ b/methods/http.cc @@ -1401,7 +1401,7 @@ bool HttpMethod::AutoDetectProxy() char buf[512]; int InFd = Pipes[0]; close(Pipes[1]); - int res = read(InFd, buf, sizeof(buf)); + int res = read(InFd, buf, sizeof(buf)-1); ExecWait(Process, "ProxyAutoDetect", true); if (res < 0) -- cgit v1.2.3-70-g09d2 From 1b7bf822ad9504f6d01cd4422d830e8815143912 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 25 Jul 2013 20:55:18 +0200 Subject: add missing "free(buffer) for allocated buffer --- apt-pkg/contrib/fileutl.cc | 1 - methods/gpgv.cc | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) (limited to 'methods') diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc index 398830ff5..0b6e07f75 100644 --- a/apt-pkg/contrib/fileutl.cc +++ b/apt-pkg/contrib/fileutl.cc @@ -251,7 +251,6 @@ int GetLock(string File,bool Errors) if (errno == ENOLCK) { - _error->Warning(_("Not using locking for nfs mounted lock file %s"),File.c_str()); return dup(0); // Need something for the caller to close } diff --git a/methods/gpgv.cc b/methods/gpgv.cc index fe8bac6c9..ea8a26fd4 100644 --- a/methods/gpgv.cc +++ b/methods/gpgv.cc @@ -152,6 +152,7 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile, } } fclose(pipein); + free(buffer); int status; waitpid(pid, &status, 0); -- cgit v1.2.3-70-g09d2 From f2380a78aa90ff8a3b76607c0c140810aff84086 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Fri, 26 Jul 2013 09:22:52 +0200 Subject: request absolute URIs from proxies again (0.9.9.3 regession) Commit 2b9c9b7f28b18f6ae3e422020e8934872b06c9f3 not only removes keep-alive, but also changes the request URI send to proxies which are required to be absolute URIs rather than the usual absolute paths. Closes: 717891 --- methods/http.cc | 20 +++++++++--- .../test-bug-717891-abolute-uris-for-proxies | 28 +++++++++++++++++ test/interactive-helper/aptwebserver.cc | 36 +++++++++++++++++----- 3 files changed, 72 insertions(+), 12 deletions(-) create mode 100755 test/integration/test-bug-717891-abolute-uris-for-proxies (limited to 'methods') diff --git a/methods/http.cc b/methods/http.cc index 6e03e9d63..82456d78b 100644 --- a/methods/http.cc +++ b/methods/http.cc @@ -682,15 +682,27 @@ void HttpMethod::SendReq(FetchItem *Itm,CircleBuf &Out) // Just in case. if (Itm->Uri.length() >= sizeof(Buf)) abort(); - + + /* RFC 2616 §5.1.2 requires absolute URIs for requests to proxies, + but while its a must for all servers to accept absolute URIs, + it is assumed clients will sent an absolute path for non-proxies */ + std::string requesturi; + if (Proxy.empty() == true || Proxy.Host.empty()) + requesturi = Uri.Path; + else + requesturi = Itm->Uri; + + // The "+" is encoded as a workaround for a amazon S3 bug + // see LP bugs #1003633 and #1086997. + requesturi = QuoteString(requesturi, "+~ "); + /* Build the request. No keep-alive is included as it is the default in 1.1, can cause problems with proxies, and we are an HTTP/1.1 client anyway. C.f. https://tools.ietf.org/wg/httpbis/trac/ticket/158 */ - // see LP bugs #1003633 and #1086997. The "+" is encoded as a workaround - // for a amazon S3 bug sprintf(Buf,"GET %s HTTP/1.1\r\nHost: %s\r\n", - QuoteString(Uri.Path,"+~ ").c_str(),ProperHost.c_str()); + requesturi.c_str(),ProperHost.c_str()); + // generate a cache control header (if needed) if (_config->FindB("Acquire::http::No-Cache",false) == true) { diff --git a/test/integration/test-bug-717891-abolute-uris-for-proxies b/test/integration/test-bug-717891-abolute-uris-for-proxies new file mode 100755 index 000000000..e9c38492e --- /dev/null +++ b/test/integration/test-bug-717891-abolute-uris-for-proxies @@ -0,0 +1,28 @@ +#!/bin/sh +set -e + +TESTDIR=$(readlink -f $(dirname $0)) +. $TESTDIR/framework +setupenvironment +configarchitecture 'amd64' + +buildsimplenativepackage 'unrelated' 'all' '0.5~squeeze1' 'unstable' + +setupaptarchive +changetowebserver --request-absolute='uri' + +msgtest 'Check that absolute paths are' 'not accepted' +aptget update >/dev/null 2>&1 && msgfail || msgpass + +echo 'Acquire::http::Proxy "http://localhost:8080";' > rootdir/etc/apt/apt.conf.d/99proxy + +msgtest 'Check that requests to proxies are' 'absolute uris' +aptget update >/dev/null 2>&1 && msgpass || msgfail + +testequal 'Reading package lists... +Building dependency tree... +The following NEW packages will be installed: + unrelated +0 upgraded, 1 newly installed, 0 to remove and 0 not upgraded. +Inst unrelated (0.5~squeeze1 unstable [all]) +Conf unrelated (0.5~squeeze1 unstable [all])' aptget install unrelated -s diff --git a/test/interactive-helper/aptwebserver.cc b/test/interactive-helper/aptwebserver.cc index a8d191d0e..fde95fec9 100644 --- a/test/interactive-helper/aptwebserver.cc +++ b/test/interactive-helper/aptwebserver.cc @@ -319,6 +319,33 @@ bool parseFirstLine(int const client, std::string const &request, /*{{{*/ sendError(client, 500, request, sendContent, "Filename contains an unencoded space"); return false; } + + std::string host = LookupTag(request, "Host", ""); + if (host.empty() == true) + { + // RFC 2616 §14.23 requires Host + sendError(client, 400, request, sendContent, "Host header is required"); + return false; + } + host = "http://" + host; + + // Proxies require absolute uris, so this is a simple proxy-fake option + std::string const absolute = _config->Find("aptwebserver::request::absolute", "uri,path"); + if (strncmp(host.c_str(), filename.c_str(), host.length()) == 0) + { + if (absolute.find("uri") == std::string::npos) + { + sendError(client, 400, request, sendContent, "Request is absoluteURI, but configured to not accept that"); + return false; + } + // strip the host from the request to make it an absolute path + filename.erase(0, host.length()); + } + else if (absolute.find("path") == std::string::npos) + { + sendError(client, 400, request, sendContent, "Request is absolutePath, but configured to not accept that"); + return false; + } filename = DeQuoteString(filename); // this is not a secure server, but at least prevent the obvious … @@ -342,6 +369,7 @@ int main(int const argc, const char * argv[]) { CommandLine::Args Args[] = { {0, "port", "aptwebserver::port", CommandLine::HasArg}, + {0, "request-absolute", "aptwebserver::request::absolute", CommandLine::HasArg}, {'c',"config-file",0,CommandLine::ConfigFile}, {'o',"option",0,CommandLine::ArbItem}, {0,0,0,0} @@ -447,14 +475,6 @@ int main(int const argc, const char * argv[]) if (parseFirstLine(client, *m, filename, sendContent, closeConnection) == false) continue; - std::string host = LookupTag(*m, "Host", ""); - if (host.empty() == true) - { - // RFC 2616 §14.23 requires Host - sendError(client, 400, *m, sendContent, "Host header is required"); - continue; - } - // string replacements in the requested filename ::Configuration::Item const *Replaces = _config->Tree("aptwebserver::redirect::replace"); if (Replaces != NULL) -- cgit v1.2.3-70-g09d2 From 11d0fb919954e79f929ef5e755f602a6ed3be46d Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Fri, 26 Jul 2013 22:12:36 +0200 Subject: fix missing va_end() --- methods/ftp.cc | 1 + methods/rsh.cc | 2 ++ 2 files changed, 3 insertions(+) (limited to 'methods') diff --git a/methods/ftp.cc b/methods/ftp.cc index d55ac1224..979adca62 100644 --- a/methods/ftp.cc +++ b/methods/ftp.cc @@ -436,6 +436,7 @@ bool FTPConn::WriteMsg(unsigned int &Ret,string &Text,const char *Fmt,...) char S[400]; vsnprintf(S,sizeof(S) - 4,Fmt,args); strcat(S,"\r\n"); + va_end(args); if (Debug == true) cerr << "-> '" << QuoteString(S,"") << "'" << endl; diff --git a/methods/rsh.cc b/methods/rsh.cc index fb3782314..d76dca6ef 100644 --- a/methods/rsh.cc +++ b/methods/rsh.cc @@ -218,6 +218,8 @@ bool RSHConn::WriteMsg(std::string &Text,bool Sync,const char *Fmt,...) // sprintf the description char S[512]; vsnprintf(S,sizeof(S) - 4,Fmt,args); + va_end(args); + if (Sync == true) strcat(S," 2> /dev/null || echo\n"); else -- cgit v1.2.3-70-g09d2