Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(src,include,test): fixed multiple cases where a bad yaml was accepted #1318

Merged
merged 2 commits into from
Sep 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions include/yaml-cpp/exceptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ const char* const UNKNOWN_TOKEN = "unknown token";
const char* const DOC_IN_SCALAR = "illegal document indicator in scalar";
const char* const EOF_IN_SCALAR = "illegal EOF in scalar";
const char* const CHAR_IN_SCALAR = "illegal character in scalar";
const char* const UNEXPECTED_SCALAR = "unexpected scalar";
const char* const UNEXPECTED_FLOW = "plain value cannot start with flow indicator character";
const char* const TAB_IN_INDENTATION =
"illegal tab when looking for indentation";
const char* const FLOW_END = "illegal flow end";
Expand Down
24 changes: 24 additions & 0 deletions src/scanner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Scanner::Scanner(std::istream& in)
m_startedStream(false),
m_endedStream(false),
m_simpleKeyAllowed(false),
m_scalarValueAllowed(false),
m_canBeJSONFlow(false),
m_simpleKeys{},
m_indents{},
Expand Down Expand Up @@ -127,6 +128,17 @@ void Scanner::ScanNextToken() {
}

if (INPUT.peek() == Keys::FlowEntry) {
// values starting with `,` are not allowed.
// eg: reject `,foo`
if (INPUT.column() == 0) {
throw ParserException(INPUT.mark(), ErrorMsg::UNEXPECTED_FLOW);
}
// if we already parsed a quoted scalar value and we are not in a flow,
// then `,` is not a valid character.
// eg: reject `"foo",`
if (!m_scalarValueAllowed) {
throw ParserException(INPUT.mark(), ErrorMsg::UNEXPECTED_SCALAR);
}
return ScanFlowEntry();
}

Expand Down Expand Up @@ -159,6 +171,13 @@ void Scanner::ScanNextToken() {
return ScanBlockScalar();
}

// if we already parsed a quoted scalar value in this line,
// another scalar value is an error.
// eg: reject `"foo" "bar"`
if (!m_scalarValueAllowed) {
throw ParserException(INPUT.mark(), ErrorMsg::UNEXPECTED_SCALAR);
}

if (INPUT.peek() == '\'' || INPUT.peek() == '\"') {
return ScanQuotedScalar();
}
Expand Down Expand Up @@ -203,6 +222,9 @@ void Scanner::ScanToNextToken() {
// oh yeah, and let's get rid of that simple key
InvalidateSimpleKey();

// new line - we accept a scalar value now
m_scalarValueAllowed = true;

// new line - we may be able to accept a simple key now
if (InBlockContext()) {
m_simpleKeyAllowed = true;
Expand Down Expand Up @@ -245,6 +267,7 @@ const RegEx& Scanner::GetValueRegex() const {
void Scanner::StartStream() {
m_startedStream = true;
m_simpleKeyAllowed = true;
m_scalarValueAllowed = true;
std::unique_ptr<IndentMarker> pIndent(
new IndentMarker(-1, IndentMarker::NONE));
m_indentRefs.push_back(std::move(pIndent));
Expand All @@ -261,6 +284,7 @@ void Scanner::EndStream() {
PopAllSimpleKeys();

m_simpleKeyAllowed = false;
m_scalarValueAllowed = false;
m_endedStream = true;
}

Expand Down
1 change: 1 addition & 0 deletions src/scanner.h
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ class Scanner {
// state info
bool m_startedStream, m_endedStream;
bool m_simpleKeyAllowed;
bool m_scalarValueAllowed;
bool m_canBeJSONFlow;
std::stack<SimpleKey> m_simpleKeys;
std::stack<IndentMarker *> m_indents;
Expand Down
7 changes: 7 additions & 0 deletions src/scantoken.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,9 @@ void Scanner::ScanValue() {
m_simpleKeyAllowed = InBlockContext();
}

// we are parsing a `key: value` pair; scalars are always allowed
m_scalarValueAllowed = true;

// eat
Mark mark = INPUT.mark();
INPUT.eat(1);
Expand Down Expand Up @@ -360,6 +363,10 @@ void Scanner::ScanQuotedScalar() {
// and scan
scalar = ScanScalar(INPUT, params);
m_simpleKeyAllowed = false;
// we just scanned a quoted scalar;
// we can only have another scalar in this line
// if we are in a flow, eg: `[ "foo", "bar" ]` is ok, but `"foo", "bar"` isn't.
m_scalarValueAllowed = InFlowContext();
m_canBeJSONFlow = true;

Token token(Token::NON_PLAIN_SCALAR, mark);
Expand Down
64 changes: 64 additions & 0 deletions test/integration/load_node_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,70 @@ TEST(NodeTest, LoadQuotedNull) {
EXPECT_EQ(node.as<std::string>(), "null");
}

TEST(NodeTest, LoadNonClosedQuotedString) {
EXPECT_THROW(Load(R"("foo)"), ParserException);
}

TEST(NodeTest, LoadWrongQuotedString) {
EXPECT_THROW(Load(R"("foo" [)"), ParserException);
EXPECT_THROW(Load(R"("foo", [)"), ParserException);
}

TEST(NodeTest, LoadUnquotedQuotedStrings) {
Node node = Load(R"(foo,"bar")");
EXPECT_EQ(node.as<std::string>(), "foo,\"bar\"");

node = Load(R"(foo,bar)");
EXPECT_EQ(node.as<std::string>(), "foo,bar");

node = Load(R"(foo,)");
EXPECT_EQ(node.as<std::string>(), "foo,");

node = Load(R"(foo "bar")");
EXPECT_EQ(node.as<std::string>(), "foo \"bar\"");
}

TEST(NodeTest, LoadCommaSeparatedStrings) {
EXPECT_THROW(Load(R"("foo","bar")"), ParserException);
EXPECT_THROW(Load(R"("foo",bar)"), ParserException);
EXPECT_THROW(Load(R"(,)"), ParserException);
EXPECT_THROW(Load(R"("foo",)"), ParserException);
EXPECT_THROW(Load(R"("foo","")"), ParserException);
EXPECT_THROW(Load(R"("foo",)"), ParserException);
EXPECT_THROW(Load(R"(,"foo")"), ParserException);
EXPECT_THROW(Load(R"(,foo)"), ParserException);
}

struct NewLineStringsTestCase {
std::string input;
std::string expected_content;
bool should_throw;
};
TEST(NodeTest, LoadNewLineStrings) {
std::vector<NewLineStringsTestCase> tests = {
{"foo\n, bar", "foo , bar", false},
{"foo\n, \"bar\"", "foo , \"bar\"", false},
{"\"foo\"\n, \"bar\"", "", true},
{"\"foo\"\n, bar", "", true},
};
for (const NewLineStringsTestCase& test : tests) {
if (test.should_throw) {
EXPECT_THROW(Load(test.input), ParserException);
} else {
Node node = Load(test.input);
Emitter emitter;
emitter << node;
EXPECT_EQ(NodeType::Scalar, node.Type());
EXPECT_EQ(test.expected_content, std::string(emitter.c_str()));
}
}
}

TEST(NodeTest, LoadSameLineStrings) {
EXPECT_THROW(Load(R"("foo" "bar")"), ParserException);
EXPECT_THROW(Load(R"("foo" bar)"), ParserException);
}

TEST(NodeTest, LoadTagWithParenthesis) {
Node node = Load("!Complex(Tag) foo");
EXPECT_EQ(node.Tag(), "!Complex(Tag)");
Expand Down