From 224dc038412459a5f64d4e7a16845847b7797a67 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Tue, 17 Jun 2014 16:55:19 +0200 Subject: fix SubstVar to be usable as a replace_all method The name suggests that it is supposed to substitute a variable with a value, but we tend to use it in a more liberal replace_all() fashion, but this breaks if either of the parameters is empty or more importantly if two "variable" occurrences follow each other directly. --- test/libapt/strutil_test.cc | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) (limited to 'test/libapt/strutil_test.cc') diff --git a/test/libapt/strutil_test.cc b/test/libapt/strutil_test.cc index bc004fd66..e9b778c6b 100644 --- a/test/libapt/strutil_test.cc +++ b/test/libapt/strutil_test.cc @@ -70,3 +70,38 @@ TEST(StrUtilTest,EndsWith) EXPECT_FALSE(Endswith("abcd", "x")); EXPECT_FALSE(Endswith("abcd", "abcndefg")); } +TEST(StrUtilTest,SubstVar) +{ + EXPECT_EQ("", SubstVar("", "fails", "passes")); + EXPECT_EQ("test ", SubstVar("test fails", "fails", "")); + EXPECT_EQ("test passes", SubstVar("test passes", "", "fails")); + + EXPECT_EQ("test passes", SubstVar("test passes", "fails", "passes")); + EXPECT_EQ("test passes", SubstVar("test fails", "fails", "passes")); + + EXPECT_EQ("starts with", SubstVar("beginnt with", "beginnt", "starts")); + EXPECT_EQ("beginnt with", SubstVar("starts with", "starts", "beginnt")); + EXPECT_EQ("is in middle", SubstVar("is in der middle", "in der", "in")); + EXPECT_EQ("is in der middle", SubstVar("is in middle", "in", "in der")); + EXPECT_EQ("does end", SubstVar("does enden", "enden", "end")); + EXPECT_EQ("does enden", SubstVar("does end", "end", "enden")); + + EXPECT_EQ("abc", SubstVar("abc", "d", "a")); + EXPECT_EQ("abc", SubstVar("abd", "d", "c")); + EXPECT_EQ("abc", SubstVar("adc", "d", "b")); + EXPECT_EQ("abc", SubstVar("dbc", "d", "a")); + + EXPECT_EQ("b", SubstVar("b", "aa", "a")); + EXPECT_EQ("bb", SubstVar("bb", "aa", "a")); + EXPECT_EQ("bbb", SubstVar("bbb", "aa", "a")); + + EXPECT_EQ("aa", SubstVar("aaaa", "aa", "a")); + EXPECT_EQ("aaaa", SubstVar("aa", "a", "aa")); + EXPECT_EQ("aaaa", SubstVar("aaaa", "a", "a")); + EXPECT_EQ("a a a a ", SubstVar("aaaa", "a", "a ")); + + EXPECT_EQ(" bb bb bb bb ", SubstVar(" a a a a ", "a", "bb")); + EXPECT_EQ(" bb bb bb bb ", SubstVar(" aaa aaa aaa aaa ", "aaa", "bb")); + EXPECT_EQ(" bb a bb a bb a bb ", SubstVar(" aaa a aaa a aaa a aaa ", "aaa", "bb")); + +} -- cgit v1.2.3-70-g09d2 From a5bb5e1e747ceb7b5a9defb6b1a8d9282a6e0957 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Tue, 8 Jul 2014 15:11:14 +0200 Subject: Only allow "apt-get build-dep path" when path starts with ./ or / This avoid the subtle problem that someone might have a directory with the same package name as the build-depends he/she is trying to fetch. Also print a note that the specific file/dir is used. --- apt-pkg/contrib/strutl.cc | 7 +++++++ apt-pkg/contrib/strutl.h | 1 + cmdline/apt-get.cc | 9 +++++++-- test/integration/test-apt-get-build-dep | 3 +++ test/libapt/strutil_test.cc | 9 +++++++++ 5 files changed, 27 insertions(+), 2 deletions(-) (limited to 'test/libapt/strutil_test.cc') diff --git a/apt-pkg/contrib/strutl.cc b/apt-pkg/contrib/strutl.cc index 0f48860b1..7948673dc 100644 --- a/apt-pkg/contrib/strutl.cc +++ b/apt-pkg/contrib/strutl.cc @@ -62,6 +62,13 @@ bool Endswith(const std::string &s, const std::string &end) return (s.substr(s.size() - end.size(), s.size()) == end); } +bool Startswith(const std::string &s, const std::string &start) +{ + if (start.size() > s.size()) + return false; + return (s.substr(0, start.size()) == start); +} + } } /*}}}*/ diff --git a/apt-pkg/contrib/strutl.h b/apt-pkg/contrib/strutl.h index 5733fd6e2..da8bebdb5 100644 --- a/apt-pkg/contrib/strutl.h +++ b/apt-pkg/contrib/strutl.h @@ -40,6 +40,7 @@ namespace APT { namespace String { std::string Strip(const std::string &s); bool Endswith(const std::string &s, const std::string &ending); + bool Startswith(const std::string &s, const std::string &starting); } } diff --git a/cmdline/apt-get.cc b/cmdline/apt-get.cc index bd866bc8c..cdbfa4708 100644 --- a/cmdline/apt-get.cc +++ b/cmdline/apt-get.cc @@ -1065,9 +1065,12 @@ static bool DoBuildDep(CommandLine &CmdL) string Src; pkgSrcRecords::Parser *Last = 0; - // a unpacked debian source tree - if (DirectoryExists(*I)) + // an unpacked debian source tree + using APT::String::Startswith; + if ((Startswith(*I, "./") || Startswith(*I, "/")) && + DirectoryExists(*I)) { + ioprintf(c1out, _("Note, using directory '%s' to get the build dependencies\n"), *I); // FIXME: how can we make this more elegant? std::string TypeName = "debian/control File Source Index"; pkgIndexFile::Type *Type = pkgIndexFile::Type::GetType(TypeName.c_str()); @@ -1077,6 +1080,8 @@ static bool DoBuildDep(CommandLine &CmdL) // if its a local file (e.g. .dsc) use this else if (FileExists(*I)) { + ioprintf(c1out, _("Note, using file '%s' to get the build dependencies\n"), *I); + // see if we can get a parser for this pkgIndexFile type string TypeName = flExtension(*I) + " File Source Index"; pkgIndexFile::Type *Type = pkgIndexFile::Type::GetType(TypeName.c_str()); diff --git a/test/integration/test-apt-get-build-dep b/test/integration/test-apt-get-build-dep index f71beae9c..87ec6e54d 100755 --- a/test/integration/test-apt-get-build-dep +++ b/test/integration/test-apt-get-build-dep @@ -34,6 +34,7 @@ EOF testequal "Reading package lists... Building dependency tree... +Note, using file '2vcard_0.5-3.dsc' to get the build dependencies The following NEW packages will be installed: build-essential debhelper 0 upgraded, 2 newly installed, 0 to remove and 0 not upgraded. @@ -76,6 +77,7 @@ EOF testequal "Reading package lists... Building dependency tree... +Note, using file '2vcard_0.5-3.dsc' to get the build dependencies The following NEW packages will be installed: build-essential debhelper 0 upgraded, 2 newly installed, 0 to remove and 0 not upgraded. @@ -117,6 +119,7 @@ EOF testequal "Reading package lists... Building dependency tree... +Note, using directory './foo-1.0' to get the build dependencies The following NEW packages will be installed: build-essential debhelper 0 upgraded, 2 newly installed, 0 to remove and 0 not upgraded. diff --git a/test/libapt/strutil_test.cc b/test/libapt/strutil_test.cc index e9b778c6b..1c2f0abac 100644 --- a/test/libapt/strutil_test.cc +++ b/test/libapt/strutil_test.cc @@ -70,6 +70,15 @@ TEST(StrUtilTest,EndsWith) EXPECT_FALSE(Endswith("abcd", "x")); EXPECT_FALSE(Endswith("abcd", "abcndefg")); } +TEST(StrUtilTest,StartWith) +{ + using APT::String::Startswith; + EXPECT_TRUE(Startswith("abcd", "a")); + EXPECT_TRUE(Startswith("abcd", "ab")); + EXPECT_TRUE(Startswith("abcd", "abcd")); + EXPECT_FALSE(Startswith("abcd", "x")); + EXPECT_FALSE(Startswith("abcd", "abcndefg")); +} TEST(StrUtilTest,SubstVar) { EXPECT_EQ("", SubstVar("", "fails", "passes")); -- cgit v1.2.3-70-g09d2 From b578738870e83db4e61d6f6591bc73884108b7d4 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sun, 7 Sep 2014 21:27:57 +0200 Subject: strip everything spacey in APT::String::Strip Git-Dch: Ignore --- apt-pkg/contrib/strutl.cc | 24 ++++++++++++++++++------ test/libapt/strutil_test.cc | 15 +++++++++++++++ 2 files changed, 33 insertions(+), 6 deletions(-) (limited to 'test/libapt/strutil_test.cc') diff --git a/apt-pkg/contrib/strutl.cc b/apt-pkg/contrib/strutl.cc index 922229e90..87f57a30e 100644 --- a/apt-pkg/contrib/strutl.cc +++ b/apt-pkg/contrib/strutl.cc @@ -45,14 +45,26 @@ using namespace std; // --------------------------------------------------------------------- namespace APT { namespace String { -std::string Strip(const std::string &s) +std::string Strip(const std::string &str) { - size_t start = s.find_first_not_of(" \t\n"); - // only whitespace - if (start == string::npos) + // ensure we have at least one character + if (str.empty() == true) + return str; + + char const * const s = str.c_str(); + size_t start = 0; + for (; isspace(s[start]) != 0; ++start) + ; // find the first not-space + + // string contains only whitespaces + if (s[start] == '\0') return ""; - size_t end = s.find_last_not_of(" \t\n"); - return s.substr(start, end-start+1); + + size_t end = str.length() - 1; + for (; isspace(s[end]) != 0; --end) + ; // find the last not-space + + return str.substr(start, end - start + 1); } bool Endswith(const std::string &s, const std::string &end) diff --git a/test/libapt/strutil_test.cc b/test/libapt/strutil_test.cc index e9b778c6b..194c9c074 100644 --- a/test/libapt/strutil_test.cc +++ b/test/libapt/strutil_test.cc @@ -19,6 +19,21 @@ TEST(StrUtilTest,DeEscapeString) EXPECT_EQ("foo\\ x", DeEscapeString("foo\\\\ x")); EXPECT_EQ("\\foo\\", DeEscapeString("\\\\foo\\\\")); } +TEST(StrUtilTest,StringStrip) +{ + EXPECT_EQ("", APT::String::Strip("")); + EXPECT_EQ("foobar", APT::String::Strip("foobar")); + EXPECT_EQ("foo bar", APT::String::Strip("foo bar")); + + EXPECT_EQ("", APT::String::Strip(" ")); + EXPECT_EQ("", APT::String::Strip(" \r\n \t ")); + + EXPECT_EQ("foo bar", APT::String::Strip("foo bar ")); + EXPECT_EQ("foo bar", APT::String::Strip("foo bar \r\n \t ")); + EXPECT_EQ("foo bar", APT::String::Strip("\r\n \t foo bar")); + EXPECT_EQ("bar foo", APT::String::Strip("\r\n \t bar foo \r\n \t ")); + EXPECT_EQ("bar \t\r\n foo", APT::String::Strip("\r\n \t bar \t\r\n foo \r\n \t ")); +} TEST(StrUtilTest,StringSplitBasic) { std::vector result = StringSplit("", ""); -- cgit v1.2.3-70-g09d2 From b0314abb0cbe5937d2d3cdcd6df9d322b69d03a0 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Thu, 23 Oct 2014 10:42:18 +0200 Subject: add test for Basic Authentication scheme Git-Dch: Ignore --- test/integration/framework | 31 ++++++--- test/integration/test-apt-cdrom | 7 +- test/integration/test-authentication-basic | 106 +++++++++++++++++++++++++++++ test/interactive-helper/aptwebserver.cc | 75 ++++++++++++++++++-- test/libapt/strutil_test.cc | 16 ++++- 5 files changed, 219 insertions(+), 16 deletions(-) create mode 100755 test/integration/test-authentication-basic (limited to 'test/libapt/strutil_test.cc') diff --git a/test/integration/framework b/test/integration/framework index 23ff0983b..fcaa4ddd3 100644 --- a/test/integration/framework +++ b/test/integration/framework @@ -944,23 +944,35 @@ signreleasefiles() { } webserverconfig() { - msgtest "Set webserver config option '${1}' to" "$2" + local NOCHECK=false + if [ "$1" = '--no-check' ]; then + NOCHECK=true + shift + fi local DOWNLOG='rootdir/tmp/download-testfile.log' - local STATUS='rootdir/tmp/webserverconfig.status' + local STATUS='downloaded/webserverconfig.status' rm -f "$STATUS" "$DOWNLOG" - if downloadfile "http://localhost:8080/_config/set/${1}/${2}" "$STATUS" > "$DOWNLOG"; then + local URI + if [ -n "$2" ]; then + msgtest "Set webserver config option '${1}' to" "$2" + URI="http://localhost:8080/_config/set/${1}/${2}" + else + msgtest 'Clear webserver config option' "${1}" + URI="http://localhost:8080/_config/clear/${1}" + fi + if downloadfile "$URI" "$STATUS" > "$DOWNLOG"; then msgpass else - cat "$DOWNLOG" "$STATUS" + cat "$DOWNLOG" "$STATUS" || true msgfail fi - testwebserverlaststatuscode '200' + $NOCHECK || testwebserverlaststatuscode '200' } rewritesourceslist() { local APTARCHIVE="file://$(readlink -f "${TMPWORKINGDIRECTORY}/aptarchive")" for LIST in $(find rootdir/etc/apt/sources.list.d/ -name 'apt-test-*.list'); do - sed -i $LIST -e "s#$APTARCHIVE#${1}#" -e "s#http://localhost:8080/#${1}#" -e "s#http://localhost:4433/#${1}#" + sed -i $LIST -e "s#$APTARCHIVE#${1}#" -e "s#http://localhost:8080/#${1}#" -e "s#https://localhost:4433/#${1}#" done } @@ -1047,7 +1059,7 @@ acquire::cdrom::autodetect 0;" > rootdir/etc/apt/apt.conf.d/00cdrom mv "${CD}" "${CD}-unmounted" # we don't want the disk to be modifiable addtrap 'prefix' "chmod -f -R +w $PWD/rootdir/media/cdrom/dists/ $PWD/rootdir/media/cdrom-unmounted/dists/ || true;" - chmod -R -w rootdir/media/cdrom-unmounted/dists + chmod -R 555 rootdir/media/cdrom-unmounted/dists } downloadfile() { @@ -1055,7 +1067,7 @@ downloadfile() { apthelper -o Debug::Acquire::${PROTO}=1 \ download-file "$1" "$2" 2>&1 || true # only if the file exists the download was successful - if [ -e "$2" ]; then + if [ -r "$2" ]; then return 0 else return 1 @@ -1312,8 +1324,7 @@ testwebserverlaststatuscode() { local STATUS='downloaded/webserverstatus-statusfile.log' rm -f "$DOWNLOG" "$STATUS" msgtest 'Test last status code from the webserver was' "$1" - downloadfile "http://localhost:8080/_config/find/aptwebserver::last-status-code" "$STATUS" > "$DOWNLOG" - if [ "$(cat "$STATUS")" = "$1" ]; then + if downloadfile "http://localhost:8080/_config/find/aptwebserver::last-status-code" "$STATUS" > "$DOWNLOG" && [ "$(cat "$STATUS")" = "$1" ]; then msgpass else echo >&2 diff --git a/test/integration/test-apt-cdrom b/test/integration/test-apt-cdrom index 4489143e4..3a33219fe 100755 --- a/test/integration/test-apt-cdrom +++ b/test/integration/test-apt-cdrom @@ -21,7 +21,7 @@ echo 'Description-de: automatisch generiertes Testpaket testing=0.8.15/stable ' >> Translation-de compressfile Translation-de rm -f Translation-en Translation-de -chmod -R -w . +chmod -R 555 . cd - > /dev/null aptcdromlog() { @@ -144,3 +144,8 @@ testcdromusage testsuccess aptget update testfileequal rootdir/tmp/testsuccess.output 'Reading package lists...' testcdromusage + +msgmsg 'Check that nothing touched our' 'CD-ROM' +for file in $(find rootdir/media/cdrom-unmounted/dists); do + testfilestats "$file" '%U:%G:%a' '=' "${USER}:${USER}:555" +done diff --git a/test/integration/test-authentication-basic b/test/integration/test-authentication-basic new file mode 100755 index 000000000..4b0ead54a --- /dev/null +++ b/test/integration/test-authentication-basic @@ -0,0 +1,106 @@ +#!/bin/sh +set -e + +TESTDIR=$(readlink -f $(dirname $0)) +. $TESTDIR/framework + +setupenvironment +configarchitecture 'i386' + +insertpackage 'unstable' 'foo' 'all' '1' +setupaptarchive --no-update + +changetohttpswebserver --authorization="$(printf '%s' 'star:hunter2' | base64 )" + +echo 'See, when YOU type hunter2, it shows to us as *******' > aptarchive/bash + +testauthfailure() { + testfailure apthelper download-file "${1}/bash" ./downloaded/bash + # crappy test, but http and https output are wastely different… + testsuccess grep 401 rootdir/tmp/testfailure.output + testsuccess test ! -s ./downloaded/bash +} + +testauthsuccess() { + testsuccess apthelper download-file "${1}/bash" ./downloaded/bash + testfileequal ./downloaded/bash "$(cat aptarchive/bash)" + testfilestats ./downloaded/bash '%U:%G:%a' '=' "${USER}:${USER}:644" + rm -f ./downloaded/bash + + # lets see if got/retains acceptable permissions + if [ -n "$AUTHCONF" ]; then + if [ "$(id -u)" = '0' ]; then + testfilestats "$AUTHCONF" '%U:%G:%a' '=' "_apt:root:600" + else + testfilestats "$AUTHCONF" '%U:%G:%a' '=' "${USER}:${USER}:600" + fi + fi + + rm -rf rootdir/var/lib/apt/lists + testsuccess aptget update + testequal 'Reading package lists... +Building dependency tree... +The following NEW packages will be installed: + foo +0 upgraded, 1 newly installed, 0 to remove and 0 not upgraded. +Inst foo (1 unstable [all]) +Conf foo (1 unstable [all])' aptget install foo -s +} + +authfile() { + local AUTHCONF='rootdir/etc/apt/auth.conf' + rm -f "$AUTHCONF" + printf '%s' "$1" > "$AUTHCONF" + chmod 600 "$AUTHCONF" +} + +runtest() { + # unauthorized fails + authfile '' + testauthfailure "$1" + + # good auth + authfile 'machine localhost +login star +password hunter2' + testauthsuccess "$1" + + # bad auth + authfile 'machine localhost +login anonymous +password hunter2' + testauthfailure "$1" + + # 2 stanzas: unmatching + good auth + authfile 'machine debian.org +login debian +password jessie + +machine localhost +login star +password hunter2' + testauthsuccess "$1" +} + +msgmsg 'server basic auth' +rewritesourceslist 'http://localhost:8080' +runtest 'http://localhost:8080' +rewritesourceslist 'https://localhost:4433' +runtest 'https://localhost:4433' +rewritesourceslist 'http://localhost:8080' + +msgmsg 'proxy to server basic auth' +webserverconfig 'aptwebserver::request::absolute' 'uri' +export http_proxy='http://localhost:8080' +runtest 'http://localhost:8080' +unset http_proxy + +msgmsg 'proxy basic auth to server basic auth' +webserverconfig 'aptwebserver::proxy-authorization' "$(printf 'moon:deer2' | base64)" +export http_proxy='http://moon:deer2@localhost:8080' +runtest 'http://localhost:8080' + +msgmsg 'proxy basic auth to server' +authfile '' +webserverconfig 'aptwebserver::authorization' '' +testauthsuccess 'http://localhost:8080' diff --git a/test/interactive-helper/aptwebserver.cc b/test/interactive-helper/aptwebserver.cc index 34476e1af..7474cf148 100644 --- a/test/interactive-helper/aptwebserver.cc +++ b/test/interactive-helper/aptwebserver.cc @@ -157,9 +157,8 @@ static bool sendData(int const client, std::string const &data) /*{{{*/ } /*}}}*/ static void sendError(int const client, int const httpcode, std::string const &request,/*{{{*/ - bool content, std::string const &error = "") + bool content, std::string const &error = "", std::list headers = std::list()) { - std::list headers; std::string response(""); response.append(httpcodeToStr(httpcode)).append(""); response.append("

").append(httpcodeToStr(httpcode)).append("

"); @@ -367,22 +366,88 @@ static bool parseFirstLine(int const client, std::string const &request,/*{{{*/ // 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 (strncmp(host.c_str(), filename.c_str(), host.length()) == 0 && APT::String::Startswith(filename, "/_config/") == false) { 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()); + + std::string const authConf = _config->Find("aptwebserver::proxy-authorization", ""); + std::string auth = LookupTag(request, "Proxy-Authorization", ""); + if (authConf.empty() != auth.empty()) + { + if (auth.empty()) + sendError(client, 407, request, sendContent, "Proxy requires authentication"); + else + sendError(client, 407, request, sendContent, "Client wants to authenticate to proxy, but proxy doesn't need it"); + return false; + } + if (authConf.empty() == false) + { + char const * const basic = "Basic "; + if (strncmp(auth.c_str(), basic, strlen(basic)) == 0) + { + auth.erase(0, strlen(basic)); + if (auth != authConf) + { + sendError(client, 407, request, sendContent, "Proxy-Authentication doesn't match"); + return false; + } + } + else + { + std::list headers; + headers.push_back("Proxy-Authenticate: Basic"); + sendError(client, 407, request, sendContent, "Unsupported Proxy-Authentication Scheme", headers); + return false; + } + } } - else if (absolute.find("path") == std::string::npos) + else if (absolute.find("path") == std::string::npos && APT::String::Startswith(filename, "/_config/") == false) { sendError(client, 400, request, sendContent, "Request is absolutePath, but configured to not accept that"); return false; } + if (APT::String::Startswith(filename, "/_config/") == false) + { + std::string const authConf = _config->Find("aptwebserver::authorization", ""); + std::string auth = LookupTag(request, "Authorization", ""); + if (authConf.empty() != auth.empty()) + { + if (auth.empty()) + sendError(client, 401, request, sendContent, "Server requires authentication"); + else + sendError(client, 401, request, sendContent, "Client wants to authenticate to server, but server doesn't need it"); + return false; + } + if (authConf.empty() == false) + { + char const * const basic = "Basic "; + if (strncmp(auth.c_str(), basic, strlen(basic)) == 0) + { + auth.erase(0, strlen(basic)); + if (auth != authConf) + { + sendError(client, 401, request, sendContent, "Authentication doesn't match"); + return false; + } + } + else + { + std::list headers; + headers.push_back("WWW-Authenticate: Basic"); + sendError(client, 401, request, sendContent, "Unsupported Authentication Scheme", headers); + return false; + } + } + } + size_t paramspos = filename.find('?'); if (paramspos != std::string::npos) { @@ -656,6 +721,8 @@ 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}, + {0, "authorization", "aptwebserver::authorization", CommandLine::HasArg}, + {0, "proxy-authorization", "aptwebserver::proxy-authorization", CommandLine::HasArg}, {'c',"config-file",0,CommandLine::ConfigFile}, {'o',"option",0,CommandLine::ArbItem}, {0,0,0,0} diff --git a/test/libapt/strutil_test.cc b/test/libapt/strutil_test.cc index 8dd9114ec..494159c87 100644 --- a/test/libapt/strutil_test.cc +++ b/test/libapt/strutil_test.cc @@ -85,7 +85,7 @@ TEST(StrUtilTest,EndsWith) EXPECT_FALSE(Endswith("abcd", "x")); EXPECT_FALSE(Endswith("abcd", "abcndefg")); } -TEST(StrUtilTest,StartWith) +TEST(StrUtilTest,StartsWith) { using APT::String::Startswith; EXPECT_TRUE(Startswith("abcd", "a")); @@ -129,3 +129,17 @@ TEST(StrUtilTest,SubstVar) EXPECT_EQ(" bb a bb a bb a bb ", SubstVar(" aaa a aaa a aaa a aaa ", "aaa", "bb")); } +TEST(StrUtilTest,Base64Encode) +{ + EXPECT_EQ("QWxhZGRpbjpvcGVuIHNlc2FtZQ==", Base64Encode("Aladdin:open sesame")); + EXPECT_EQ("cGxlYXN1cmUu", Base64Encode("pleasure.")); + EXPECT_EQ("bGVhc3VyZS4=", Base64Encode("leasure.")); + EXPECT_EQ("ZWFzdXJlLg==", Base64Encode("easure.")); + EXPECT_EQ("YXN1cmUu", Base64Encode("asure.")); + EXPECT_EQ("c3VyZS4=", Base64Encode("sure.")); + EXPECT_EQ("dXJlLg==", Base64Encode("ure.")); + EXPECT_EQ("cmUu", Base64Encode("re.")); + EXPECT_EQ("ZS4=", Base64Encode("e.")); + EXPECT_EQ("Lg==", Base64Encode(".")); + EXPECT_EQ("", Base64Encode("")); +} -- cgit v1.2.3-70-g09d2 From d8c71b3b5dc98daa247433503ad8242c9e7b77db Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Fri, 24 Oct 2014 23:55:15 +0200 Subject: rewrite ReadMessages() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Central methods of our infrastructure like this one responsible for communication with our methods shouldn't be more complicated then they have to and not claim to have (albeit unlikely) bugs. While I am not sure about having improved the first part, the bug is now gone and a few explicit tests check that it stays that way, so nobody will notice the difference (hopefully) – expect that this should a very tiny bit faster as well as we don't manually proceed through the string. Git-Dch: Ignore --- apt-pkg/contrib/strutl.cc | 124 +++++++++++++++++++++++--------------------- test/libapt/strutil_test.cc | 70 +++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 58 deletions(-) (limited to 'test/libapt/strutil_test.cc') diff --git a/apt-pkg/contrib/strutl.cc b/apt-pkg/contrib/strutl.cc index aad358a55..ebf9c9ea6 100644 --- a/apt-pkg/contrib/strutl.cc +++ b/apt-pkg/contrib/strutl.cc @@ -779,86 +779,94 @@ string TimeRFC1123(time_t Date) In particular: this reads blocks from the input until it believes that it's run out of input text. Each block is terminated by a - double newline ('\n' followed by '\n'). As noted below, there is a - bug in this code: it assumes that all the blocks have been read if - it doesn't see additional text in the buffer after the last one is - parsed, which will cause it to lose blocks if the last block - coincides with the end of the buffer. + double newline ('\n' followed by '\n'). */ bool ReadMessages(int Fd, vector &List) { char Buffer[64000]; - char *End = Buffer; // Represents any left-over from the previous iteration of the // parse loop. (i.e., if a message is split across the end // of the buffer, it goes here) string PartialMessage; - - while (1) - { - int Res = read(Fd,End,sizeof(Buffer) - (End-Buffer)); + + do { + int const Res = read(Fd, Buffer, sizeof(Buffer)); if (Res < 0 && errno == EINTR) continue; - - // Process is dead, this is kind of bad.. + + // process we read from has died if (Res == 0) return false; - + // No data - if (Res < 0 && errno == EAGAIN) + if (Res < 0 && (errno == EAGAIN || errno == EWOULDBLOCK)) return true; if (Res < 0) return false; - - End += Res; - - // Look for the end of the message - for (char *I = Buffer; I + 1 < End; I++) + + // extract the message(s) from the buffer + char const *Start = Buffer; + char const * const End = Buffer + Res; + + char const * NL = (char const *) memchr(Start, '\n', End - Start); + if (NL == NULL) { - if (I[1] != '\n' || - (I[0] != '\n' && strncmp(I, "\r\n\r\n", 4) != 0)) - continue; - - // Pull the message out - string Message(Buffer,I-Buffer); - PartialMessage += Message; - - // Fix up the buffer - for (; I < End && (*I == '\n' || *I == '\r'); ++I); - End -= I-Buffer; - memmove(Buffer,I,End-Buffer); - I = Buffer; - - List.push_back(PartialMessage); - PartialMessage.clear(); + // end of buffer: store what we have so far and read new data in + PartialMessage.append(Start, End - Start); + Start = End; } - if (End != Buffer) - { - // If there's text left in the buffer, store it - // in PartialMessage and throw the rest of the buffer - // away. This allows us to handle messages that - // are longer than the static buffer size. - PartialMessage += string(Buffer, End); - End = Buffer; - } else - { - // BUG ALERT: if a message block happens to end at a - // multiple of 64000 characters, this will cause it to - // terminate early, leading to a badly formed block and - // probably crashing the method. However, this is the only - // way we have to find the end of the message block. I have - // an idea of how to fix this, but it will require changes - // to the protocol (essentially to mark the beginning and - // end of the block). - // - // -- dburrows 2008-04-02 - return true; - } + ++NL; + + if (PartialMessage.empty() == false && Start < End) + { + // if we start with a new line, see if the partial message we have ended with one + // so that we properly detect records ending between two read() runs + // cases are: \n|\n , \r\n|\r\n and \r\n\r|\n + // the case \r|\n\r\n is handled by the usual double-newline handling + if ((NL - Start) == 1 || ((NL - Start) == 2 && *Start == '\r')) + { + if (APT::String::Endswith(PartialMessage, "\n") || APT::String::Endswith(PartialMessage, "\r\n\r")) + { + PartialMessage.erase(PartialMessage.find_last_not_of("\r\n") + 1); + List.push_back(PartialMessage); + PartialMessage.clear(); + while (NL < End && (*NL == '\n' || *NL == '\r')) ++NL; + Start = NL; + } + } + } + + while (Start < End) { + char const * NL2 = (char const *) memchr(NL, '\n', End - NL); + if (NL2 == NULL) + { + // end of buffer: store what we have so far and read new data in + PartialMessage.append(Start, End - Start); + break; + } + ++NL2; + + // did we find a double newline? + if ((NL2 - NL) == 1 || ((NL2 - NL) == 2 && *NL == '\r')) + { + PartialMessage.append(Start, NL2 - Start); + PartialMessage.erase(PartialMessage.find_last_not_of("\r\n") + 1); + List.push_back(PartialMessage); + PartialMessage.clear(); + while (NL2 < End && (*NL2 == '\n' || *NL2 == '\r')) ++NL2; + Start = NL2; + } + NL = NL2; + } + + // we have read at least one complete message and nothing left + if (PartialMessage.empty() == true) + return true; if (WaitFd(Fd) == false) return false; - } + } while (true); } /*}}}*/ // MonthConv - Converts a month string into a number /*{{{*/ diff --git a/test/libapt/strutil_test.cc b/test/libapt/strutil_test.cc index 494159c87..9bc3c76fd 100644 --- a/test/libapt/strutil_test.cc +++ b/test/libapt/strutil_test.cc @@ -1,10 +1,13 @@ #include #include +#include #include #include #include +#include "file-helpers.h" + TEST(StrUtilTest,DeEscapeString) { // nothing special @@ -143,3 +146,70 @@ TEST(StrUtilTest,Base64Encode) EXPECT_EQ("Lg==", Base64Encode(".")); EXPECT_EQ("", Base64Encode("")); } +void ReadMessagesTestWithNewLine(char const * const nl, char const * const ab) +{ + SCOPED_TRACE(SubstVar(SubstVar(nl, "\n", "n"), "\r", "r") + " # " + ab); + FileFd fd; + std::string pkgA = "Package: pkgA\n" + "Version: 1\n" + "Size: 100\n" + "Description: aaa\n" + " aaa"; + std::string pkgB = "Package: pkgB\n" + "Version: 1\n" + "Flag: no\n" + "Description: bbb"; + std::string pkgC = "Package: pkgC\n" + "Version: 2\n" + "Flag: yes\n" + "Description:\n" + " ccc"; + + createTemporaryFile("readmessage", fd, NULL, (pkgA + nl + pkgB + nl + pkgC + nl).c_str()); + std::vector list; + EXPECT_TRUE(ReadMessages(fd.Fd(), list)); + EXPECT_EQ(3, list.size()); + EXPECT_EQ(pkgA, list[0]); + EXPECT_EQ(pkgB, list[1]); + EXPECT_EQ(pkgC, list[2]); + + size_t const msgsize = 63990; + createTemporaryFile("readmessage", fd, NULL, NULL); + for (size_t j = 0; j < msgsize; ++j) + fd.Write(ab, strlen(ab)); + for (size_t i = 0; i < 21; ++i) + { + std::string msg; + strprintf(msg, "msgsize=%zu i=%zu", msgsize, i); + SCOPED_TRACE(msg); + fd.Seek((msgsize + (i - 1)) * strlen(ab)); + fd.Write(ab, strlen(ab)); + fd.Write(nl, strlen(nl)); + fd.Seek(0); + list.clear(); + EXPECT_TRUE(ReadMessages(fd.Fd(), list)); + EXPECT_EQ(1, list.size()); + EXPECT_EQ((msgsize + i) * strlen(ab), list[0].length()); + EXPECT_EQ(std::string::npos, list[0].find_first_not_of(ab)); + } + + list.clear(); + fd.Write(pkgA.c_str(), pkgA.length()); + fd.Write(nl, strlen(nl)); + fd.Seek(0); + EXPECT_TRUE(ReadMessages(fd.Fd(), list)); + EXPECT_EQ(2, list.size()); + EXPECT_EQ((msgsize + 20) * strlen(ab), list[0].length()); + EXPECT_EQ(std::string::npos, list[0].find_first_not_of(ab)); + EXPECT_EQ(pkgA, list[1]); + + + fd.Close(); +} +TEST(StrUtilTest,ReadMessages) +{ + ReadMessagesTestWithNewLine("\n\n", "a"); + ReadMessagesTestWithNewLine("\r\n\r\n", "a"); + ReadMessagesTestWithNewLine("\n\n", "ab"); + ReadMessagesTestWithNewLine("\r\n\r\n", "ab"); +} -- cgit v1.2.3-70-g09d2 From b8eba208daebe3e3f235983e44da9c398d6f7a57 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Tue, 10 Mar 2015 14:11:54 +0100 Subject: reimplement the last uses of sprintf Working with strings c-style is complicated and error-prune, so by converting to c++ style we gain some simplicity and avoid buffer overflows by later extensions. Git-Dch: Ignore --- apt-pkg/contrib/cdromutl.cc | 31 ++++---- apt-pkg/contrib/strutl.cc | 126 ++++++++++++++------------------ apt-private/acqprogress.cc | 173 ++++++++++++++++++++++---------------------- apt-private/acqprogress.h | 4 +- ftparchive/writer.cc | 8 +- methods/ftp.cc | 16 ++-- test/libapt/strutil_test.cc | 22 ++++++ test/libapt/uri_test.cc | 12 +++ 8 files changed, 206 insertions(+), 186 deletions(-) (limited to 'test/libapt/strutil_test.cc') diff --git a/apt-pkg/contrib/cdromutl.cc b/apt-pkg/contrib/cdromutl.cc index 936e377fb..6eb917457 100644 --- a/apt-pkg/contrib/cdromutl.cc +++ b/apt-pkg/contrib/cdromutl.cc @@ -207,7 +207,6 @@ bool IdentCdrom(string CD,string &Res,unsigned int Version) /* Run over the directory, we assume that the reader order will never change as the media is read-only. In theory if the kernel did some sort of wacked caching this might not be true.. */ - char S[300]; for (struct dirent *Dir = readdir(D); Dir != 0; Dir = readdir(D)) { // Skip some files.. @@ -215,30 +214,32 @@ bool IdentCdrom(string CD,string &Res,unsigned int Version) strcmp(Dir->d_name,"..") == 0) continue; + std::string S; if (Version <= 1) { - sprintf(S,"%lu",(unsigned long)Dir->d_ino); + strprintf(S, "%lu", (unsigned long)Dir->d_ino); } else { struct stat Buf; if (stat(Dir->d_name,&Buf) != 0) continue; - sprintf(S,"%lu",(unsigned long)Buf.st_mtime); + strprintf(S, "%lu", (unsigned long)Buf.st_mtime); } - - Hash.Add(S); + + Hash.Add(S.c_str()); Hash.Add(Dir->d_name); }; - + if (chdir(StartDir.c_str()) != 0) { _error->Errno("chdir",_("Unable to change to %s"),StartDir.c_str()); closedir(D); return false; } closedir(D); - + // Some stats from the fsys + std::string S; if (_config->FindB("Debug::identcdrom",false) == false) { struct statvfs Buf; @@ -248,19 +249,19 @@ bool IdentCdrom(string CD,string &Res,unsigned int Version) // We use a kilobyte block size to advoid overflow if (writable_media) { - sprintf(S,"%lu",(long)(Buf.f_blocks*(Buf.f_bsize/1024))); + strprintf(S, "%lu", (unsigned long)(Buf.f_blocks*(Buf.f_bsize/1024))); } else { - sprintf(S,"%lu %lu",(long)(Buf.f_blocks*(Buf.f_bsize/1024)), - (long)(Buf.f_bfree*(Buf.f_bsize/1024))); + strprintf(S, "%lu %lu", (unsigned long)(Buf.f_blocks*(Buf.f_bsize/1024)), + (unsigned long)(Buf.f_bfree*(Buf.f_bsize/1024))); } - Hash.Add(S); - sprintf(S,"-%u",Version); + Hash.Add(S.c_str()); + strprintf(S, "-%u", Version); } else - sprintf(S,"-%u.debug",Version); - + strprintf(S, "-%u.debug", Version); + Res = Hash.Result().Value() + S; - return true; + return true; } /*}}}*/ // FindMountPointForDevice - Find mountpoint for the given device /*{{{*/ diff --git a/apt-pkg/contrib/strutl.cc b/apt-pkg/contrib/strutl.cc index 0ac587a9e..0db4c57b2 100644 --- a/apt-pkg/contrib/strutl.cc +++ b/apt-pkg/contrib/strutl.cc @@ -324,21 +324,19 @@ bool ParseCWord(const char *&String,string &Res) /* */ string QuoteString(const string &Str, const char *Bad) { - string Res; + std::stringstream Res; for (string::const_iterator I = Str.begin(); I != Str.end(); ++I) { - if (strchr(Bad,*I) != 0 || isprint(*I) == 0 || + if (strchr(Bad,*I) != 0 || isprint(*I) == 0 || *I == 0x25 || // percent '%' char *I <= 0x20 || *I >= 0x7F) // control chars { - char Buf[10]; - sprintf(Buf,"%%%02x",(int)*I); - Res += Buf; + ioprintf(Res,"%%%02x",(int)*I); } else - Res += *I; + Res << *I; } - return Res; + return Res.str(); } /*}}}*/ // DeQuoteString - Convert a string from quoted from /*{{{*/ @@ -379,13 +377,12 @@ string DeQuoteString(string::const_iterator const &begin, YottaBytes (E24) */ string SizeToStr(double Size) { - char S[300]; double ASize; if (Size >= 0) ASize = Size; else ASize = -1*Size; - + /* bytes, KiloBytes, MegaBytes, GigaBytes, TeraBytes, PetaBytes, ExaBytes, ZettaBytes, YottaBytes */ char Ext[] = {'\0','k','M','G','T','P','E','Z','Y'}; @@ -394,20 +391,21 @@ string SizeToStr(double Size) { if (ASize < 100 && I != 0) { - sprintf(S,"%'.1f %c",ASize,Ext[I]); - break; + std::string S; + strprintf(S, "%'.1f %c", ASize, Ext[I]); + return S; } - + if (ASize < 10000) { - sprintf(S,"%'.0f %c",ASize,Ext[I]); - break; + std::string S; + strprintf(S, "%'.0f %c", ASize, Ext[I]); + return S; } ASize /= 1000.0; I++; } - - return S; + return ""; } /*}}}*/ // TimeToStr - Convert the time into a string /*{{{*/ @@ -415,36 +413,27 @@ string SizeToStr(double Size) /* Converts a number of seconds to a hms format */ string TimeToStr(unsigned long Sec) { - char S[300]; - - while (1) + std::string S; + if (Sec > 60*60*24) { - if (Sec > 60*60*24) - { - //d means days, h means hours, min means minutes, s means seconds - sprintf(S,_("%lid %lih %limin %lis"),Sec/60/60/24,(Sec/60/60) % 24,(Sec/60) % 60,Sec % 60); - break; - } - - if (Sec > 60*60) - { - //h means hours, min means minutes, s means seconds - sprintf(S,_("%lih %limin %lis"),Sec/60/60,(Sec/60) % 60,Sec % 60); - break; - } - - if (Sec > 60) - { - //min means minutes, s means seconds - sprintf(S,_("%limin %lis"),Sec/60,Sec % 60); - break; - } - - //s means seconds - sprintf(S,_("%lis"),Sec); - break; + //TRANSLATOR: d means days, h means hours, min means minutes, s means seconds + strprintf(S,_("%lid %lih %limin %lis"),Sec/60/60/24,(Sec/60/60) % 24,(Sec/60) % 60,Sec % 60); + } + else if (Sec > 60*60) + { + //TRANSLATOR: h means hours, min means minutes, s means seconds + strprintf(S,_("%lih %limin %lis"),Sec/60/60,(Sec/60) % 60,Sec % 60); + } + else if (Sec > 60) + { + //TRANSLATOR: min means minutes, s means seconds + strprintf(S,_("%limin %lis"),Sec/60,Sec % 60); + } + else + { + //TRANSLATOR: s means seconds + strprintf(S,_("%lis"),Sec); } - return S; } /*}}}*/ @@ -1423,7 +1412,7 @@ size_t strv_length(const char **str_array) ; return i; } - + /*}}}*/ // DeEscapeString - unescape (\0XX and \xXX) from a string /*{{{*/ // --------------------------------------------------------------------- /* */ @@ -1605,51 +1594,46 @@ void URI::CopyFrom(const string &U) /* */ URI::operator string() { - string Res; - + std::stringstream Res; + if (Access.empty() == false) - Res = Access + ':'; - + Res << Access << ':'; + if (Host.empty() == false) - { + { if (Access.empty() == false) - Res += "//"; - + Res << "//"; + if (User.empty() == false) { // FIXME: Technically userinfo is permitted even less // characters than these, but this is not conveniently // expressed with a blacklist. - Res += QuoteString(User, ":/?#[]@"); + Res << QuoteString(User, ":/?#[]@"); if (Password.empty() == false) - Res += ":" + QuoteString(Password, ":/?#[]@"); - Res += "@"; + Res << ":" << QuoteString(Password, ":/?#[]@"); + Res << "@"; } - + // Add RFC 2732 escaping characters - if (Access.empty() == false && - (Host.find('/') != string::npos || Host.find(':') != string::npos)) - Res += '[' + Host + ']'; + if (Access.empty() == false && Host.find_first_of("/:") != string::npos) + Res << '[' << Host << ']'; else - Res += Host; - + Res << Host; + if (Port != 0) - { - char S[30]; - sprintf(S,":%u",Port); - Res += S; - } + Res << ':' << Port; } - + if (Path.empty() == false) { if (Path[0] != '/') - Res += "/" + Path; + Res << "/" << Path; else - Res += Path; + Res << Path; } - - return Res; + + return Res.str(); } /*}}}*/ // URI::SiteOnly - Return the schema and site for the URI /*{{{*/ diff --git a/apt-private/acqprogress.cc b/apt-private/acqprogress.cc index 14a53eacb..e834d7d6a 100644 --- a/apt-private/acqprogress.cc +++ b/apt-private/acqprogress.cc @@ -23,6 +23,7 @@ #include #include #include +#include #include #include @@ -34,9 +35,8 @@ using namespace std; // --------------------------------------------------------------------- /* */ AcqTextStatus::AcqTextStatus(unsigned int &ScreenWidth,unsigned int const Quiet) : - pkgAcquireStatus(), ScreenWidth(ScreenWidth), ID(0), Quiet(Quiet) + pkgAcquireStatus(), ScreenWidth(ScreenWidth), LastLineLength(0), ID(0), Quiet(Quiet) { - BlankLine[0] = 0; // testcases use it to disable pulses without disabling other user messages if (Quiet == 0 && _config->FindB("quiet::NoUpdate", false) == true) this->Quiet = 1; @@ -48,7 +48,7 @@ AcqTextStatus::AcqTextStatus(unsigned int &ScreenWidth,unsigned int const Quiet) void AcqTextStatus::Start() { pkgAcquireStatus::Start(); - BlankLine[0] = 0; + LastLineLength = 0; ID = 1; } /*}}}*/ @@ -60,8 +60,7 @@ void AcqTextStatus::IMSHit(pkgAcquire::ItemDesc &Itm) if (Quiet > 1) return; - if (Quiet <= 0) - cout << '\r' << BlankLine << '\r'; + clearLastLine(); cout << _("Hit ") << Itm.Description; cout << endl; @@ -82,8 +81,7 @@ void AcqTextStatus::Fetch(pkgAcquire::ItemDesc &Itm) if (Quiet > 1) return; - if (Quiet <= 0) - cout << '\r' << BlankLine << '\r'; + clearLastLine(); cout << _("Get:") << Itm.Owner->ID << ' ' << Itm.Description; if (Itm.Owner->FileSize != 0) @@ -111,8 +109,7 @@ void AcqTextStatus::Fail(pkgAcquire::ItemDesc &Itm) if (Itm.Owner->Status == pkgAcquire::Item::StatIdle) return; - if (Quiet <= 0) - cout << '\r' << BlankLine << '\r'; + clearLastLine(); if (Itm.Owner->Status == pkgAcquire::Item::StatDone) { @@ -140,8 +137,7 @@ void AcqTextStatus::Stop() if (Quiet > 1) return; - if (Quiet <= 0) - cout << '\r' << BlankLine << '\r' << flush; + clearLastLine(); if (_config->FindB("quiet::NoStatistic", false) == true) return; @@ -167,77 +163,66 @@ bool AcqTextStatus::Pulse(pkgAcquire *Owner) enum {Long = 0,Medium,Short} Mode = Medium; - char Buffer[sizeof(BlankLine)]; - char *End = Buffer + sizeof(Buffer); - char *S = Buffer; - if (ScreenWidth >= sizeof(Buffer)) - ScreenWidth = sizeof(Buffer)-1; - - // Put in the percent done - sprintf(S,"%.0f%%", Percent); - - bool Shown = false; - for (pkgAcquire::Worker *I = Owner->WorkersBegin(); I != 0; - I = Owner->WorkerStep(I)) + std::string Line; { - S += strlen(S); - - // There is no item running - if (I->CurrentItem == 0) + std::stringstream S; + for (pkgAcquire::Worker *I = Owner->WorkersBegin(); I != 0; + I = Owner->WorkerStep(I)) { - if (I->Status.empty() == false) + // There is no item running + if (I->CurrentItem == 0) { - snprintf(S,End-S," [%s]",I->Status.c_str()); - Shown = true; - } - - continue; - } - - Shown = true; + if (I->Status.empty() == false) + S << " [" << I->Status << "]"; - // Add in the short description - if (I->CurrentItem->Owner->ID != 0) - snprintf(S,End-S," [%lu %s",I->CurrentItem->Owner->ID, - I->CurrentItem->ShortDesc.c_str()); - else - snprintf(S,End-S," [%s",I->CurrentItem->ShortDesc.c_str()); - S += strlen(S); + continue; + } - // Show the short mode string - if (I->CurrentItem->Owner->ActiveSubprocess.empty() == false) - { - snprintf(S,End-S, " %s", I->CurrentItem->Owner->ActiveSubprocess.c_str()); - S += strlen(S); - } + // Add in the short description + S << " ["; + if (I->CurrentItem->Owner->ID != 0) + S << I->CurrentItem->Owner->ID << " "; + S << I->CurrentItem->ShortDesc; - // Add the current progress - if (Mode == Long) - snprintf(S,End-S," %llu",I->CurrentSize); - else - { - if (Mode == Medium || I->TotalSize == 0) - snprintf(S,End-S," %sB",SizeToStr(I->CurrentSize).c_str()); - } - S += strlen(S); + // Show the short mode string + if (I->CurrentItem->Owner->ActiveSubprocess.empty() == false) + S << " " << I->CurrentItem->Owner->ActiveSubprocess; - // Add the total size and percent - if (I->TotalSize > 0 && I->CurrentItem->Owner->Complete == false) - { - if (Mode == Short) - snprintf(S,End-S," %.0f%%", - (I->CurrentSize*100.0)/I->TotalSize); + // Add the current progress + if (Mode == Long) + S << " " << I->CurrentSize; else - snprintf(S,End-S,"/%sB %.0f%%",SizeToStr(I->TotalSize).c_str(), + { + if (Mode == Medium || I->TotalSize == 0) + S << " " << SizeToStr(I->CurrentSize) << "B"; + } + + // Add the total size and percent + if (I->TotalSize > 0 && I->CurrentItem->Owner->Complete == false) + { + if (Mode == Short) + ioprintf(S, " %.0f%%", (I->CurrentSize*100.0)/I->TotalSize); + else + ioprintf(S, "/%sB %.0f%%", SizeToStr(I->TotalSize).c_str(), (I->CurrentSize*100.0)/I->TotalSize); + } + S << "]"; } - S += strlen(S); - snprintf(S,End-S,"]"); - } - // Show something.. - if (Shown == false) - snprintf(S,End-S,_(" [Working]")); + // Show at least something + Line = S.str(); + S.clear(); + if (Line.empty() == true) + Line = _(" [Working]"); + } + // Put in the percent done + { + std::stringstream S; + ioprintf(S, "%.0f%%", Percent); + S << Line; + Line = S.str(); + S.clear(); + } /* Put in the ETA and cps meter, block off signals to prevent strangeness during resizing */ @@ -248,34 +233,33 @@ bool AcqTextStatus::Pulse(pkgAcquire *Owner) if (CurrentCPS != 0) { - char Tmp[300]; unsigned long long ETA = (TotalBytes - CurrentBytes)/CurrentCPS; - sprintf(Tmp," %sB/s %s",SizeToStr(CurrentCPS).c_str(),TimeToStr(ETA).c_str()); - unsigned int Len = strlen(Buffer); - unsigned int LenT = strlen(Tmp); - if (Len + LenT < ScreenWidth) + std::string Tmp = " " + SizeToStr(CurrentCPS) + "B/s " + TimeToStr(ETA); + size_t alignment = Line.length() + Tmp.length(); + if (alignment < ScreenWidth) { - memset(Buffer + Len,' ',ScreenWidth - Len); - strcpy(Buffer + ScreenWidth - LenT,Tmp); + alignment = ScreenWidth - alignment; + for (size_t i = 0; i < alignment; ++i) + Line.append(" "); + Line.append(Tmp); } } - Buffer[ScreenWidth] = 0; - BlankLine[ScreenWidth] = 0; + if (Line.length() > ScreenWidth) + Line.erase(ScreenWidth); sigprocmask(SIG_SETMASK,&OldSigs,0); // Draw the current status if (_config->FindB("Apt::Color", false) == true) cout << _config->Find("APT::Color::Yellow"); - if (strlen(Buffer) == strlen(BlankLine)) - cout << '\r' << Buffer << flush; + if (LastLineLength > Line.length()) + clearLastLine(); else - cout << '\r' << BlankLine << '\r' << Buffer << flush; + cout << '\r'; + cout << Line << flush; if (_config->FindB("Apt::Color", false) == true) cout << _config->Find("APT::Color::Neutral") << flush; - memset(BlankLine,' ',strlen(Buffer)); - BlankLine[strlen(Buffer)] = 0; - + LastLineLength = Line.length(); Update = false; return true; @@ -296,8 +280,7 @@ bool AcqTextStatus::MediaChange(string Media,string Drive) return false; - if (Quiet <= 0) - cout << '\r' << BlankLine << '\r'; + clearLastLine(); ioprintf(cout,_("Media change: please insert the disc labeled\n" " '%s'\n" "in the drive '%s' and press enter\n"), @@ -317,3 +300,17 @@ bool AcqTextStatus::MediaChange(string Media,string Drive) return bStatus; } /*}}}*/ +void AcqTextStatus::clearLastLine() { /*{{{*/ + if (Quiet > 0) + return; + + // do not try to clear more than the (now smaller) screen + if (LastLineLength > ScreenWidth) + LastLineLength = ScreenWidth; + + std::cout << '\r'; + for (size_t i = 0; i < LastLineLength; ++i) + std::cout << ' '; + std::cout << '\r' << std::flush; +} + /*}}}*/ diff --git a/apt-private/acqprogress.h b/apt-private/acqprogress.h index 71a10d78a..ab8170126 100644 --- a/apt-private/acqprogress.h +++ b/apt-private/acqprogress.h @@ -17,10 +17,12 @@ class APT_PUBLIC AcqTextStatus : public pkgAcquireStatus { unsigned int &ScreenWidth; - char BlankLine[1024]; + size_t LastLineLength; unsigned long ID; unsigned long Quiet; + void clearLastLine(); + public: virtual bool MediaChange(std::string Media,std::string Drive); diff --git a/ftparchive/writer.cc b/ftparchive/writer.cc index a63d8846b..db68c21f0 100644 --- a/ftparchive/writer.cc +++ b/ftparchive/writer.cc @@ -440,9 +440,6 @@ bool PackagesWriter::DoPackage(string FileName) OverItem->Priority = Tags.FindS("Priority"); } - char Size[40]; - sprintf(Size,"%llu", (unsigned long long) FileSize); - // Strip the DirStrip prefix from the FileName and add the PathPrefix string NewFileName; if (DirStrip.empty() == false && @@ -466,7 +463,10 @@ bool PackagesWriter::DoPackage(string FileName) // This lists all the changes to the fields we are going to make. std::vector Changes; - Changes.push_back(SetTFRewriteData("Size", Size)); + std::string Size; + strprintf(Size, "%llu", (unsigned long long) FileSize); + Changes.push_back(SetTFRewriteData("Size", Size.c_str())); + for (HashStringList::const_iterator hs = Db.HashesList.begin(); hs != Db.HashesList.end(); ++hs) { if (hs->HashType() == "MD5Sum") diff --git a/methods/ftp.cc b/methods/ftp.cc index 0504e5872..7764acf6a 100644 --- a/methods/ftp.cc +++ b/methods/ftp.cc @@ -259,19 +259,21 @@ bool FTPConn::Login() { if (Opts->Value.empty() == true) continue; - + // Substitute the variables into the command - char SitePort[20]; - if (ServerName.Port != 0) - sprintf(SitePort,"%u",ServerName.Port); - else - strcpy(SitePort,"21"); string Tmp = Opts->Value; Tmp = SubstVar(Tmp,"$(PROXY_USER)",Proxy.User); Tmp = SubstVar(Tmp,"$(PROXY_PASS)",Proxy.Password); Tmp = SubstVar(Tmp,"$(SITE_USER)",User); Tmp = SubstVar(Tmp,"$(SITE_PASS)",Pass); - Tmp = SubstVar(Tmp,"$(SITE_PORT)",SitePort); + if (ServerName.Port != 0) + { + std::string SitePort; + strprintf(SitePort, "%u", ServerName.Port); + Tmp = SubstVar(Tmp,"$(SITE_PORT)", SitePort); + } + else + Tmp = SubstVar(Tmp,"$(SITE_PORT)", "21"); Tmp = SubstVar(Tmp,"$(SITE)",ServerName.Host); // Send the command diff --git a/test/libapt/strutil_test.cc b/test/libapt/strutil_test.cc index 9bc3c76fd..23dc08727 100644 --- a/test/libapt/strutil_test.cc +++ b/test/libapt/strutil_test.cc @@ -97,6 +97,28 @@ TEST(StrUtilTest,StartsWith) EXPECT_FALSE(Startswith("abcd", "x")); EXPECT_FALSE(Startswith("abcd", "abcndefg")); } +TEST(StrUtilTest,TimeToStr) +{ + EXPECT_EQ("0s", TimeToStr(0)); + EXPECT_EQ("42s", TimeToStr(42)); + EXPECT_EQ("9min 21s", TimeToStr((9*60) + 21)); + EXPECT_EQ("20min 42s", TimeToStr((20*60) + 42)); + EXPECT_EQ("10h 42min 21s", TimeToStr((10*3600) + (42*60) + 21)); + EXPECT_EQ("10h 42min 21s", TimeToStr((10*3600) + (42*60) + 21)); + EXPECT_EQ("1988d 3h 29min 7s", TimeToStr((1988*86400) + (3*3600) + (29*60) + 7)); + + EXPECT_EQ("59s", TimeToStr(59)); + EXPECT_EQ("60s", TimeToStr(60)); + EXPECT_EQ("1min 1s", TimeToStr(61)); + EXPECT_EQ("59min 59s", TimeToStr(3599)); + EXPECT_EQ("60min 0s", TimeToStr(3600)); + EXPECT_EQ("1h 0min 1s", TimeToStr(3601)); + EXPECT_EQ("1h 1min 0s", TimeToStr(3660)); + EXPECT_EQ("23h 59min 59s", TimeToStr(86399)); + EXPECT_EQ("24h 0min 0s", TimeToStr(86400)); + EXPECT_EQ("1d 0h 0min 1s", TimeToStr(86401)); + EXPECT_EQ("1d 0h 1min 0s", TimeToStr(86460)); +} TEST(StrUtilTest,SubstVar) { EXPECT_EQ("", SubstVar("", "fails", "passes")); diff --git a/test/libapt/uri_test.cc b/test/libapt/uri_test.cc index 1662f51f0..5d5ae9679 100644 --- a/test/libapt/uri_test.cc +++ b/test/libapt/uri_test.cc @@ -12,6 +12,7 @@ TEST(URITest, BasicHTTP) EXPECT_EQ(90, U.Port); EXPECT_EQ("www.debian.org", U.Host); EXPECT_EQ("/temp/test", U.Path); + EXPECT_EQ("http://www.debian.org:90/temp/test", (std::string)U); // Login data U = URI("http://jgg:foo@ualberta.ca/blah"); EXPECT_EQ("http", U.Access); @@ -20,6 +21,7 @@ TEST(URITest, BasicHTTP) EXPECT_EQ(0, U.Port); EXPECT_EQ("ualberta.ca", U.Host); EXPECT_EQ("/blah", U.Path); + EXPECT_EQ("http://jgg:foo@ualberta.ca/blah", (std::string)U); } TEST(URITest, SingeSlashFile) { @@ -30,6 +32,7 @@ TEST(URITest, SingeSlashFile) EXPECT_EQ(0, U.Port); EXPECT_EQ("", U.Host); EXPECT_EQ("/usr/bin/foo", U.Path); + EXPECT_EQ("file:/usr/bin/foo", (std::string)U); } TEST(URITest, BasicCDROM) { @@ -40,6 +43,7 @@ TEST(URITest, BasicCDROM) EXPECT_EQ(0, U.Port); EXPECT_EQ("Moo Cow Rom", U.Host); EXPECT_EQ("/debian", U.Path); + EXPECT_EQ("cdrom://Moo Cow Rom/debian", (std::string)U); } TEST(URITest, RelativeGzip) { @@ -50,6 +54,7 @@ TEST(URITest, RelativeGzip) EXPECT_EQ(0, U.Port); EXPECT_EQ(".", U.Host); EXPECT_EQ("/bar/cow", U.Path); + EXPECT_EQ("gzip://./bar/cow", (std::string)U); } TEST(URITest, NoSlashFTP) { @@ -60,6 +65,7 @@ TEST(URITest, NoSlashFTP) EXPECT_EQ(0, U.Port); EXPECT_EQ("ftp.fr.debian.org", U.Host); EXPECT_EQ("/debian/pool/main/x/xtel/xtel_3.2.1-15_i386.deb", U.Path); + EXPECT_EQ("ftp://ftp.fr.debian.org/debian/pool/main/x/xtel/xtel_3.2.1-15_i386.deb", (std::string)U); } TEST(URITest, RFC2732) { @@ -70,6 +76,7 @@ TEST(URITest, RFC2732) EXPECT_EQ(0, U.Port); EXPECT_EQ("1080::8:800:200C:417A", U.Host); EXPECT_EQ("/foo", U.Path); + EXPECT_EQ("http://[1080::8:800:200C:417A]/foo", (std::string)U); // with port U = URI("http://[::FFFF:129.144.52.38]:80/index.html"); EXPECT_EQ("http", U.Access); @@ -78,6 +85,7 @@ TEST(URITest, RFC2732) EXPECT_EQ(80, U.Port); EXPECT_EQ("::FFFF:129.144.52.38", U.Host); EXPECT_EQ("/index.html", U.Path); + EXPECT_EQ("http://[::FFFF:129.144.52.38]:80/index.html", (std::string)U); // extra colon U = URI("http://[::FFFF:129.144.52.38:]:80/index.html"); EXPECT_EQ("http", U.Access); @@ -86,6 +94,7 @@ TEST(URITest, RFC2732) EXPECT_EQ(80, U.Port); EXPECT_EQ("::FFFF:129.144.52.38:", U.Host); EXPECT_EQ("/index.html", U.Path); + EXPECT_EQ("http://[::FFFF:129.144.52.38:]:80/index.html", (std::string)U); // extra colon port U = URI("http://[::FFFF:129.144.52.38:]/index.html"); EXPECT_EQ("http", U.Access); @@ -94,6 +103,7 @@ TEST(URITest, RFC2732) EXPECT_EQ(0, U.Port); EXPECT_EQ("::FFFF:129.144.52.38:", U.Host); EXPECT_EQ("/index.html", U.Path); + EXPECT_EQ("http://[::FFFF:129.144.52.38:]/index.html", (std::string)U); // My Evil Corruption of RFC 2732 to handle CDROM names! // Fun for the whole family! */ U = URI("cdrom:[The Debian 1.2 disk, 1/2 R1:6]/debian/"); @@ -103,6 +113,7 @@ TEST(URITest, RFC2732) EXPECT_EQ(0, U.Port); EXPECT_EQ("The Debian 1.2 disk, 1/2 R1:6", U.Host); EXPECT_EQ("/debian/", U.Path); + EXPECT_EQ("cdrom://[The Debian 1.2 disk, 1/2 R1:6]/debian/", (std::string)U); // no brackets U = URI("cdrom:Foo Bar Cow/debian/"); EXPECT_EQ("cdrom", U.Access); @@ -111,6 +122,7 @@ TEST(URITest, RFC2732) EXPECT_EQ(0, U.Port); EXPECT_EQ("Foo Bar Cow", U.Host); EXPECT_EQ("/debian/", U.Path); + EXPECT_EQ("cdrom://Foo Bar Cow/debian/", (std::string)U); // percent encoded U = URI("ftp://foo:b%40r@example.org"); EXPECT_EQ("foo", U.User); -- cgit v1.2.3-70-g09d2