Skip to content
This repository has been archived by the owner on Nov 8, 2024. It is now read-only.

Commit

Permalink
refactor: revert "fix: avoid uninitialized read in markdown-parser"
Browse files Browse the repository at this point in the history
This reverts commit 11a8788.
This reverts commit 893921c.

Fixes #738
  • Loading branch information
kylef committed Sep 10, 2019
1 parent a1cba47 commit 77028db
Show file tree
Hide file tree
Showing 28 changed files with 346 additions and 258 deletions.
25 changes: 24 additions & 1 deletion ext/snowcrash/ext/markdown-parser/src/MarkdownNode.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,34 @@
using namespace mdp;

MarkdownNode::MarkdownNode(MarkdownNodeType type_, MarkdownNode* parent_, const ByteBuffer& text_, const Data& data_)
: type(type_), text(text_), data(data_), sourceMap(), m_parent(parent_), m_children(nullptr)
: type(type_), text(text_), data(data_), m_parent(parent_)
{
m_children.reset(::new MarkdownNodes);
}

MarkdownNode::MarkdownNode(const MarkdownNode& rhs)
{
this->type = rhs.type;
this->text = rhs.text;
this->data = rhs.data;
this->sourceMap = rhs.sourceMap;
this->m_children.reset(::new MarkdownNodes(*rhs.m_children.get()));
this->m_parent = rhs.m_parent;
}

MarkdownNode& MarkdownNode::operator=(const MarkdownNode& rhs)
{
this->type = rhs.type;
this->text = rhs.text;
this->data = rhs.data;
this->sourceMap = rhs.sourceMap;
this->m_children.reset(::new MarkdownNodes(*rhs.m_children.get()));
this->m_parent = rhs.m_parent;
return *this;
}

MarkdownNode::~MarkdownNode() {}

MarkdownNode& MarkdownNode::parent()
{
if (!hasParent())
Expand Down
8 changes: 3 additions & 5 deletions ext/snowcrash/ext/markdown-parser/src/MarkdownNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,15 +82,13 @@ namespace mdp
const Data& data_ = Data());

/** Copy constructor */
MarkdownNode(const MarkdownNode& rhs) = delete;
MarkdownNode(MarkdownNode&& rhs) = default;
MarkdownNode(const MarkdownNode& rhs);

/** Assignment operator */
MarkdownNode& operator=(const MarkdownNode& rhs) = delete;
MarkdownNode& operator=(MarkdownNode&& rhs) = default;
MarkdownNode& operator=(const MarkdownNode& rhs);

/** Destructor */
~MarkdownNode() = default;
~MarkdownNode();

#ifdef DEBUG
/** Prints the node to the stderr */
Expand Down
30 changes: 18 additions & 12 deletions ext/snowcrash/ext/markdown-parser/src/MarkdownParser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ static ByteBuffer ByteBufferFromSundown(const struct buf* text)

MarkdownParser::MarkdownParser() : m_workingNode(NULL), m_listBlockContext(false), m_source(NULL), m_sourceLength(0) {}

MarkdownNode MarkdownParser::parse(const ByteBuffer& source)
void MarkdownParser::parse(const ByteBuffer& source, MarkdownNode& ast)
{
MarkdownNode ast{};
ast = MarkdownNode();
m_workingNode = *
m_workingNode->type = RootMarkdownNodeType;
m_workingNode->sourceMap.push_back(BytesRange(0, source.length()));
Expand All @@ -57,8 +57,6 @@ MarkdownNode MarkdownParser::parse(const ByteBuffer& source)
m_source = NULL;
m_sourceLength = 0;
m_listBlockContext = false;

return ast;
}

MarkdownParser::RenderCallbacks MarkdownParser::renderCallbacks()
Expand Down Expand Up @@ -107,7 +105,8 @@ void MarkdownParser::renderHeader(const ByteBuffer& text, int level)
if (!m_workingNode)
throw NO_WORKING_NODE_ERR;

m_workingNode->children().emplace_back(HeaderMarkdownNodeType, m_workingNode, text, level);
MarkdownNode node(HeaderMarkdownNodeType, m_workingNode, text, level);
m_workingNode->children().push_back(node);
}

void MarkdownParser::beginList(int flags, void* opaque)
Expand Down Expand Up @@ -153,7 +152,8 @@ void MarkdownParser::beginListItem(int flags)
if (!m_workingNode)
throw NO_WORKING_NODE_ERR;

m_workingNode->children().emplace_back(ListItemMarkdownNodeType, m_workingNode, ByteBuffer(), flags);
MarkdownNode node(ListItemMarkdownNodeType, m_workingNode, ByteBuffer(), flags);
m_workingNode->children().push_back(node);

// Push context
m_workingNode = &m_workingNode->children().back();
Expand All @@ -180,7 +180,8 @@ void MarkdownParser::renderListItem(const ByteBuffer& text, int flags)
// Instead of storing the text on the list item
// create the artificial paragraph node to store the text.
if (m_workingNode->children().empty() || m_workingNode->children().front().type != ParagraphMarkdownNodeType) {
m_workingNode->children().emplace_front(ParagraphMarkdownNodeType, m_workingNode, text);
MarkdownNode textNode(ParagraphMarkdownNodeType, m_workingNode, text);
m_workingNode->children().push_front(textNode);
}

m_workingNode->data = flags;
Expand All @@ -203,7 +204,8 @@ void MarkdownParser::renderBlockCode(const ByteBuffer& text, const ByteBuffer& l
if (!m_workingNode)
throw NO_WORKING_NODE_ERR;

m_workingNode->children().emplace_back(CodeMarkdownNodeType, m_workingNode, text);
MarkdownNode node(CodeMarkdownNodeType, m_workingNode, text);
m_workingNode->children().push_back(node);
}

