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.

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

Change markdown module (markdown 0.5.0 to marked 0.5.2) #3170

Closed
wants to merge 4 commits into from

Conversation

44int
Copy link

@44int 44int commented Dec 7, 2018

"markdown 0.5.0" will escape the entered HTML.
Therefore, even if "REDASH_ALLOW_SCRIPTS_IN_USER_INPUT = true" is enabled, it will not be rendered as intended.
By using marked 0.5.2, the above problem can be avoided and markdown can also be made effective.

Related pull requests are as follows.
#2438

Marked documents
https://marked.js.org/#/USING_ADVANCED.md

@deecay
Copy link
Contributor

deecay commented Dec 7, 2018

Related to #1966, #2146 ? Is vulnerability in marked resolved? I do not believe HTML will be allowed anyways?

@44int
Copy link
Author

44int commented Dec 11, 2018

@deecay
Thank you for finding related issues.

Is vulnerability in marked resolved?

Yes, i think resolved in 0.3.9.
markedjs/marked#997
Also, since "marked" is being maintained continuously, I think that there is no problem in use.
https://github.com/markedjs/marked/releases

I do not believe HTML will be allowed anyways?

No. It will not be effective unless the environment variable "REDASH_ALLOW_SCRIPTS_IN_USER_INPUT" is set to "true".
Default is false.

If you want to do the same as existing display, change to the following code.
what will you do?

#client/app/filters/markdown.js

import marked from 'marked';

export default function init(ngModule) {
  ngModule.filter('markdown', ($sce, clientConfig) =>
    function parseMarkdown(text) {
      if (!text) {
        return '';
      }

      let html = '';
      if (clientConfig.allowScriptsInUserInput) {
        html = $sce.trustAsHtml(marked(String(text)));
      } else {
        html = marked(String(text), { sanitize: true });
      }

      return html;
    });
}

init.init = true;

@arikfr arikfr added the review label Dec 17, 2018
@arikfr
Copy link
Member

arikfr commented Jan 21, 2020

Hi,

(This is a template message, but I mean every word of it. Also you're welcome to reply)

Thank you for making this contribution. While we couldn't bring it to completion and merge, it's still very much appreciated. 🙇

In the past year the Redash code base gone under massive updates: on the backend we moved to Python 3 & RQ instead of Celery and on the frontend we replaced Angular with React. It's very likely this makes this PR irrelevant without significant changes. :-(

I'm closing this PR now. But if you're still interested in making it happen, let me know and I will reopen.

Thanks.

@arikfr arikfr closed this Jan 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants