Marked: Any `def` string in code block prevents block detection

Created on 8 Jan 2018  路  19Comments  路  Source: markedjs/marked

When there is a def at the beginning of any line within a code block, it is not recognized as such.

Sample inputs

A

`
def`

B

```ruby
def foo(x)
  return 3
end
```

Expectation

The code block should be recognized as code block.

Result

The code block is not recognized and just rendered as paragraphs.

A

<p>`</p>
    <p>def
    `</p>

B

<p>`
    ruby</p>
    <p>def foo(x)
      return 3
    end
    `</p>

More information

This broke by commit https://github.com/chjj/marked/pull/976/commits/98ac7a43958014832788b9d01580f394c84bfb27
which seemed to have originated in https://github.com/chjj/marked/pull/974
and was supposed to fix https://github.com/chjj/marked/issues/465

L1 - broken code blocks help wanted

All 19 comments

I think we need to revert and review #974 as soon as possible. I don't recall how that could have slipped my checking.
This can be quite tricky since we changed the whole test system, so expect to manually resolve some conflicts.

@Feder1co5oave and @KostyaTretyak: Agreed.

I know of three ways to do that - and seem to only be able to do one of them myself. (Note: Believe it or not, I've never had to do a reversion.)

  1. Copy-paste and submit a new PR...not revert.
  2. https://help.github.com/articles/reverting-a-pull-request/ - in theory, there should be a button at the bottom of the PRs to allow reversion - there is not.
  3. Using git via terminal - https://git-scm.com/docs/git-revert.html - not comfortable enough to do that one myself for a live project in production.

@joshbruce, please do the following via terminal from root the project:

git checkout master
git revert a477d1d --no-edit
git push origin master

It should go through without problems. And now I will pull request for revert 98ac7a4.

@KostyaTretyak can you not open a PR to revert both commits? That would be neater.

Well, I fixed my mistake, but now you need to fix the tests.

Interestingly, CommonMark Spec have such a rule, but it seems that in our case it does not work for some reason (see tab HTML and [1]: foo at bottom).

> hello
> [1]: hello

* * *

> hello
[2]: hello


* hello
* [3]: hello


* hello
[4]: hello


> foo
> bar
[1]: foo
> bar

Output:

<blockquote>
<p>hello
[1]: hello</p>
</blockquote>
<hr />
<blockquote>
<p>hello
[2]: hello</p>
</blockquote>
<ul>
<li>
<p>hello</p>
</li>
<li></li>
<li>
<p>hello
[4]: hello</p>
</li>
</ul>
<blockquote>
<p>foo
bar
[1]: foo
bar</p>
</blockquote>

And this is not works too

[foo]

> first row
> [foo]: /url
> third row

Output:

<p>[foo]</p>
<blockquote>
<p>first row
[foo]: /url
third row</p>
</blockquote>

WAIT

I cannot reproduce the issue reported by @rndstr

~~~
$ bin/marked

def foo(x)
  return 3
end

^D

def foo(x)
  return 3
end

~~~

Guys, this is about _codeblocks_, #974 was about _blockquotes_, very different beasts.

Whereas this is a problem:
~~~
$ bin/marked
def ?

ciao?

^D

`

def ?`

ciao?

~~~

Looks like this is a problem with the paragraph block rule.

All right, I'm gonna just hold off until we answer a couple of things:

  • [ ] This issue is unrelated to the commits being reverted (a new issue).
  • [ ] Fixing this is something we should correct before performing the 0.3.10 release, which includes #991, which is related to the highest priority XSS-related defects.

This issue is unexpected related to #1007.

Yes it is correlated, because regexp are built via a cascade of replaces. By changing the blockquote regexp, the paragraph one is now changed too, and it breaks

`
def`

into two separate paragraphs, so that the code inline rule cannot parse correctly the item.
I'm working around this issue. Still, my recent review of your #974 is still very valid and we need to take a look at the blockquote rule too, which is one of the most complicated ones, alas.

Still, my recent review of your #974 is still very valid

Agree. I did #974 when I studied marked just a couple of days.

However, in two of your three comments, you were wrong with the rules. Now we know that my test edits were correct, but I was incorrect when I deleted rule with blockquote from marked.js.

Okay, so #1007 and 0.3.12 brought this back to the pre-0.3.9 behaviour. Thanks for the quick release!

Not sure I follow your discussion, feel free to close this (and re-open whatever #974 was meant to fix).

@rndstr: Thanks for the information. Gonna go ahead and close this per your comment.

Think we've completed all the issues related to XSS vulnerabilities; so, we can get to the other parser issues.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

FireflyAndStars picture FireflyAndStars  路  3Comments

toc
zoe-cjf picture zoe-cjf  路  3Comments

vsemozhetbyt picture vsemozhetbyt  路  4Comments

pigtooter picture pigtooter  路  4Comments

camwiegert picture camwiegert  路  4Comments