From ec96d929a071636fb5059aae5106b066e200e7ec Mon Sep 17 00:00:00 2001 From: Paul Beckingham Date: Sun, 29 Jan 2012 15:36:05 -0500 Subject: [PATCH] Bug 879 - Description/Annotation ending with Slash Causes Problems - Backslashes actually. The escaping mechanism in the low-level parser was eating leading \ characters when it should not. Very hard bug to find, trivial to fix. - Added unit tests to several components while narrowing this down. --- ChangeLog | 2 ++ src/File.cpp | 1 - src/JSON.cpp | 5 ++-- src/Nibbler.cpp | 8 ++++++ src/Task.cpp | 2 +- src/util.cpp | 7 ++--- test/bug.879.t | 67 ++++++++++++++++++++++++++++++++++++++++++++++ test/json.t.cpp | 41 +++++++++++++++++----------- test/nibbler.t.cpp | 40 ++++++++++++++++----------- test/util.t.cpp | 3 +++ 10 files changed, 135 insertions(+), 41 deletions(-) create mode 100755 test/bug.879.t diff --git a/ChangeLog b/ChangeLog index 3f4a57b82..9b7ead32b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -228,6 +228,8 @@ + Fixed bug #860, which prevented lower-case priority values from being accepted (thanks to Michelle Crane). + Fixed bug #862, which suppressed feedback from the 'denotate' command. + + Fixed bug #879, which mis-parsed escaped characters in the data file (thanks + to Michelle Crane). + Fixed bug #892, which caused a segfault due to misuse of std::map::operator[] (thanks to Dmitriy Samborskiy). + Fixed bug #897, which adds the UUID field to the 'completed' report diff --git a/src/File.cpp b/src/File.cpp index 6599bd7f9..6613b0500 100644 --- a/src/File.cpp +++ b/src/File.cpp @@ -25,7 +25,6 @@ // //////////////////////////////////////////////////////////////////////////////// - #define L10N // Localization complete. #include diff --git a/src/JSON.cpp b/src/JSON.cpp index ffc1a4db8..76f4df520 100644 --- a/src/JSON.cpp +++ b/src/JSON.cpp @@ -25,7 +25,6 @@ // //////////////////////////////////////////////////////////////////////////////// - #define L10N // Localization complete. #include @@ -410,10 +409,10 @@ std::string json::encode (const std::string& input) } //////////////////////////////////////////////////////////////////////////////// +// TODO Pointers might speed this up. std::string json::decode (const std::string& input) { std::string output; - for (unsigned int i = 0; i < input.length (); ++i) { if (input[i] == '\\') @@ -437,7 +436,7 @@ std::string json::decode (const std::string& input) i += 3; break; - // If it is an unrecognized seqeence, do nothing. + // If it is an unrecognized sequence, do nothing. default: output += '\\'; output += input[i]; diff --git a/src/Nibbler.cpp b/src/Nibbler.cpp index a51abfe02..e23b7f64d 100644 --- a/src/Nibbler.cpp +++ b/src/Nibbler.cpp @@ -234,6 +234,7 @@ bool Nibbler::getQuoted ( { bool inquote = false; bool inescape = false; + char previous = 0; char current = 0; result = ""; @@ -250,6 +251,7 @@ bool Nibbler::getQuoted ( if (current == '\\' && !inescape) { inescape = true; + previous = current; continue; } @@ -270,6 +272,12 @@ bool Nibbler::getQuoted ( } else { + if (previous) + { + result += previous; + previous = 0; + } + result += current; inescape = false; } diff --git a/src/Task.cpp b/src/Task.cpp index a99e20ad0..7c0671f9c 100644 --- a/src/Task.cpp +++ b/src/Task.cpp @@ -309,7 +309,7 @@ void Task::parse (const std::string& input) nl.skip (':') && nl.getQuoted ('"', value)) { - // Experimental legacy value translation. + // Experimental legacy value translation of 'recur:m' --> 'recur:mo'. if (name == "recur" && digitsOnly (value.substr (0, value.length () - 1)) && value[value.length () - 1] == 'm') diff --git a/src/util.cpp b/src/util.cpp index 5697f9315..24ed18aa2 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -492,11 +492,9 @@ unsigned burndown_size (unsigned ntasks) //////////////////////////////////////////////////////////////////////////////// // Encode values prior to serialization. -// \t -> &tab; // " -> &dquot; // [ -> &open; // ] -> &close; -// \ -> \\ (extra chars to disambiguate multi-line comment) const std::string encode (const std::string& value) { std::string modified = value; @@ -510,9 +508,8 @@ const std::string encode (const std::string& value) //////////////////////////////////////////////////////////////////////////////// // Decode values after parse. -// \t <- &tab; -// " <- " or &dquot; -// ' <- &squot; +// " <- &dquot; +// ' <- &squot; or " // , <- , // [ <- &open; // ] <- &close; diff --git a/test/bug.879.t b/test/bug.879.t new file mode 100755 index 000000000..2385389a4 --- /dev/null +++ b/test/bug.879.t @@ -0,0 +1,67 @@ +#! /usr/bin/perl +################################################################################ +## taskwarrior - a command line task list manager. +## +## Copyright 2006-2012, Paul Beckingham, Federico Hernandez. +## +## Permission is hereby granted, free of charge, to any person obtaining a copy +## of this software and associated documentation files (the "Software"), to deal +## in the Software without restriction, including without limitation the rights +## to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +## copies of the Software, and to permit persons to whom the Software is +## furnished to do so, subject to the following conditions: +## +## The above copyright notice and this permission notice shall be included +## in all copies or substantial portions of the Software. +## +## THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +## OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +## FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +## THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +## LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +## OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +## SOFTWARE. +## +## http://www.opensource.org/licenses/mit-license.php +## +################################################################################ + +use strict; +use warnings; +use Test::More tests => 8; + +# Create the rc file. +if (open my $fh, '>', 'bug.rc') +{ + print $fh "data.location=.\n"; + close $fh; + ok (-r 'bug.rc', 'Created bug.rc'); +} + +# Bug 879: Backslash at end of description/annotation causes problems. +qx{../src/task rc:bug.rc add one\\\\}; +my $output = qx{../src/task rc:bug.rc list}; +like ($output, qr/one\\/, 'Backslash preserved in description'); + +qx{../src/task rc:bug.rc 1 annotate foo\\\\}; +$output = qx{../src/task rc:bug.rc list}; +like ($output, qr/one\\/, 'Backslash preserved in description'); +like ($output, qr/foo\\/, 'Backslash preserved in annotation 1'); + +qx{../src/task rc:bug.rc 1 annotate bar\\\\}; +$output = qx{../src/task rc:bug.rc list}; +like ($output, qr/one\\/, 'Backslash preserved in description'); +like ($output, qr/foo\\/, 'Backslash preserved in annotation 1'); +like ($output, qr/bar\\/, 'Backslash preserved in annotation 2'); + +# Cleanup. +unlink qw(pending.data completed.data undo.data backlog.data synch.key bug.rc); +ok (! -r 'pending.data' && + ! -r 'completed.data' && + ! -r 'undo.data' && + ! -r 'backlog.data' && + ! -r 'synch.key' && + ! -r 'bug.rc', 'Cleanup'); + +exit 0; + diff --git a/test/json.t.cpp b/test/json.t.cpp index d11efc799..c83230e82 100644 --- a/test/json.t.cpp +++ b/test/json.t.cpp @@ -104,7 +104,7 @@ const char *negative_tests[] = //////////////////////////////////////////////////////////////////////////////// int main (int argc, char** argv) { - UnitTest t (NUM_POSITIVE_TESTS + NUM_NEGATIVE_TESTS + 14); + UnitTest t (NUM_POSITIVE_TESTS + NUM_NEGATIVE_TESTS + 22); // Positive tests. for (int i = 0; i < NUM_POSITIVE_TESTS; ++i) @@ -141,26 +141,37 @@ int main (int argc, char** argv) try { // Regular unit tests. - t.is (json::encode ("1\b2"), "1\\b2", "json::encode \\b -> \\\\b"); - t.is (json::decode ("1\\b2"), "1\b2", "json::decode \\\\b -> \\b"); + t.is (json::encode ("1\b2"), "1\\b2", "json::encode \\b -> \\\\b"); + t.is (json::decode ("1\\b2"), "1\b2", "json::decode \\\\b -> \\b"); - t.is (json::encode ("1\n2"), "1\\n2", "json::encode \\n -> \\\\n"); - t.is (json::decode ("1\\n2"), "1\n2", "json::decode \\\\n -> \\n"); + t.is (json::encode ("1\n2"), "1\\n2", "json::encode \\n -> \\\\n"); + t.is (json::decode ("1\\n2"), "1\n2", "json::decode \\\\n -> \\n"); - t.is (json::encode ("1\r2"), "1\\r2", "json::encode \\r -> \\\\r"); - t.is (json::decode ("1\\r2"), "1\r2", "json::decode \\\\r -> \\r"); + t.is (json::encode ("1\r2"), "1\\r2", "json::encode \\r -> \\\\r"); + t.is (json::decode ("1\\r2"), "1\r2", "json::decode \\\\r -> \\r"); - t.is (json::encode ("1\t2"), "1\\t2", "json::encode \\t -> \\\\t"); - t.is (json::decode ("1\\t2"), "1\t2", "json::decode \\\\t -> \\t"); + t.is (json::encode ("1\t2"), "1\\t2", "json::encode \\t -> \\\\t"); + t.is (json::decode ("1\\t2"), "1\t2", "json::decode \\\\t -> \\t"); - t.is (json::encode ("1\\2"), "1\\\\2", "json::encode \\ -> \\\\"); - t.is (json::decode ("1\\\\2"), "1\\2", "json::decode \\\\ -> \\"); + t.is (json::encode ("1\\2"), "1\\\\2", "json::encode \\ -> \\\\"); + t.is (json::decode ("1\\\\2"), "1\\2", "json::decode \\\\ -> \\"); - t.is (json::encode ("1\x2"), "1\x2", "json::encode \\x -> \\x (NOP)"); - t.is (json::decode ("1\x2"), "1\x2", "json::decode \\x -> \\x (NOP)"); + t.is (json::encode ("1\x2"), "1\x2", "json::encode \\x -> \\x (NOP)"); + t.is (json::decode ("1\x2"), "1\x2", "json::decode \\x -> \\x (NOP)"); - t.is (json::encode ("1€2"), "1€2", "json::encode € -> €"); - t.is (json::decode ("1\\u20ac2"), "1€2", "json::decode \\u20ac -> €"); + t.is (json::encode ("1€2"), "1€2", "json::encode € -> €"); + t.is (json::decode ("1\\u20ac2"), "1€2", "json::decode \\u20ac -> €"); + + std::string encoded = json::encode ("one\\"); + t.is (encoded, "one\\\\", "json::encode one\\\\ -> one\\\\\\\\"); + t.is ((int)encoded.length (), 5, "json::encode one\\\\ -> length 5"); + t.is (encoded[0], 'o', "json::encode one\\\\[0] -> o"); + t.is (encoded[1], 'n', "json::encode one\\\\[1] -> n"); + t.is (encoded[2], 'e', "json::encode one\\\\[2] -> e"); + t.is (encoded[3], '\\', "json::encode one\\\\[3] -> \\"); + t.is (encoded[4], '\\', "json::encode one\\\\[4] -> \\"); + + t.is (json::decode (encoded), "one\\", "json::decode one\\\\\\\\ -> one\\\\"); } catch (std::string& e) {t.diag (e);} diff --git a/test/nibbler.t.cpp b/test/nibbler.t.cpp index 1de0070bc..25d25518e 100644 --- a/test/nibbler.t.cpp +++ b/test/nibbler.t.cpp @@ -39,15 +39,15 @@ int main (int argc, char** argv) { #ifdef NIBBLER_FEATURE_DATE #ifdef NIBBLER_FEATURE_REGEX - UnitTest t (292); + UnitTest t (296); #else - UnitTest t (268); + UnitTest t (272); #endif #else #ifdef NIBBLER_FEATURE_REGEX - UnitTest t (242); + UnitTest t (246); #else - UnitTest t (218); + UnitTest t (222); #endif #endif @@ -210,36 +210,44 @@ int main (int argc, char** argv) n = Nibbler ("'\"'"); t.ok (n.getQuoted ('\'', s), " ''\"'' : getQuoted (''') -> true"); - t.is (s, "\"", " ''\"'' : getQuoted (''') -> '\"'"); // 81 + t.is (s, "\"", " ''\"'' : getQuoted (''') -> '\"'"); n = Nibbler ("'x'"); t.ok (n.getQuoted ('\'', s), " ''x'' : getQuoted (''') -> true"); - t.is (s, "x", " ''x'' : getQuoted (''') -> ''"); // 83 + t.is (s, "x", " ''x'' : getQuoted (''') -> ''"); n = Nibbler ("'x"); t.notok (n.getQuoted ('\'', s), " ''x' : getQuoted (''') -> false"); n = Nibbler ("x"); - t.notok (n.getQuoted ('\'', s), " 'x' : getQuoted (''') -> false"); + t.notok (n.getQuoted ('\'', s), " 'x' : getQuoted (''') -> false"); // 90 n = Nibbler ("\"one\\\"two\""); - t.notok (n.getQuoted ('\'', s), "\"one\\\"two\" : getQuoted (''') -> false"); // 86 + t.notok (n.getQuoted ('\'', s), "\"one\\\"two\" : getQuoted (''') -> false"); n = Nibbler ("\"one\\\"two\""); - t.ok (n.getQuoted ('"', s, false), "\"one\\\"two\" : getQuoted ('\"', false, false) -> true"); // 87 - t.is (s, "one\"two", "getQuoted ('\"', false) -> one\"two"); // 88 + t.ok (n.getQuoted ('"', s, false), "\"one\\\"two\" : getQuoted ('\"', false, false) -> true"); + t.is (s, "one\\\"two", "getQuoted ('\"', false) -> one\\\"two"); n = Nibbler ("\"one\\\"two\""); - t.ok (n.getQuoted ('"', s, true), "\"one\\\"two\" : getQuoted ('\"', false, true) -> true"); // 89 - t.is (s, "\"one\"two\"", "getQuoted ('\"', true) -> \"one\"two\""); // 90 + t.ok (n.getQuoted ('"', s, true), "\"one\\\"two\" : getQuoted ('\"', false, true) -> true"); + t.is (s, "\"one\\\"two\"", "getQuoted ('\"', true) -> \"one\\\"two\""); n = Nibbler ("\"one\\\"two\""); - t.ok (n.getQuoted ('"', s, false), "\"one\\\"two\" : getQuoted ('\"', true, false) -> true"); // 91 - t.is (s, "one\"two", "getQuoted ('\"', false) -> one\"two"); // 92 + t.ok (n.getQuoted ('"', s, false), "\"one\\\"two\" : getQuoted ('\"', true, false) -> true"); + t.is (s, "one\\\"two", "getQuoted ('\"', false) -> one\\\"two"); n = Nibbler ("\"one\\\"two\""); - t.ok (n.getQuoted ('"', s, true), "\"one\\\"two\" : getQuoted ('\"', true, true) -> true"); // 93 - t.is (s, "\"one\"two\"", "getQuoted ('\"', true) -> \"one\"two\""); // 94 + t.ok (n.getQuoted ('"', s, true), "\"one\\\"two\" : getQuoted ('\"', s, true) -> true"); + t.is (s, "\"one\\\"two\"", "getQuoted ('\"', s, true) -> \"one\\\"two\""); + + n = Nibbler ("\"one\\\\\""); + t.ok (n.getQuoted ('\"', s, true), "\"one\\\" : getQuoted ('\"', s, true) -> true"); + t.is (s, "\"one\\\\\"", "getQuoted ('\"', s, true) -> \"one\\\\\""); + + n = Nibbler ("\"one\\\\\""); + t.ok (n.getQuoted ('\"', s, false), "one\\ : getQuoted ('\"', s, false) -> true"); + t.is (s, "one\\\\", "getQuoted ('\"', s, false) -> \"one\\\\\""); // bool getDigit (int&); t.diag ("Nibbler::getDigit"); diff --git a/test/util.t.cpp b/test/util.t.cpp index fad371633..a749700be 100644 --- a/test/util.t.cpp +++ b/test/util.t.cpp @@ -129,6 +129,9 @@ int main (int argc, char** argv) t.is (structured[3], " one.four", "indentTree 'one.four' -> ' one.four'"); t.is (structured[4], "two", "indentTree 'two' -> 'two'"); + // TODO const std::string encode (const std::string& value); + // TODO const std::string decode (const std::string& value); + return 0; }