StringToBool: only act if the entire string is consumed by strtol()

StringToBool uses strtol() internally to check if the argument is
a number. This function stops when it does not find any more numbers.
So a string like "0ad" (which is a valid packagename) is interpreted
as a "0". The code now checks that the entire string is consumed
not just a part of it. Thanks to Johannes Schauer for raising this
issue.

diff --git a/apt-pkg/contrib/strutl.cc b/apt-pkg/contrib/strutl.cc
index ce69c7a027ea6306c5a1d421e8c756fcd9885a4a..922229e902f02a0e9eb3bfc71802c060dc9ac349 100644 (file)
--- a/apt-pkg/contrib/strutl.cc
+++ b/apt-pkg/contrib/strutl.cc
@@ -704,9 +704,12 @@ string LookupTag(const string &Message,const char *Tag,const char *Default)
    then returns the result. Several varients on true/false are checked. */
 int StringToBool(const string &Text,int Default)
 {
-   char *End;
-   int Res = strtol(Text.c_str(),&End,0);   
-   if (End != Text.c_str() && Res >= 0 && Res <= 1)
+   char *ParseEnd;
+   int Res = strtol(Text.c_str(),&ParseEnd,0);
+   // ensure that the entire string was converted by strtol to avoid
+   // failures on "apt-cache show -a 0ad" where the "0" is converted
+   const char *TextEnd = Text.c_str()+Text.size();
+   if (ParseEnd == TextEnd && Res >= 0 && Res <= 1)
       return Res;
    
    // Check for positives

diff --git a/test/libapt/commandline_test.cc b/test/libapt/commandline_test.cc
index 26e80bfdec9204ed4d6f42611bd44fd61fa97426..e403a28c865100513636fbe94a4eb975689e9dcb 100644 (file)
--- a/test/libapt/commandline_test.cc
+++ b/test/libapt/commandline_test.cc
@@ -56,3 +56,32 @@ TEST(CommandLineTest,Parsing)
    EXPECT_TRUE(c.FindB("Test::Worked", false));
    EXPECT_FALSE(c.FindB("Test::Zero", false));
 }
+
+TEST(CommandLineTest, BoolParsing)
+{
+   CommandLine::Args Args[] = {
+      { 't', 0, "Test::Worked", 0 },
+      {0,0,0,0}
+   };
+   ::Configuration c;
+   CommandLine CmdL(Args, &c);
+
+   // the commandline parser used to use strtol() on the argument
+   // to check if the argument is a boolean expression - that
+   // stopped after the "0". this test ensures that we always check 
+   // that the entire string was consumed by strtol
+   {
+   char const * argv[] = { "show", "-t", "0ad" };
+   bool res = CmdL.Parse(sizeof(argv)/sizeof(char*), argv);
+   EXPECT_TRUE(res);
+   ASSERT_EQ(std::string(CmdL.FileList[0]), "0ad");
+   }
+
+   {
+   char const * argv[] = { "show", "-t", "0", "ad" };
+   bool res = CmdL.Parse(sizeof(argv)/sizeof(char*), argv);
+   EXPECT_TRUE(res);
+   ASSERT_EQ(std::string(CmdL.FileList[0]), "ad");
+   }
+
+}