Repository: mesos Updated Branches: refs/heads/master b5ba2b8dd -> eb3263af2
Disallowed some special path components in IDs. - Such IDs should lead to surprising or even dangerous agent side directory structure. Review: https://reviews.apache.org/r/56527 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/eb3263af Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/eb3263af Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/eb3263af Branch: refs/heads/master Commit: eb3263af2730dbc17db3e35286e58b44092c08da Parents: b5ba2b8 Author: Jiang Yan Xu <[email protected]> Authored: Wed Feb 8 11:22:46 2017 -0800 Committer: Yan Xu <[email protected]> Committed: Thu Feb 23 11:28:58 2017 -0800 ---------------------------------------------------------------------- src/common/validation.cpp | 15 ++++++++++----- src/tests/master_validation_tests.cpp | 24 ++++++++++++++++++++++++ src/tests/slave_validation_tests.cpp | 12 +++++++++++- 3 files changed, 45 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/eb3263af/src/common/validation.cpp ---------------------------------------------------------------------- diff --git a/src/common/validation.cpp b/src/common/validation.cpp index 028679d..544d3a0 100644 --- a/src/common/validation.cpp +++ b/src/common/validation.cpp @@ -33,7 +33,16 @@ namespace validation { Option<Error> validateID(const string& id) { - // Rules: + if (id.empty()) { + return Error("ID must not be empty"); + } + + // The ID cannot be exactly these special path components. + if (id == "." || id == "..") { + return Error("'" + id + "' is disallowed"); + } + + // Rules on invalid characters in the ID: // - Control charaters are obviously not allowed. // - Slashes are disallowed as IDs are likely mapped to directories in Mesos. auto invalidCharacter = [](char c) { @@ -42,10 +51,6 @@ Option<Error> validateID(const string& id) c == os::WINDOWS_PATH_SEPARATOR; }; - if (id.empty()) { - return Error("ID must be non-empty"); - } - if (std::any_of(id.begin(), id.end(), invalidCharacter)) { return Error("'" + id + "' contains invalid characters"); } http://git-wip-us.apache.org/repos/asf/mesos/blob/eb3263af/src/tests/master_validation_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/master_validation_tests.cpp b/src/tests/master_validation_tests.cpp index 53771f6..4525473 100644 --- a/src/tests/master_validation_tests.cpp +++ b/src/tests/master_validation_tests.cpp @@ -2187,6 +2187,7 @@ TEST_F(ExecutorValidationTest, ExecutorID) EXPECT_SOME(::executor::internal::validateExecutorID(executorInfo)); } + // This is currently allowed. { ExecutorInfo executorInfo = DEFAULT_EXECUTOR_INFO; executorInfo.mutable_executor_id()->set_value("ab c"); @@ -2200,6 +2201,29 @@ TEST_F(ExecutorValidationTest, ExecutorID) EXPECT_SOME(::executor::internal::validateExecutorID(executorInfo)); } + + // Containing a dot is allowed. + { + ExecutorInfo executorInfo = DEFAULT_EXECUTOR_INFO; + executorInfo.mutable_executor_id()->set_value("a.b"); + + EXPECT_NONE(::executor::internal::validateExecutorID(executorInfo)); + } + + // Being only a dot is not allowed. + { + ExecutorInfo executorInfo = DEFAULT_EXECUTOR_INFO; + executorInfo.mutable_executor_id()->set_value("."); + + EXPECT_SOME(::executor::internal::validateExecutorID(executorInfo)); + } + + { + ExecutorInfo executorInfo = DEFAULT_EXECUTOR_INFO; + executorInfo.mutable_executor_id()->set_value(".."); + + EXPECT_SOME(::executor::internal::validateExecutorID(executorInfo)); + } } http://git-wip-us.apache.org/repos/asf/mesos/blob/eb3263af/src/tests/slave_validation_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/slave_validation_tests.cpp b/src/tests/slave_validation_tests.cpp index 588c5b2..7845284 100644 --- a/src/tests/slave_validation_tests.cpp +++ b/src/tests/slave_validation_tests.cpp @@ -63,15 +63,25 @@ TEST(AgentValidationTest, ContainerID) EXPECT_SOME(error); // No spaces. - containerId.set_value(" "); + containerId.set_value("redis backup"); error = validation::container::validateContainerId(containerId); EXPECT_SOME(error); // No periods. + containerId.set_value("redis.backup"); + error = validation::container::validateContainerId(containerId); + EXPECT_SOME(error); + + // Cannot be '.'. containerId.set_value("."); error = validation::container::validateContainerId(containerId); EXPECT_SOME(error); + // Cannot be '..'. + containerId.set_value(".."); + error = validation::container::validateContainerId(containerId); + EXPECT_SOME(error); + // Valid. containerId.set_value("redis"); error = validation::container::validateContainerId(containerId);
