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.

Sign up for GitHub

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

Jump to bottom

HTML parser: certain nested HTML elements cause fatal crash #302

Closed
a-murph opened this issue Apr 26, 2019 · 5 comments · Fixed by #563
Closed

HTML parser: certain nested HTML elements cause fatal crash #302

a-murph opened this issue Apr 26, 2019 · 5 comments · Fixed by #563
Labels
🐛 type/bug This is a problem

Comments

@a-murph
Copy link

a-murph commented Apr 26, 2019

Nesting certain HTML elements within the source string that is sent to ReactMarkdown causes a fatal crash. In particular I have noticed issues with the mark and span tags, though even divs can cause this crash in some circumstances.

This occurs when passing a string that includes HTML tags as the source prop to the ReactMarkdown element, with escapeHtml set to false. The project in which I encountered this issue is using ReactMarkdown 4.0.6, though the same behavior is replicable in the live editor, replacing the example blockquote with other HTML.

Here are some examples of HTML that causes ReactMarkdown to crash:

  • <span><span>Text</span></span>
  • <div><div>Text</div></div>
  • <mark><mark>Text</mark></mark>

divs seem to usually avoid the crash if there is whitespace between the tags. The following all render properly:

  • <div> <div> Text </div> </div>
  • <div> <div>Text</div> </div>
  • <div><div>Text</div> </div>

Although this crashes:

  • <div><div> Text </div></div>

spans, marks, and other inline elements crash even with spacing. The following all crash:

  • <mark> <mark> Text </mark> </mark>
  • <span> <span> Text </span> </span>
  • <em> <em> Text </em> </em>

Here is the error that my project produces in the console when this crash occurs:

ast-to-react.js:214 Uncaught TypeError: Cannot read property 'props' of undefined
at mergeNodeChildren (ast-to-react.js:214)
at getNodeProps (ast-to-react.js:175)
at astToReact (ast-to-react.js:18)
at ast-to-react.js:23
at Array.map ()
at resolveChildren (ast-to-react.js:22)
at astToReact (ast-to-react.js:19)
at ast-to-react.js:23
at Array.map ()
at resolveChildren (ast-to-react.js:22)
@polskikiel
Copy link

In my case two <img> tags in one line causes the crash of the ReactMarkdown. On the live editor I get the following output:

TypeError: Cannot read property 'start' of undefined
    at demo.js:14
    at demo.js:14
    at demo.js:1
    at a (demo.js:14)
    at demo.js:14
    at a (demo.js:14)
    at demo.js:14
    at a (demo.js:14)
    at s (demo.js:14)
    at a (demo.js:1)

If I make a newline between the tags the ReactMarkdown handle that.

@ianhe8x
Copy link

ianhe8x commented Oct 9, 2019

it was claimed solved in #43 but unfortunately not.

@algomaster99 algomaster99 mentioned this issue Oct 19, 2019
Closed
@jorgeorpinel
Copy link

We're having a similar problem with <abbr> elements inside <details>. See iterative/dvc.org/issues/510 for context. Hoping for a resolution 🤞

fabiosantoscode added a commit to fabiosantoscode/react-markdown that referenced this issue Jan 24, 2020
@fabiosantoscode
allow nesting tags and close correct tags when multiple tags exist. f…
9dbb56f 
…ixes remarkjs#302
@fabiosantoscode fabiosantoscode mentioned this issue Jan 24, 2020
Closed
@ChristianMurphy
Copy link
Member

If merged #428 would allow this to be fixed.

@ChristianMurphy ChristianMurphy added the 🐛 type/bug This is a problem label Oct 8, 2020
@wooorm wooorm changed the title Certain nested HTML elements cause fatal crash HTML parser: certain nested HTML elements cause fatal crash Oct 19, 2020
@rcowsill rcowsill mentioned this issue Oct 25, 2020
Closed
@ChristianMurphy ChristianMurphy mentioned this issue Jan 21, 2021
Closed
@wooorm wooorm mentioned this issue Apr 9, 2021
Merged
wooorm added a commit that referenced this issue Apr 12, 2021
@wooorm
Replace renderers w/ components, remove HTML parser from core (#563)
77c680f 
* Replace `renderers` w/ `components`
* Replace `allowNode` w/ `allowElement`, which is now given a hast element (as
  the first parameter)
* Replace `allowedTypes` w/ `allowedElements`
* Replace `disallowedTypes` w/ `disallowedElements`
* Change signature of `linkTarget` and `transformLinkUri`, which are now given
  hast children (as the second parameter)
* Change signature of `transformImageUri`, which is now given the `alt` string
  as the second parameter (instead of the fourth)
* Replace `plugins` w/ `remarkPlugins` (backwards compatible change)
* Add `rehypePlugins`
* Change `includeNodeIndex` to `includeElementIndex`: it still sets an `index`,
  but that value now represents the number of preceding elements, it also sets a
  `siblingCount` (instead of `parentChildCount`) with the number of sibling
  elements in the parent
* The `columnAlignment` prop is no longer given to table elements: it’s
  available as `style` on `th` and `td` elements instead
* The `spread` prop is no longer given to list elements: it’s already handled

Remove buggy HTML parsers from core

* If you want HTML, add [`rehype-raw`](https://github.com/rehypejs/rehype-raw)
  to `rehypePlugins` and it’ll work without bugs!
* Remove `allowDangerousHtml` (previously called `escapeHtml`) option: pass
  `rehype-raw` in `rehypePlugins` to allow HTML instead
* Remove `with-html.js`, `plugins/html-parser.js` entries from library
* Remove naïve HTML parser too: either use `rehype-raw` to properly support
  HTML, or don’t allow it at all

Closes GH-549.
Closes GH-563.

The following issues are solved as rehype is now available:

Closes GH-522.
Closes GH-465.
Closes GH-427.
Closes GH-384.
Closes GH-356.

The following issues are solved as a proper HTML parser (`rehype-raw`) is now
available:

Closes GH-562.
Closes GH-460.
Closes GH-454.
Closes GH-452.
Closes GH-433.
Closes GH-386.
Closes GH-385.
Closes GH-345.
Closes GH-320.
Closes GH-302.
Closes GH-267.
Closes GH-259.

The following issues are solved as docs are improved:

Closes GH-251.
@wooorm
Copy link
Member

wooorm commented Apr 12, 2021

This should be solved by landing GH-563 today, which will soon be released in v6.0.0!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
🐛 type/bug This is a problem
Milestone
No milestone
6 participants
@wooorm @jorgeorpinel @ChristianMurphy @polskikiel @a-murph @ianhe8x