Restore old anchor generation behaviour from before v1.15.0 #578
Conversation
There was a problem hiding this comment.
The original anchor issue arose because [email protected] was too liberal in using regex on unsafe user input. Hence it got reported as a vulnerability.
For that reason, I don't think we should re-adopt and re-adapt their code, and definitely not try maintaining their code lest we get hit with the same problem. :P
Let's explore some npm packages first like slugify. They used to be reported with similar vulnerabilities but they seem to have fixed it with their later versions. So let's import their packages whenever possible, instead of implementing and maintaining our own slugify. This will make it easier to fix the issue on vue-strap too.
|
@yamgent if you are referring to jprichardson/string.js#212, note that the regex in question are not the ones used in the logic of Nevertheless, I do agree that if there is a external package available that is maintained with this functionality, we should probably go with that instead. Will look into simov/slugify. Thanks for the heads up! Update: simov/slugify replaces |
|
@sindresorhus/slugify seems to work. |
The main MarkBind project has switched to using the @sindresorhus/slugify library [1] to generate the anchor IDs for headings. The switch is done in one of the recently merged PR [2]. Our panels still uses the old anchor ID generation algorithm for headings in its header. As a step towards allowing panels to use the same anchor ID generation algorithm, let's install @sindresorhus/slugify. [1] - https://github.com/sindresorhus/slugify [2] - MarkBind/markbind#578
The main MarkBind project has switched to using the @sindresorhus/slugify library[1] to generate the anchor IDs for headings. The switch is done in one of the recently merged PR in the main MarkBind project[2]. Our panels still uses the old anchor ID generation algorithm for headings in its header. Let's switch the anchor ID generation algorithm of panels to use @sindresorhus/slugify. [1] - https://github.com/sindresorhus/slugify [2] - MarkBind/markbind#578
What is the purpose of this pull request? (put "X" next to an item, remove the rest)
• [X] Bug fix
Resolves #576.
What is the rationale for this request?
When we upgraded
markdown-it-anchorto5.0.0in #469, we also changed the anchor tag generation behaviour: instead of ignoring special characters, the new version now URL-encodes them instead. For example, "Guideline: Avoid Unsafe Shortcuts" becomesguideline%3A-avoid-unsafe-shortcuts. This breaks compatibility with headings generated by GFMD, which expectsguideline-avoid-unsafe-shortcuts.What changes did you make? (Give an overview)
I added
@sindresorhus/slugifywhich provides slugify behaviour similar to what we had previously, and configuredmarkdown-it-anchorto use this new library.Provide some example code that this change will affect:
## Guideline: Avoid Unsafe ShortcutsIs there anything you'd like reviewers to focus on?
This change needs to be coupled with a change to MarkBind/vue-strap to revert similar changes made in MarkBind/vue-strap#92.
Testing instructions:
:. The anchor link should not contain special characters.