3 files changed, 40 insertions, 10 deletions

diff --git a/apt-pkg/cachefilter-patterns.cc b/apt-pkg/cachefilter-patterns.cc
index 9fab0281d..cf3e59ac6 100644
--- a/apt-pkg/cachefilter-patterns.cc
+++ b/apt-pkg/cachefilter-patterns.cc
@@ -58,6 +58,9 @@ std::unique_ptr<PatternTreeParser::Node> PatternTreeParser::parseTop()
    auto node = parse();
    skipSpace();
 
+   if (node == nullptr)
+      throw Error{Node{0, sentence.size()}, "Expected pattern"};
+
    if (node->end != sentence.size())
       throw Error{Node{node->end, sentence.size()}, "Expected end of file"};
 
@@ -98,10 +101,19 @@ std::unique_ptr<PatternTreeParser::Node> PatternTreeParser::parsePrimary()
       return node;
    if ((node = parsePattern()) != nullptr)
       return node;
+
+   return nullptr;
+}
+
+std::unique_ptr<PatternTreeParser::Node> PatternTreeParser::parseArgument()
+{
+   std::unique_ptr<Node> node;
    if ((node = parseQuotedWord()) != nullptr)
       return node;
    if ((node = parseWord()) != nullptr)
       return node;
+   if ((node = parse()) != nullptr)
+      return node;
 
    throw Error{Node{state.offset, sentence.size()},
 	       "Expected pattern, quoted word, or word"};
@@ -125,7 +137,7 @@ std::unique_ptr<PatternTreeParser::Node> PatternTreeParser::parseShortPattern()
       state.offset += sp.shortName.size() + 1;
       if (sp.takesArgument)
       {
-	 node->arguments.push_back(parse());
+	 node->arguments.push_back(parseArgument());
 	 node->haveArgumentList = true;
       }
       node->end = state.offset;
@@ -173,7 +185,7 @@ std::unique_ptr<PatternTreeParser::Node> PatternTreeParser::parsePattern()
       return node;
    }
 
-   node->arguments.push_back(parse());
+   node->arguments.push_back(parseArgument());
    skipSpace();
    while (sentence[state.offset] == ',')
    {
@@ -182,7 +194,7 @@ std::unique_ptr<PatternTreeParser::Node> PatternTreeParser::parsePattern()
       // This was a trailing comma - allow it and break the loop
       if (sentence[state.offset] == ')')
 	 break;
-      node->arguments.push_back(parse());
+      node->arguments.push_back(parseArgument());
       skipSpace();
    }
 
diff --git a/apt-pkg/cachefilter-patterns.h b/apt-pkg/cachefilter-patterns.h
index 76318eafa..1770c7307 100644
--- a/apt-pkg/cachefilter-patterns.h
+++ b/apt-pkg/cachefilter-patterns.h
@@ -99,6 +99,7 @@ struct PatternTreeParser
    std::unique_ptr<Node> parsePrimary();
    std::unique_ptr<Node> parsePattern();
    std::unique_ptr<Node> parseShortPattern();
+   std::unique_ptr<Node> parseArgument();
    std::unique_ptr<Node> parseWord();
    std::unique_ptr<Node> parseQuotedWord();
 };
diff --git a/test/libapt/pattern_test.cc b/test/libapt/pattern_test.cc
index 39959cd31..7fc6a1f8f 100644
--- a/test/libapt/pattern_test.cc
+++ b/test/libapt/pattern_test.cc
@@ -16,19 +16,25 @@ using namespace APT::Internal;
 
 TEST(TreeParserTest, ParseWord)
 {
-   auto node = PatternTreeParser("word").parseTop();
-   auto wordNode = dynamic_cast<PatternTreeParser::WordNode *>(node.get());
+   auto node = PatternTreeParser("?word(word)").parseTop();
+   auto patternNode = dynamic_cast<PatternTreeParser::PatternNode *>(node.get());
+
+   ASSERT_EQ(patternNode->arguments.size(), 1u);
+   auto wordNode = dynamic_cast<PatternTreeParser::WordNode *>(patternNode->arguments[0].get());
 
-   EXPECT_EQ(node.get(), wordNode);
+   EXPECT_EQ(patternNode->arguments[0].get(), wordNode);
    EXPECT_EQ(wordNode->word, "word");
 }
 
 TEST(TreeParserTest, ParseQuotedWord)
 {
-   auto node = PatternTreeParser("\"a word\"").parseTop();
-   auto wordNode = dynamic_cast<PatternTreeParser::WordNode *>(node.get());
+   auto node = PatternTreeParser("?word(\"a word\")").parseTop();
+   auto patternNode = dynamic_cast<PatternTreeParser::PatternNode *>(node.get());
+
+   ASSERT_EQ(patternNode->arguments.size(), 1u);
+   auto wordNode = dynamic_cast<PatternTreeParser::WordNode *>(patternNode->arguments[0].get());
 
-   EXPECT_EQ(node.get(), wordNode);
+   EXPECT_EQ(patternNode->arguments[0].get(), wordNode);
    EXPECT_EQ(wordNode->word, "a word");
 }
 
@@ -158,5 +164,16 @@ TEST(TreeParserTest, ParseShortPattern)
    EXPECT_PATTERN_EQ_ATOMIC("~U", "?upgradable");
    EXPECT_PATTERN_EQ("~Vverstr", "?version(verstr)");
    EXPECT_PATTERN_EQ_ATOMIC("~v", "?virtual");
-   EXPECT_PATTERN_EQ("!foo", "?not(foo)");
+   EXPECT_PATTERN_EQ("!?foo", "?not(?foo)");
+
+   caught = false;
+   try
+   {
+      PatternTreeParser("!x").parseTop();
+   }
+   catch (PatternTreeParser::Error &e)
+   {
+      caught = true;
+   };
+   EXPECT_TRUE(caught) << "!X should have thrown an exception";
 }