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

Fix markdown blockquote preview difference #1245

Merged
merged 4 commits into from
Feb 21, 2024

Conversation

nknguyenhc
Copy link
Contributor

Summary:

Fixes #1214

Changes Made:

To elaborate on the issue, the preview blockquote does not render if there is any HTML element in the source string. This is because in that case, all closing bracket > are changed to >.

To fix this, I remove the sanitation by DOMPurify. As mentioned in #1214, ngx-markdown sanitises content by default, hence there is no need to sanitise again using DOMPurify. Furthermore, DOMPurify does not consider markdown syntaxes in sanitation.

I have checked out the examples given by DOMPurify, malicious content are also removed with the sanitation from ngx-markdown. The following content:

> quote stops working in preview???

> quote with &lt;br/&gt; <br/> 
> new quote

> another quote like `this`

<div>For example</div>

<script>console.log("hello")</script>

Hello, test <code>D<sub>SA</sub></code>

<p>abc<iframe src="https://www.youtube.com/embed/sUtJHqzU9dc" allowfullscreen="" title="Youtube intro video"></iframe></p>

<TABLE><tr><td>HELLO</tr></TABL>

<UL><li><A HREF=//google.com>click</UL>

is rendered as

image

Notice that the script and the iframe tags are removed from the rendered HTML, and the broken tags are properly rendered.

This closely matches the rendering on Github:

image

The only difference is that the sanitised strings are rendered as literal strings on Github, while removed in CATcher. However, given the purpose of CATcher, I believe there is no need for rendering malicious HTML elements as literal strings.

Proposed Commit Message:

Fix markdown blockquote preview difference

Due to DOMPurify, the content used for preview is different.
However, given that ngx-markdown already has sufficient sanitation
by default, we remove sanitation by DOMPurify.

Copy link
Contributor

@cheehongw cheehongw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Can you check if the DOMPurify library is used anywhere else? If not, it should be safe to remove it from pacakge.json as well.

@nknguyenhc
Copy link
Contributor Author

I have checked, DOMPurify is no longer used. I have removed it from package.json.

@nknguyenhc nknguyenhc merged commit 944a424 into CATcher-org:master Feb 21, 2024
4 of 5 checks passed
vigneshsankariyer1234567890 added a commit that referenced this pull request Mar 4, 2024
* Fix broken duplicate link (#1233)

Fix the broken link of a duplicate issue

Currently, the user cannot open the link to a duplicate issue
when opening an issue, as described in #1228.

The links now work as expected.

* Add whitespace validation (#1237)

* Add whitespace validation

* Update whitespace validation for new issue

* Update whitespace validation for title of new issues

* Update whitespace validation for title of new issues

* Move validators into core

* Update import order

---------

Co-authored-by: Misra Aditya <e1096355@u.nus.edu>

* Fix uncaught errors when attempting to access an invalid route

There is an uncaught error when the users click on an invalid internal link in Markdown or enter an invalid link in browser.

Internal links are unlikely to be used for bug reporting and are more likely to be invalid.

Let's show an error toaster and stop the navigation when clicking on an internal link in Markdown. Also, redirect the users to the login page if the users enter invalid link in browser.

* Set default branch to `main`

Previously, image uploads depend on the user's default branch.
Now, we set the branch for image upload to be `main`. Images will 
be uploaded to `main` as a result.

---------

Co-authored-by: Chee Hong <c.h.wong2606@gmail.com>

* Preserve linebreaks (#1241)

With preserving linebreaks, subset list items are rendered as a list,
and paragraph rendering is the same as Github.

* Faulty list view when back navigating (#1243)

Issue table settings such as page index are not 
saved when table is re-mounted.

This behavior inconveniences users as their settings 
are reset everytime they navigate to a specific issue and back.

Let's lift up the table settings of each mounted table to 
a service which the tables pull from when mounted.

* Upgrade to Angular 12 (#1242)

Some of our packages are old and outdated. We should actively maintain 
and keep these packages up-to-date so it is easier to maintain in the 
future.

Let's upgrade to Angular 12 to keep our packages up-to-date.

* Fix markdown blockquote preview difference (#1245)

Due to DOMPurify, the content used for preview is different.
However, given that ngx-markdown already has sufficient sanitation
by default, we remove sanitation by DOMPurify.

* Create release for v3.5.3

---------

Co-authored-by: Nguyen <87511888+nknguyenhc@users.noreply.github.com>
Co-authored-by: AdityaMisra <114080910+MadLamprey@users.noreply.github.com>
Co-authored-by: Misra Aditya <e1096355@u.nus.edu>
Co-authored-by: NereusWB922 <107099783+NereusWB922@users.noreply.github.com>
Co-authored-by: Chee Hong <c.h.wong2606@gmail.com>
Co-authored-by: Arif Khalid <88131400+Arif-Khalid@users.noreply.github.com>
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.

Markdown preview and actual content may differ
2 participants