void MarkdownParser::renderParagraph(struct buf* ob, const struct buf* text, void* opaque)
Expand All @@ -220,7 +222,8 @@ void MarkdownParser::renderParagraph(const ByteBuffer& text)
if (!m_workingNode)
throw NO_WORKING_NODE_ERR;

m_workingNode->children().emplace_back(ParagraphMarkdownNodeType, m_workingNode, text);
MarkdownNode node(ParagraphMarkdownNodeType, m_workingNode, text);
m_workingNode->children().push_back(node);
}

void MarkdownParser::renderHorizontalRule(struct buf* ob, void* opaque)
Expand All @@ -237,7 +240,8 @@ void MarkdownParser::renderHorizontalRule()
if (!m_workingNode)
throw NO_WORKING_NODE_ERR;

m_workingNode->children().emplace_back(HRuleMarkdownNodeType, m_workingNode, ByteBuffer(), MarkdownNode::Data());
MarkdownNode node(HRuleMarkdownNodeType, m_workingNode, ByteBuffer(), MarkdownNode::Data());
m_workingNode->children().push_back(node);
}

void MarkdownParser::renderHTML(struct buf* ob, const struct buf* text, void* opaque)
Expand All @@ -254,7 +258,8 @@ void MarkdownParser::renderHTML(const ByteBuffer& text)
if (!m_workingNode)
throw NO_WORKING_NODE_ERR;

m_workingNode->children().emplace_back(HTMLMarkdownNodeType, m_workingNode, text);
MarkdownNode node(HTMLMarkdownNodeType, m_workingNode, text);
m_workingNode->children().push_back(node);
}

void MarkdownParser::beginQuote(void* opaque)
Expand All @@ -271,7 +276,8 @@ void MarkdownParser::beginQuote()
if (!m_workingNode)
throw NO_WORKING_NODE_ERR;

m_workingNode->children().emplace_back(QuoteMarkdownNodeType, m_workingNode);
MarkdownNode node(QuoteMarkdownNodeType, m_workingNode);
m_workingNode->children().push_back(node);

// Push context
m_workingNode = &m_workingNode->children().back();
Expand Down
4 changes: 2 additions & 2 deletions ext/snowcrash/ext/markdown-parser/src/MarkdownParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ namespace mdp
* \brief Parse source buffer
*
* \param source Markdown source data to be parsed
* \return Parsed AST (root node)
* \param ast Parsed AST (root node)
*/
MarkdownNode parse(const ByteBuffer& source);
void parse(const ByteBuffer& source, MarkdownNode& ast);

private:
MarkdownNode* m_workingNode;
Expand Down
9 changes: 6 additions & 3 deletions ext/snowcrash/ext/markdown-parser/test/test-ByteBuffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,14 @@ using namespace mdp;
TEST_CASE("Multi-byte characters Czech", "[bytebuffer][sourcemap]")
{
MarkdownParser parser;
MarkdownNode ast;

ByteBuffer src
= "\x50\xC5\x99\xC3\xAD\xC5\xA1\x65\x72\x6E\xC4\x9B\x20\xC5\xBE\x6C\x75\xC5\xA5\x6F\x75\xC4\x8D\x6B\xC3\xBD\x20"
"\x6B\xC5\xAF\xC5\x88\x20\xC3\xBA\x70\xC4\x9B\x6C\x20\xC4\x8F\xC3\xA1\x62\x65\x6C\x73\x6B\xC3\xA9\x20\xC3\xB3"
"\x64\x79\n";

MarkdownNode ast = parser.parse(src);
parser.parse(src, ast);

REQUIRE(ast.type == RootMarkdownNodeType);
REQUIRE(ast.text.empty());
Expand All @@ -42,11 +43,12 @@ TEST_CASE("Multi-byte characters Czech", "[bytebuffer][sourcemap]")
TEST_CASE("Multi-byte characters in blockquote", "[bytebuffer][sourcemap]")
{
MarkdownParser parser;
MarkdownNode ast;

// "> Ni Hao"
ByteBuffer src = "> \xE4\xBD\xA0\xE5\xA5\xBD\n";

MarkdownNode ast = parser.parse(src);
parser.parse(src, ast);

REQUIRE(ast.type == RootMarkdownNodeType);
REQUIRE(ast.text.empty());
Expand Down Expand Up @@ -120,6 +122,7 @@ TEST_CASE("Character index")
TEST_CASE("Byte buffer and Index should provide equal information", "[bytebuffer][sourcemap]")
{
MarkdownParser parser;
MarkdownNode ast;

ByteBuffer src
= "\x50\xC5\x99\xC3\xAD\xC5\xA1\x65\x72\x6E\xC4\x9B\x20\xC5\xBE\x6C\x75\xC5\xA5\x6F\x75\xC4\x8D\x6B\xC3\xBD\x20"
Expand All @@ -129,7 +132,7 @@ TEST_CASE("Byte buffer and Index should provide equal information", "[bytebuffer
ByteBufferCharacterIndex index;
mdp::BuildCharacterIndex(index, src);

MarkdownNode ast = parser.parse(src);
parser.parse(src, ast);

CharactersRangeSet charMap = BytesRangeSetToCharactersRangeSet(ast.sourceMap, src);
CharactersRangeSet indexMap = BytesRangeSetToCharactersRangeSet(ast.sourceMap, index);
Expand Down
Loading

0 comments on commit 77028db

Please sign in to comment.