Fixed Parser::Tokenizer::prefix() to return false on all empty prefixes.

The code was returning true and sometimes even setting the returnedToken
to a non-empty string when no permitted tokenChars were found.

Added unit test cases to catch similar bugs in the future.

diff --git a/src/parser/Tokenizer.cc b/src/parser/Tokenizer.cc
index a7dbdba6b6746aba2c89c010dad1ccd4e0806723..c9c8959dbec24d596853b693ed8e27f099ff4cd4 100644 (file)
--- a/src/parser/Tokenizer.cc
+++ b/src/parser/Tokenizer.cc
@@ -66,7 +66,9 @@ Parser::Tokenizer::prefix(SBuf &returnedToken, const CharacterSet &tokenChars, c
     const SBuf::size_type prefixLen = buf_.substr(0,limit).findFirstNotOf(tokenChars);
     if (prefixLen == 0)
         return false;
-    returnedToken = consume(prefixLen);
+    if (prefixLen == SBuf::npos && (atEnd() || limit == 0))
+        return false;
+    returnedToken = consume(prefixLen); // cannot be empty after the npos check
     return true;
 }
 

diff --git a/src/parser/testTokenizer.cc b/src/parser/testTokenizer.cc
index 7443e07eeb46dd3b5e1204f4f262a2ac57d89d8c..b730a9429cb487315b016c84040fd476bcaeb6af 100644 (file)
--- a/src/parser/testTokenizer.cc
+++ b/src/parser/testTokenizer.cc
@@ -18,9 +18,26 @@ const CharacterSet numbers("numbers","0123456789");
 void
 testTokenizer::testTokenizerPrefix()
 {
+    const SBuf canary("This text should not be changed.");
+
     Parser::Tokenizer t(text);
     SBuf s;
 
+    CharacterSet all(whitespace);
+    all += alpha;
+    all += crlf;
+    all += numbers;
+    all.add(':').add('.').add('/');
+
+    // an empty prefix should return false (the full output buffer case)
+    s = canary;
+    const SBuf before = t.remaining();
+    CPPUNIT_ASSERT(!t.prefix(s, all, 0));
+    // ... and a false return value means no parameter changes
+    CPPUNIT_ASSERT_EQUAL(canary, s);
+    // ... and a false return value means no input buffer changes
+    CPPUNIT_ASSERT_EQUAL(before, t.remaining());
+
     // successful prefix tokenization
     CPPUNIT_ASSERT(t.prefix(s,alpha));
     CPPUNIT_ASSERT_EQUAL(SBuf("GET"),s);
@@ -40,13 +57,14 @@ testTokenizer::testTokenizerPrefix()
     CPPUNIT_ASSERT_EQUAL(SBuf("http"),s); //output SBuf left untouched
 
     // match until the end of the sample
-    CharacterSet all(whitespace);
-    all += alpha;
-    all += crlf;
-    all += numbers;
-    all.add(':').add('.').add('/');
     CPPUNIT_ASSERT(t.prefix(s,all));
     CPPUNIT_ASSERT_EQUAL(SBuf(),t.remaining());
+
+    // empty prefix should return false (the empty input buffer case)
+    s = canary;
+    CPPUNIT_ASSERT(!t.prefix(s, all));
+    // ... and a false return value means no parameter changes
+    CPPUNIT_ASSERT_EQUAL(canary, s);
 }
 
 void