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 1 commit
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
63 changes: 63 additions & 0 deletions test/integration/load_node_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,69 @@ 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);

const char *doc_ok = R"(
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to EXPECT_EQ what these actually parse to? (And maybe put them in the above "valid" test or their own one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, thanks!

top
, aa
)";
EXPECT_NO_THROW(Load(doc_ok));

const char *doc_ok_2 = R"(
top
, "aa"
)";
EXPECT_NO_THROW(Load(doc_ok_2));

const char *doc_fail = R"(
"top"
, "aa"
)";
EXPECT_THROW(Load(doc_fail), ParserException);

const char *doc_fail_2 = R"(
"top"
, aa
)";
EXPECT_THROW(Load(doc_fail_2), ParserException);
}

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
Loading