syntax-tree / mdast-util-to-markdown

mdast utility to serialize markdown
http://unifiedjs.com
MIT License
100 stars 20 forks source link

Error processing in `container-phrasing.js` with specific markup combination #63

Closed vnphanquang closed 3 weeks ago

vnphanquang commented 4 weeks ago

Initial checklist

Affected packages and versions

mdast-util-to-markdown@2.1.1, possibly mdast-util-gfm@3.0.0

Link to runnable example

https://github.com/vnphanquang/repro-mdast-util-to-markdown

Steps to reproduce

Reproduction

  1. Clone the repository: https://github.com/vnphanquang/repro-mdast-util-to-markdown
  2. Install dependencies (pnpm, yarn, npm)
  3. Run script: node index.js

More information

[!NOTE] this reproduction does involve mdast-util-gfm, however stack trace points to mdast-util-to-markdown.

Given this input:

<table>
  <tbody>
    <tr>
      <td>
        <p>Paragraph 1</p>
        <p>
          <em><strong>Paragraph 2</strong></em>
        </p>
      </td>
    </tr>
  </tbody>
</table>

I suspect the key troubling markup is at <em><strong>Paragraph 2</strong></em>. However, it does require an odd combination of being inside a table and also having multiple paragraphs in td to trigger the observed error.

From my test, skipping...

https://github.com/syntax-tree/mdast-util-to-markdown/blob/ec4eaf0cb0065afb755494403624edc8940ed333/lib/util/container-phrasing.js#L106

...produces a successful output. That is:

- results[results.length - 1].slice(-1)
+ results[results.length - 1]?.slice(-1)

However, I lack knowledge of the codebase to determine whether this change produces other false positive. Happy to open PR if given direction.

Expected behavior

Should output output.md

Actual behavior

Error thrown as observed in console:

[..truncated]/node_modules/.pnpm/mdast-util-to-markdown@2.1.1/node_modules/mdast-util-to-markdown/lib/util/container-phrasing.js:106
        before === results[results.length - 1].slice(-1)
                                               ^

TypeError: Cannot read properties of undefined (reading 'slice')
    at containerPhrasing ([..truncated]/node_modules/.pnpm/mdast-util-to-markdown@2.1.1/node_modules/mdast-util-to-markdown/lib/util/container-phrasing.js:106:48)
    at Object.containerPhrasingBound [as containerPhrasing] ([..truncated]/node_modules/.pnpm/mdast-util-to-markdown@2.1.1/node_modules/mdast-util-to-markdown/lib/index.js:138:10)
    at paragraph ([..truncated]/node_modules/.pnpm/mdast-util-to-markdown@2.1.1/node_modules/mdast-util-to-markdown/lib/handle/paragraph.js:16:23)
    at containerPhrasing ([..truncated]/node_modules/.pnpm/mdast-util-to-markdown@2.1.1/node_modules/mdast-util-to-markdown/lib/util/container-phrasing.js:50:11)
    at Object.containerPhrasingBound [as containerPhrasing] ([..truncated]/node_modules/.pnpm/mdast-util-to-markdown@2.1.1/node_modules/mdast-util-to-markdown/lib/index.js:138:10)
    at handleTableCell ([..truncated]/node_modules/.pnpm/mdast-util-gfm-table@2.0.0/node_modules/mdast-util-gfm-table/lib/index.js:216:25)
    at handleTableRowAsData ([..truncated]/node_modules/.pnpm/mdast-util-gfm-table@2.0.0/node_modules/mdast-util-gfm-table/lib/index.js:279:23)
    at handleTableAsData ([..truncated]/node_modules/.pnpm/mdast-util-gfm-table@2.0.0/node_modules/mdast-util-gfm-table/lib/index.js:255:23)
    at Object.handleTable ([..truncated]/node_modules/.pnpm/mdast-util-gfm-table@2.0.0/node_modules/mdast-util-gfm-table/lib/index.js:191:26)
    at Object.one [as handle] ([..truncated]/node_modules/.pnpm/zwitch@2.0.4/node_modules/zwitch/index.js:108:17)

Affected runtime and version

node@22.11.0

Affected package manager and version

pnpm@9.12.3

Affected OS and version

Linux 6.6 Arch Linux

Build and bundle tools

other (please specify in steps to reproduce)

wooorm commented 3 weeks ago

Thanks for the repro!

It’s a bit of a weird case: p in td, as that isn’t allowed in GFM markdown itself. But it’s fine in HTML.

Anyway, good to add a guard to that check indeed!

wooorm commented 3 weeks ago

Released in https://github.com/syntax-tree/mdast-util-to-markdown/releases/tag/2.1.2!

vnphanquang commented 3 weeks ago

Thanks a lot @wooorm. That was super quick :tada: