nested parentheses link

[UziTech]

Marked version: 0.6.0

Description

This is my proposed fix for #1405. It works with allowing multiple nested parentheses.

fixes #1405
closes #1408

Contributor

• Test(s) exist to ensure functionality and minimize regression (if no tests added, list tests covering this PR)

Committer

In most cases, this should be a different person than the contributor.

• Draft GitHub release notes have been updated.
• CI is green (no forced merge required).
• Merge PR

[UziTech]

I'd like to get opinions from @Feder1co5oave and @davisjam on this approach.

[UziTech]

We should merge this soon and release a new version to fix https://snyk.io/vuln/SNYK-JS-MARKED-73637

[styfle]

@UziTech Why is this PR still failing snyk? (I can't actually see the Details when I click it)

[UziTech]

The snyk check is failing because of https://snyk.io/vuln/SNYK-JS-MARKED-73637

maybe we should disable that check since it seems to be more of a false positive on most PRs.

[styfle]

But enough people use snyk that its probably good to know when we are failing.

It sounds like the one thats failing is not related to this PR so I see why that's confusing.

[joshbruce]

I'm approving because I appreciate that this fixes a defect; however, before merging I would really like to know more about the Snyk thing (same with @styfle). I don't want to disable Snyk because they are widely, they've always done right by us, and it is the one area we said we would take very seriously.

[UziTech]

The Snyk Github integration matches package version and dependencies to vulnerabilities in their database.

Since there is a vulnerability in the current version of marked, all new PRs will fail the Snyk check.

Marked doesn't have any regular dependencies so the only thing the Snyk check will tell us is if the current version of marked has a vulnerability or if there is a vulnerability in one of our dev dependencies (neither of which I think is especially helpful to check on every new PR)

[Feder1co5oave]

Hi, this looks good to me, although I'm not sure escaping works as expected: can you verify inputs such as

[Text](uri\()
[Text](uri(\())
[Text](uri\\())

? Unfortunately I can't do it myself right now

[UziTech]

demo

it looks like it works

[UziTech]

If the function sees a \ it just skips the next character

[styfle]

only thing the Snyk check will tell us is if the current version of marked has a vulnerability

Let’s remove the Snyk check on PRs (I think @joshbruce has to disable it)

Maybe we should add the badge to the README

https://snyk.io/test/npm/marked/badge.svg

[joshbruce]

I'm fine with that - sound argument from @UziTech and good suggestion from @styfle.

Can we also double check a few things on the GitHub operations front?

You both are admins.

1. You should have access to the settings tab + web hooks section. There you should be able to delete Snyk...could be wrong...just double checking.

2. We also have WIP inside the integrations area. Are folks using it (are they even away of it)? If not, makes me question its value. Further GitHub released the idea of draft PRs that kinda offer the same protection; therefore, thinking we should default to native experience over integrating a package: https://help.github.com/articles/about-pull-requests/#draft-pull-requests

Thoughts and feedback, as always, welcome.

[Feder1co5oave]

[demo](https://docs-8bxhez41v.now.sh/demo/?outputType=html&text=%5BText%5D(uri%5C()%0A%5BText%5D(uri(%5C())%0A%5BText%5D(uri%5C%5C())&options=&version=pr%3A1414)

it looks like it works

I just noticed that page automatically redirects to version=master!
Checking out locally, it really seems working:

[Text](uri\()
[Text](uri(\())
[Text](uri\\())
[Text](uri\)\))
[Text](uri\(\(())
^D

<p><a href="uri(">Text</a>
<a href="uri(()">Text</a>
<a href="uri%5C()">Text</a>
<a href="uri))">Text</a></p>
<p><a href="uri((()">Text</a></p>