From 28e268bd26f558aff967887b53941ecb3a270a73 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Mon, 17 Jan 2022 23:36:32 +0000 Subject: [PATCH] fix parsing invalid depends from server The data from the server is read via Task::parseJSON, not Task::parse. This also reverts the tests for Task::parse, and adds new tests for this specific issue. --- src/Task.cpp | 35 ++++++++++-------- test/.gitignore | 1 + test/CMakeLists.txt | 2 +- test/t.t.cpp | 42 --------------------- test/tw-2689.t.cpp | 89 +++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 110 insertions(+), 59 deletions(-) create mode 100644 test/tw-2689.t.cpp diff --git a/src/Task.cpp b/src/Task.cpp index 31c3125bb..063ae1193 100644 --- a/src/Task.cpp +++ b/src/Task.cpp @@ -657,21 +657,6 @@ void Task::parse (const std::string& input) ++annotation_count; data[name] = decode (json::decode (value)); - - // Fix for issue#2689: taskserver sometimes encodes the depends - // property as a JSON-encoded array of strings. To avoid this whole - // issue, we strip anything that isn't a UUID character or a comma. - if (name == "depends") { - std::string newDep; - for (auto &c: data[name]) { - if ((c >= '0' && c <= '9') || - (c >= 'a' && c <= 'f') || - c == ',' || c == '-') { - newDep.push_back(c); - } - } - data[name] = newDep; - } } attLine.skip (' '); @@ -792,7 +777,25 @@ void Task::parseJSON (const json::object* root_obj) else if (i.first == "depends" && i.second->type() == json::j_string) { auto deps = (json::string*)i.second; - auto uuids = split (deps->_data, ','); + + // Fix for issue#2689: taskserver sometimes encodes the depends + // property as a string of the format `[\"uuid\",\"uuid\"]` + // The string includes the backslash-escaped `"` characters, making + // it invalid JSON. Since we know the characters we're looking for, + // we'll just filter out everything else. + std::string deps_str = deps->_data; + if (deps_str.front () == '[' && deps_str.back () == ']') { + std::string filtered; + for (auto &c: deps_str) { + if ((c >= '0' && c <= '9') || + (c >= 'a' && c <= 'f') || + c == ',' || c == '-') { + filtered.push_back(c); + } + } + deps_str = filtered; + } + auto uuids = split (deps_str, ','); for (const auto& uuid : uuids) addDependency (uuid); diff --git a/test/.gitignore b/test/.gitignore index 7593246a7..b3803c61f 100644 --- a/test/.gitignore +++ b/test/.gitignore @@ -34,5 +34,6 @@ variant_partial.t variant_subtract.t variant_xor.t view.t +tw-2689.t json_test diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 3e37e5e3b..21b7dc444 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -16,7 +16,7 @@ include_directories (${CMAKE_SOURCE_DIR} ${CMAKE_SOURCE_DIR}/test ${TASK_INCLUDE_DIRS}) -set (test_SRCS col.t dom.t eval.t lexer.t t.t tdb2.t util.t variant_add.t variant_and.t variant_cast.t variant_divide.t variant_equal.t variant_exp.t variant_gt.t variant_gte.t variant_inequal.t variant_lt.t variant_lte.t variant_match.t variant_math.t variant_modulo.t variant_multiply.t variant_nomatch.t variant_not.t variant_or.t variant_partial.t variant_subtract.t variant_xor.t view.t) +set (test_SRCS col.t dom.t eval.t lexer.t t.t tw-2689.t tdb2.t util.t variant_add.t variant_and.t variant_cast.t variant_divide.t variant_equal.t variant_exp.t variant_gt.t variant_gte.t variant_inequal.t variant_lt.t variant_lte.t variant_match.t variant_math.t variant_modulo.t variant_multiply.t variant_nomatch.t variant_not.t variant_or.t variant_partial.t variant_subtract.t variant_xor.t view.t) add_custom_target (test ./run_all --verbose DEPENDS ${test_SRCS} task_executable diff --git a/test/t.t.cpp b/test/t.t.cpp index d053f0643..69ac90a11 100644 --- a/test/t.t.cpp +++ b/test/t.t.cpp @@ -119,48 +119,6 @@ int main (int, char**) value = ff4.get ("description"); test.is (value, "Description", "ff4 description"); - // depends as a comma-separated set of strings - sample = "[" - "uuid:\"00000000-0000-0000-0000-000000000000\" " - "description:\"d\" " - "depends:\"cfee3170-f153-4075-aa1d-e20bcac2841b,f5982cca-2ea1-4bfd-832c-9bd571dc0743\"" - "]"; - ff4 = Task (sample); - value = ff4.get ("uuid"); - test.is (value, "00000000-0000-0000-0000-000000000000", "ff4 uuid"); - value = ff4.get ("depends"); - test.is (value, "cfee3170-f153-4075-aa1d-e20bcac2841b,f5982cca-2ea1-4bfd-832c-9bd571dc0743", "ff4 depends"); - test.ok (ff4.has ("dep_cfee3170-f153-4075-aa1d-e20bcac2841b"), "ff4 dep attr"); - test.ok (ff4.has ("dep_f5982cca-2ea1-4bfd-832c-9bd571dc0743"), "ff4 dep attr"); - - // depends as JSON string (issue#2689) - sample = "[" - "uuid:\"00000000-0000-0000-0000-000000000000\" " - "description:\"d\" " - "depends:\"[\\\"cfee3170-f153-4075-aa1d-e20bcac2841b\\\",\\\"f5982cca-2ea1-4bfd-832c-9bd571dc0743\\\"]\"" - "]"; - ff4 = Task (sample); - value = ff4.get ("uuid"); - test.is (value, "00000000-0000-0000-0000-000000000000", "ff4 uuid"); - value = ff4.get ("depends"); - test.is (value, "cfee3170-f153-4075-aa1d-e20bcac2841b,f5982cca-2ea1-4bfd-832c-9bd571dc0743", "ff4 depends"); - test.ok (ff4.has ("dep_cfee3170-f153-4075-aa1d-e20bcac2841b"), "ff4 dep attr"); - test.ok (ff4.has ("dep_f5982cca-2ea1-4bfd-832c-9bd571dc0743"), "ff4 dep attr"); - - // depends as HTML-entity-encoded JSON string (issue#2689) - sample = "[" - "uuid:\"00000000-0000-0000-0000-000000000000\" " - "description:\"d\" " - "depends:\"&open;\\\"cfee3170-f153-4075-aa1d-e20bcac2841b\\\",\\\"f5982cca-2ea1-4bfd-832c-9bd571dc0743\\\"&close;\"" - "]"; - ff4 = Task (sample); - value = ff4.get ("uuid"); - test.is (value, "00000000-0000-0000-0000-000000000000", "ff4 uuid"); - value = ff4.get ("depends"); - test.is (value, "cfee3170-f153-4075-aa1d-e20bcac2841b,f5982cca-2ea1-4bfd-832c-9bd571dc0743", "ff4 depends"); - test.ok (ff4.has ("dep_cfee3170-f153-4075-aa1d-e20bcac2841b"), "ff4 dep attr"); - test.ok (ff4.has ("dep_f5982cca-2ea1-4bfd-832c-9bd571dc0743"), "ff4 dep attr"); - /* TODO Task::composeCSV diff --git a/test/tw-2689.t.cpp b/test/tw-2689.t.cpp new file mode 100644 index 000000000..3b67f1f4e --- /dev/null +++ b/test/tw-2689.t.cpp @@ -0,0 +1,89 @@ +//////////////////////////////////////////////////////////////////////////////// +// +// Copyright 2006 - 2021, Tomas Babej, 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. +// +// https://www.opensource.org/licenses/mit-license.php +// +//////////////////////////////////////////////////////////////////////////////// + +#include +#include +#include +#include +#include + +//////////////////////////////////////////////////////////////////////////////// +int main (int, char**) +{ + UnitTest test (12); + + // Ensure environment has no influence. + unsetenv ("TASKDATA"); + unsetenv ("TASKRC"); + + // Inform Task about the attributes in the JSON below + Task::attributes["depends"] = "string"; + Task::attributes["uuid"] = "string"; + + // depends in [..] string from a taskserver (issue#2689) + auto sample = "{" + "\"depends\":\"[\\\"92a40a34-37f3-4785-8ca1-ff89cfbfd105\\\",\\\"e08e35fa-e42b-4de0-acc4-518fca8f6365\\\"]\"," + "\"uuid\":\"00000000-0000-0000-0000-000000000000\"" + "}"; + auto json = Task (sample); + auto value = json.get ("uuid"); + test.is (value, "00000000-0000-0000-0000-000000000000", "json [..] uuid"); + value = json.get ("depends"); + test.is (value, "92a40a34-37f3-4785-8ca1-ff89cfbfd105,e08e35fa-e42b-4de0-acc4-518fca8f6365", "json [..] depends"); + test.ok (json.has ("dep_92a40a34-37f3-4785-8ca1-ff89cfbfd105"), "json [..] dep attr"); + test.ok (json.has ("dep_e08e35fa-e42b-4de0-acc4-518fca8f6365"), "json [..] dep attr"); + + // depends in comma-delimited string from a taskserver (deprecated format) + sample = "{" + "\"depends\":\"92a40a34-37f3-4785-8ca1-ff89cfbfd105,e08e35fa-e42b-4de0-acc4-518fca8f6365\"," + "\"uuid\":\"00000000-0000-0000-0000-000000000000\"" + "}"; + json = Task (sample); + value = json.get ("uuid"); + test.is (value, "00000000-0000-0000-0000-000000000000", "json comma-separated uuid"); + value = json.get ("depends"); + test.is (value, "92a40a34-37f3-4785-8ca1-ff89cfbfd105,e08e35fa-e42b-4de0-acc4-518fca8f6365", "json comma-separated depends"); + test.ok (json.has ("dep_92a40a34-37f3-4785-8ca1-ff89cfbfd105"), "json comma-separated dep attr"); + test.ok (json.has ("dep_e08e35fa-e42b-4de0-acc4-518fca8f6365"), "json comma-separated dep attr"); + + // depends in a JSON array from a taskserver + sample = "{" + "\"depends\":[\"92a40a34-37f3-4785-8ca1-ff89cfbfd105\",\"e08e35fa-e42b-4de0-acc4-518fca8f6365\"]," + "\"uuid\":\"00000000-0000-0000-0000-000000000000\"" + "}"; + json = Task (sample); + value = json.get ("uuid"); + test.is (value, "00000000-0000-0000-0000-000000000000", "json array uuid"); + value = json.get ("depends"); + test.is (value, "92a40a34-37f3-4785-8ca1-ff89cfbfd105,e08e35fa-e42b-4de0-acc4-518fca8f6365", "json array depends"); + test.ok (json.has ("dep_92a40a34-37f3-4785-8ca1-ff89cfbfd105"), "json array dep attr"); + test.ok (json.has ("dep_e08e35fa-e42b-4de0-acc4-518fca8f6365"), "json array dep attr"); + + return 0; +} + +//////////////////////////////////////////////////////////////////////////////// +