Custom classes for extra styling

michaelmyc commented 4 years ago

I was frustrated that there was no extra styling options available. The project was not actively maintained, and the dependencies are out of date and has security vulnerabilities. So I decided to fork the project and overhaul everything.

My package supports adding custom classes, ids, and attributes, so you would be able to more fancy styling with CSS. For example, you may want some images to be rendered small and some large, some centered and some aligned to the right. My new package would allow you to do so with ease.

Here's a demo: https://github.michaelmao.me/vue-markdown-plus. You can find it on NPM here: https://www.npmjs.com/package/vue-markdown-plus.

pcmacdon commented 3 years ago

For others considering using -plus, I gave it a try noted a number of issues.

Site has no list of feature enhancements/differences.
The styling feature could be implemented using postrender.
The dist contains only minified source, which is a security issue in itself.

michaelmyc commented 3 years ago

For others considering using -plus, I gave it a try noted a number of issues.

Site has no list of feature enhancements/differences.

The styling feature could be implemented using postrender.

The dist contains only minified source, which is a security issue in itself.

Good suggestions.

It's a quick feature add I mostly intend to use myself. I forgot to thoroughly update the README. I'll do that soon.
Can you elaborate more about your comment on postrender? I think even if you try to change styling through javascript, you would still need to have to specify class/id for a particular segment of the content, which isn't offered here. That is the feature added in the -plus repo.
This repo does not offer prettified code either, and since I basically forked it, I didn't make changes to the build pipeline. You can check the commit history to see the differences (fairly minor). Or you can also prettify it to see what's in there. It make sense that a non-prettified one will make debugging a lot easier, but I'm not sure I understand why it is a security issue. Could you also elaborate on why this is a security issue?

pcmacdon commented 3 years ago

Looking only at your first example, a pattern such as [work]{.class} would be fairly simple to use with "replace" in postrender, although admittedly more difficult with code blocks.

I just looked at the vue-markdown repo and dist/* is not minified, unlike -plus.

wc vue-markdown-master/dist/vue-markdown-plus.js 
  22447  104344 1017818 vue-markdown-master-plus/dist/vue-markdown.js
wc vue-markdown-plus-master/dist/vue-markdown-plus.js 
    11   2918 561903 vue-markdown-plus-master/dist/vue-markdown-plus.js

The security issue arises due to the inability to run "diff" to see what lines you had changed from the original.

I ran into a real life example yesterday going to http://particlephysics.com/ which Ublock origin broke due to including:

http://img.sedoparking.com/js/jquery-1.11.3.custom.min.js

Turns out sedoparking is a known malware site, and worse nefarious scammers often claim to be providing "security improved versions".

Finally, the prettify version is not really human readable.

eg. it was fairly easily able to add the missing "tocAnchorLinkBefore" option to vue-markdown.

53,256d252
<           tocAnchorLinkBefore: {
<             type: Boolean,
<             default: true
<           },
331d326
<               anchorLinkBefore: this.tocAnchorLinkBefore,

Try that with minified js.

Yes, I could have got node and rebuilt the package, but that doesn't help when I was trying to debug the above problem.

pcmacdon commented 3 years ago

BTW, on the issue of security: your demo page https://github.michaelmao.me/vue-markdown-plus/

I get GET https://static.cloudflareinsights.com/beacon.min.js net::ERR_BLOCKED_BY_CLIENT

The first result from a google search "beacon.min.js" is

Beacon.min.js as Malware - General - Cloudflare Community https://community.cloudflare.com/t/beacon-min-js-as-malware/117552

Ok, maybe it's not actually malware, but minifying makes it difficult to determine tampering one way or another.

michaelmyc commented 3 years ago

BTW, on the issue of security: your demo page https://github.michaelmao.me/vue-markdown-plus/

I get GET https://static.cloudflareinsights.com/beacon.min.js net::ERR_BLOCKED_BY_CLIENT

The first result from a google search "beacon.min.js" is

Beacon.min.js as Malware - General - Cloudflare Community https://community.cloudflare.com/t/beacon-min-js-as-malware/117552

Ok, maybe it's not actually malware, but minifying makes it difficult to determine tampering one way or another.

That is most likely because I'm using cloudflare. I'm not getting that on my chrome browser. Which browser are you using, and which version is your browser? I'll turn off the feature anyways since it has no real use to me.

pcmacdon commented 3 years ago

I use chrome as well, but it is Ublock Origin blocking it, not the browser.

michaelmyc commented 3 years ago

Looking only at your first example, a pattern such as [work]{.class} would be fairly simple to use with "replace" in postrender, although admittedly more difficult with code blocks.

I just looked at the vue-markdown repo and dist/* is not minified, unlike -plus.
wc vue-markdown-master/dist/vue-markdown-plus.js 
  22447  104344 1017818 vue-markdown-master-plus/dist/vue-markdown.js
wc vue-markdown-plus-master/dist/vue-markdown-plus.js 
    11   2918 561903 vue-markdown-plus-master/dist/vue-markdown-plus.js
The security issue arises due to the inability to run "diff" to see what lines you had changed from the original.

I ran into a real life example yesterday going to http://particlephysics.com/ which Ublock origin broke due to including:

http://img.sedoparking.com/js/jquery-1.11.3.custom.min.js

Turns out sedoparking is a known malware site, and worse nefarious scammers often claim to be providing "security improved versions".

Finally, the prettify version is not really human readable.

eg. it was fairly easily able to add the missing "tocAnchorLinkBefore" option to vue-markdown.
53,256d252
<           tocAnchorLinkBefore: {
<             type: Boolean,
<             default: true
<           },
331d326
<               anchorLinkBefore: this.tocAnchorLinkBefore,
Try that with minified js.

Yes, I could have got node and rebuilt the package, but that doesn't help when I was trying to debug the above problem.

I'll take a look. I'll generate both minified and non-minified versions.

michaelmyc commented 3 years ago

I use chrome as well, but it is Ublock Origin blocking it, not the browser.

Funny. I also have uBlock. It's not happening to me. I turned off that feature and it shouldn't be an issue anymore.

michaelmyc commented 3 years ago

Actually I found out that markdown-it-katex has XSS vulnerabilities and it's not been updated for 4 years. Will look into some drop-in replacements.

michaelmyc commented 3 years ago

I realized there are some more serious issues with vue-markdown-it, as it doesn't protect against xss. It doesn't look trivial to solve it and I don't have the time to fix this at the moment.

pcmacdon commented 3 years ago

Do you mean there are xss problems outside of "html" and "katex"? I know only the basics of xss, but it seems to me the enabling the "html" feature is problematic. And I would think you should also be able to disable "katex", especially if it has potential xss issues.

pcmacdon commented 3 years ago

Re: Katex. Looks to me like the xss exploit is only if you enable "raw html" in katex (disabled by default) ie. this is ignored when used in vue-markdown:

$\href{javascript:alert('hello');}{test}$

michaelmyc commented 3 years ago

$<img src="" onerror="alert('hi')" />%$ will run. It's a known issue for markdown-it-katex. I tried sanitizing the output html before mounting, but that didn't help prevent script execution. I tried to use markdown-it-katexx, which claims to fix xss, and their demo page actually did prevent scripts from executing. That didn't help even with katex rendering. Needs more time to triage the issue, but I'm pretty busy atm. I'd be happy to review your code if you open a PR.

pcmacdon commented 3 years ago

There are two different issues here.

The first and most important one is that vue-markdown use of Katex should (like emoji) be optional: see patch below.

The other (katex not doing the right thing) is more open ended. And not really relevant to the 99.99% of markdown users who do not need math rendering.

And sure, blog sites could still allow conditional math use via a moderator.

--- vue-markdown.js.orig    2020-07-14 15:28:15.698495629 -0700
+++ vue-markdown.js 2020-08-13 15:28:36.352299159 -0700
@@ -250,6 +250,10 @@
          type: Boolean,
          default: true
        },
+       tocAnchorLinkBefore: {
+         type: Boolean,
+         default: true
+       },
        tocAnchorLinkClass: {
          type: String,
          default: 'toc-anchor-link'
@@ -271,6 +275,10 @@
          default: function _default(htmlData) {
            return htmlData;
          }
+       },
+       katex: {
+         type: Boolean,
+         default: false
        }
      },

@@ -283,8 +291,10 @@
      render: function render(createElement) {
        var _this = this;

-       this.md = new _markdownIt2.default().use(_markdownItSub2.default).use(_markdownItSup2.default).use(_markdownItFootnote2.default).use(_markdownItDeflist2.default).use(_markdownItAbbr2.default).use(_markdownItIns2.default).use(_markdownItMark2.default).use(_markdownItKatex2.default, { "throwOnError": false, "errorColor": " #cc0000" }).use(_markdownItTaskLists2.default, { enabled: this.taskLists });
-
+       this.md = new _markdownIt2.default().use(_markdownItSub2.default).use(_markdownItSup2.default).use(_markdownItFootnote2.default).use(_markdownItDeflist2.default).use(_markdownItAbbr2.default).use(_markdownItIns2.default).use(_markdownItMark2.default).use(_markdownItTaskLists2.default, { enabled: this.taskLists });
+        if (this.katex) {
+            this.md.use(_markdownItKatex2.default, { "throwOnError": false, "errorColor": " #cc0000" });
+        }
        if (this.emoji) {
          this.md.use(_markdownItEmoji2.default);
        }
@@ -324,6 +334,7 @@
            tocLastLevel: this.tocLastLevelComputed,
            anchorLink: this.tocAnchorLink,
            anchorLinkSymbol: this.tocAnchorLinkSymbol,
+           anchorLinkBefore: this.tocAnchorLinkBefore,
            anchorLinkSpace: this.tocAnchorLinkSpace,
            anchorClassName: this.tocAnchorClass,
            anchorLinkSymbolClassName: this.tocAnchorLinkClass,
@@ -10649,6 +10660,7 @@
        anchorLinkBefore: true,
        anchorClassName: "markdownIt-Anchor",
        resetIds: true,
+       katex:false,
        anchorLinkSpace: true,
        anchorLinkSymbolClassName: null
      }, options);

pcmacdon commented 3 years ago

To answer your previous question, here is how we might use "postrender" to provide block spans with attributes. eg.

   here is some
   ::: red
   important stuff
   :::
   for you!

postrender looks something like:

        postrender(val) {
            return val.replace(/\n:::(.*?)\n:::/gms, function (mat, pat) {
                let attrs, opts = pat.match(/^\s*([^\n]+)\n(.*)$/);
                if (opts && opts.length>=3) {
                    attrs = opts[1].trim();
                    pat = opts[2];
                    let sattrs = '';
                    if (attrs[0] == '{' && attrs[attrs.length-1]==='}') { // TODO: parse {attrs} options...
                    } else {
                        sattrs = '  class="'+attrs+'"'; // Insert as class.
                    }
                    pat = '<span'+sattrs+'>'+pat+'</span>';
                }
                return '\n'+pat+'\n';
            });
        },

FYI, I grabbed webpack, rebuilt vue-markdown adding spans/attributes: the result was buggy.

michaelmyc commented 3 years ago

I investigated a little more. The XSS issue seems to only apply to the simple html example but not the vue and webpack examples (both uses webpack). I'm not sure why that happens, but it seems like webpack is doing something to combat XSS. I'm waiting for my PR on markdown-it-katex to get accepted and I'll add a warning about this.

michaelmyc commented 3 years ago

Postrender looks complicated. I feel the whole point of markdown is to make things simple, and I feel having custom clas/id/attributes and slight modifications in CSS makes things very simple. Postrender can be used for fancier stuff.

What do you mean by adding spans/attributes is buggy? I'm using markdown-it-bracketed-spans, and it's not giving me any trouble.

pcmacdon commented 3 years ago

My bad. After updating nodejs and webpack to all newest versions the problems with attrs went way. And yes, postrender is non-trivial.

miaolz123 / vue-markdown

Custom classes for extra styling #120