escape thing inside of code tag #2337

Kevan-Y · 2021-10-05T15:02:34Z

Issue This PR Addresses

Fixes #2322

Type of Change

Bugfix: Change which fixes an issue
New Feature: Change which adds functionality
Documentation Update: Change which improves documentation
UI: Change which improves UI

Description

Removed replace-entities.js to fix our current issue but will re-introduce #1091.
Added a regex at end of

telescope/src/backend/utils/html/syntax-highlight.js

Line 58 in bce0b21

code.innerHTML = value;

to turn &gt; back to >

Checklist

Quality: This PR builds and passes our npm test and works locally
Tests: This PR includes thorough tests or an explanation of why it does not
Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not (if applicable)
Documentation: This PR includes updated/added documentation to user exposed functionality or configuration variables are added/changed or an explanation of why it does not(if applicable)

humphd

This is different than what I thought we were discussing. I thought you were going to submit a PR to change the options we pass to the sanitizer so it escapes vs. deletes unknown tags.

I don't think running regexes on the HTML is a good idea.

Kevan-Y · 2021-10-05T15:10:43Z

This is different than what I thought we were discussing. I thought you were going to submit a PR to change the options we pass to the sanitizer so it escapes vs. deletes unknown tags.

I don't think running regexes on the HTML is a good idea.

But adding the escape will convert tag like <code><text></code> to <code><text></text></code> or <code><text/></code> to <code><text></text></code>
By converting < and > to < and >, it escapes and doesn't not change <code><text></code> to <code><text></text></code>

humphd · 2021-10-05T15:41:52Z

I want to understand the underlying problem here vs. fix one specific case. Consider:

<code><text></code>
<code><text></code>

The example in 1. is not valid HTML5, only 2. is. Why are we being dealing with invalid HTML in the first place? Are we being sent it? Or is our feed pipeline producing it somehow?

In the case of your post, I see that we get this: <code>Closed by <short_name_issue_code>.</code>

I think the underlying issue is the fact that the sanitizer is forcing entities to be decoded, see fb55/htmlparser2#105. Undoing this has some interesting security implications.

manekenpix · 2021-10-12T03:51:57Z

@Kevan-Y any updates?

humphd · 2021-10-12T13:17:56Z

I think this is a hard problem. My gut says that we should turn off automatic entity decoding in the sanitizer, which is on by default to catch some edge case bugs. Because we pull content from existing systems vs. ingesting user-defined HTML directly, it's probably safe to accept what comes in and just strip the bits we don't want (e.g., scripts, etc).

humphd · 2021-10-13T12:35:52Z

Let's try an experiment to change our HTML sanitizer to stop encoding automatically:

htmlparser2 Options

sanitize-html is built on htmlparser2. By default the only option passed down is decodeEntities: true. You can set the options to pass by using the parser option.

Kevan-Y · 2021-10-22T15:00:52Z

Update:
After some traceback, I notice that the main issue was

telescope/src/backend/utils/html/replace-entities.js

Lines 5 to 6 in bce0b21

    
           const result = entities.decodeHTML(codeElement.innerHTML); 
        
           return entities.decodeHTML(result);

Looked at the feed information that we get when something is inside of the code tag, it is in escaped character already, but because of the decode HTML. It turns into an actual character. Which produces turning to actual HTML.
By removing replace-entities.js, the current issue will be fixed but this issue #1091 will be reproducible.
Traced back where it is causing

telescope/src/backend/utils/html/syntax-highlight.js

Line 44 in bce0b21

const { value, language } = hljs.highlightAuto(code.innerHTML);

I think when we pass our code.innerHTML to the hljs.highlightAuto() It's changing for example > to &gt;.
So maybe having a regex at the end

telescope/src/backend/utils/html/syntax-highlight.js

Line 58 in bce0b21

code.innerHTML = value;

to change it back should be enough to fix that bug.

humphd · 2021-10-22T15:04:06Z

src/backend/utils/html/syntax-highlight.js

    // Replace the contents with newly marked up syntax highlighting
-    code.innerHTML = value;
+    code.innerHTML = value.replace(/\&amp;([a-z]*);/gm, '&$1');


HTML entities can be a lot more than just a-z. Maybe this should be:

/\&([^;]+);/gm

In this case, it matches 1 or more non-semi-colon characters before a semi-colon.

What happens if & is present on its own in the post? Won't this destroy it?

Changed the regex, added one more condition if the user input &&lg; it will stay the same. The change only apply for thing e.g, &lt; or &gt; ...

humphd · 2021-10-22T15:04:32Z

src/backend/utils/html/syntax-highlight.js

@@ -54,7 +54,8 @@ module.exports = function (dom) {
      pre.classList.add('hljs', language);
    }

+    // Convert &amp; followed by another escape character to a single escape character


This comment needs more details. It's odd that we're doing this, so we should explain why we have chosen to do it, and what it is fixing.

humphd · 2021-10-22T15:39:50Z

src/backend/utils/html/syntax-highlight.js

@@ -53,8 +53,9 @@ module.exports = function (dom) {
    if (pre) {
      pre.classList.add('hljs', language);
    }
-
+    // Value from hljs.highlightAuto turn escape character to e.g &lt; to &amp;lt;
+    // Adding regex to convert &amp; of that escape character back to &


For reference, here is what they do: https://github.com/highlightjs/highlight.js/blob/7c87adcc39fed1754e9c85c2e7f744b560acd24e/src/lib/regex.js#L5-L7

humphd · 2021-10-22T15:40:58Z

src/backend/utils/html/syntax-highlight.js

    // Replace the contents with newly marked up syntax highlighting
-    code.innerHTML = value;
+    code.innerHTML = value.replace(/\&amp;([^;|^&]+);/gm, '&$1');


You've got two cases here: not ; and not &. Can we get test coverage of both?

Also, there are about 8 issues filed on this, which reference various blog posts in the wild that have broken. Can you create some more tests based on those cases? It would be good to know that your code fixes them.

Added some test cases. Some issues filed are duplicated from others so combined into 1 test case.

You've got two cases here: not ; and not &. Can we get test coverage of both?

Not sure if it is possible I can test for like && or ;&lt; to stay the same when calling the whole process but not test &lt; to turn < by calling the whole process since this is happening inside of syntax-highlight.js with value. Like for example if we pass &lt; to hljs.highlightAuto(code.innerHTML); it will turn to &amp;lt; then with the regex it will turned back to &lt;
We can cover that if we create a test case to test this specific regex. But I don't think we need it

humphd · 2021-10-24T15:13:09Z

@Kevan-Y can you rebase and fix the conflict please?

Kevan-Y · 2021-10-24T16:50:49Z

@Kevan-Y can you rebase and fix the conflict please?

Rebased the branch to master, squash commit together to make it looks nicer, and fixed conflict

humphd · 2021-10-24T17:34:12Z

Did you rebase onto the right commit? Looks like you've got issues with removeDevToFullscreen, which I think we ripped out quite a while ago.

Summary of all failing tests
FAIL test/feed.test.js
  ● Post data class tests › Get and Set Feed objects from database › Removing feeds should also remove posts

    ReferenceError: removeDevToFullscreen is not defined

      41 |   lazyLoad(dom);
      42 |   // Deal with "Enter fullscreen mode Exit fullscreen mode" text from dev.to bug
    > 43 |   removeDevToFullscreen(dom);
         |   ^
      44 |
      45 |   // Return the resulting HTML
      46 |   return dom.window.document.body.innerHTML;

      at processHTML (src/backend/utils/html/index.js:43:3)
      at _callee$ (src/backend/data/post.js:134:14)
      at tryCatch (node_modules/regenerator-runtime/runtime.js:63:40)
      at Generator.invoke [as _invoke] (node_modules/regenerator-runtime/runtime.js:294:22)
      at Generator.next (node_modules/regenerator-runtime/runtime.js:119:21)
      at asyncGeneratorStep (node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:24)
      at _next (node_modules/@babel/runtime/helpers/asyncToGenerator.js:25:9)
      at node_modules/@babel/runtime/helpers/asyncToGenerator.js:32:7
      at Function.<anonymous> (node_modules/@babel/runtime/helpers/asyncToGenerator.js:21:12)

FAIL test/post.test.js
  ● Post data class tests › Post.createFromArticle() tests › should work with real world RSS

    ReferenceError: removeDevToFullscreen is not defined

      41 |   lazyLoad(dom);
      42 |   // Deal with "Enter fullscreen mode Exit fullscreen mode" text from dev.to bug
    > 43 |   removeDevToFullscreen(dom);
         |   ^
      44 |
      45 |   // Return the resulting HTML
      46 |   return dom.window.document.body.innerHTML;

      at processHTML (src/backend/utils/html/index.js:43:3)
      at _callee$ (src/backend/data/post.js:134:14)
      at tryCatch (node_modules/regenerator-runtime/runtime.js:63:40)
      at Generator.invoke [as _invoke] (node_modules/regenerator-runtime/runtime.js:294:22)
      at Generator.next (node_modules/regenerator-runtime/runtime.js:119:21)
      at asyncGeneratorStep (node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:24)
      at _next (node_modules/@babel/runtime/helpers/asyncToGenerator.js:25:9)
      at node_modules/@babel/runtime/helpers/asyncToGenerator.js:32:7
      at Function.<anonymous> (node_modules/@babel/runtime/helpers/asyncToGenerator.js:21:12)

  ● Post data class tests › Post.createFromArticle() tests › Post.createFromArticle() with missing date should not throw

    ReferenceError: removeDevToFullscreen is not defined

      41 |   lazyLoad(dom);
      42 |   // Deal with "Enter fullscreen mode Exit fullscreen mode" text from dev.to bug
    > 43 |   removeDevToFullscreen(dom);
         |   ^
      44 |
      45 |   // Return the resulting HTML
      46 |   return dom.window.document.body.innerHTML;

      at processHTML (src/backend/utils/html/index.js:43:3)
      at _callee$ (src/backend/data/post.js:134:14)
      at tryCatch (node_modules/regenerator-runtime/runtime.js:63:40)
      at Generator.invoke [as _invoke] (node_modules/regenerator-runtime/runtime.js:294:22)
      at Generator.next (node_modules/regenerator-runtime/runtime.js:119:21)
      at asyncGeneratorStep (node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:24)
      at _next (node_modules/@babel/runtime/helpers/asyncToGenerator.js:25:9)
      at node_modules/@babel/runtime/helpers/asyncToGenerator.js:32:7
      at Function.<anonymous> (node_modules/@babel/runtime/helpers/asyncToGenerator.js:21:12)

  ● Post data class tests › Post.createFromArticle() tests › Post.createFromArticle() with missing title should use Untitled

    ReferenceError: removeDevToFullscreen is not defined

      41 |   lazyLoad(dom);
      42 |   // Deal with "Enter fullscreen mode Exit fullscreen mode" text from dev.to bug
    > 43 |   removeDevToFullscreen(dom);
         |   ^
      44 |
      45 |   // Return the resulting HTML
      46 |   return dom.window.document.body.innerHTML;

      at processHTML (src/backend/utils/html/index.js:43:3)
      at _callee$ (src/backend/data/post.js:134:14)
      at tryCatch (node_modules/regenerator-runtime/runtime.js:63:40)
      at Generator.invoke [as _invoke] (node_modules/regenerator-runtime/runtime.js:294:22)
      at Generator.next (node_modules/regenerator-runtime/runtime.js:119:21)
      at asyncGeneratorStep (node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:24)
      at _next (node_modules/@babel/runtime/helpers/asyncToGenerator.js:25:9)
      at node_modules/@babel/runtime/helpers/asyncToGenerator.js:32:7
      at Function.<anonymous> (node_modules/@babel/runtime/helpers/asyncToGenerator.js:21:12)

FAIL test/process-html.test.js
  ● Process HTML › Unknown tags within code tag should be stay the same

    ReferenceError: removeDevToFullscreen is not defined

      41 |   lazyLoad(dom);
      42 |   // Deal with "Enter fullscreen mode Exit fullscreen mode" text from dev.to bug
    > 43 |   removeDevToFullscreen(dom);
         |   ^
      44 |
      45 |   // Return the resulting HTML
      46 |   return dom.window.document.body.innerHTML;

      at process (src/backend/utils/html/index.js:43:3)
      at Object.<anonymous> (test/process-html.test.js:7:18)

  ● Process HTML › https://github.com/Seneca-CDOT/telescope/issues/2313

    ReferenceError: removeDevToFullscreen is not defined

      41 |   lazyLoad(dom);
      42 |   // Deal with "Enter fullscreen mode Exit fullscreen mode" text from dev.to bug
    > 43 |   removeDevToFullscreen(dom);
         |   ^
      44 |
      45 |   // Return the resulting HTML
      46 |   return dom.window.document.body.innerHTML;

      at process (src/backend/utils/html/index.js:43:3)
      at Object.<anonymous> (test/process-html.test.js:12:18)

  ● Process HTML › https://github.com/Seneca-CDOT/telescope/issues/2220

    ReferenceError: removeDevToFullscreen is not defined

      41 |   lazyLoad(dom);
      42 |   // Deal with "Enter fullscreen mode Exit fullscreen mode" text from dev.to bug
    > 43 |   removeDevToFullscreen(dom);
         |   ^
      44 |
      45 |   // Return the resulting HTML
      46 |   return dom.window.document.body.innerHTML;

      at process (src/backend/utils/html/index.js:43:3)
      at Object.<anonymous> (test/process-html.test.js:20:18)

  ● Process HTML › Double escape character stay the same

    ReferenceError: removeDevToFullscreen is not defined

      41 |   lazyLoad(dom);
      42 |   // Deal with "Enter fullscreen mode Exit fullscreen mode" text from dev.to bug
    > 43 |   removeDevToFullscreen(dom);
         |   ^
      44 |
      45 |   // Return the resulting HTML
      46 |   return dom.window.document.body.innerHTML;

      at process (src/backend/utils/html/index.js:43:3)
      at Object.<anonymous> (test/process-html.test.js:29:18)

  ● Process HTML › Escape character stay the same

    ReferenceError: removeDevToFullscreen is not defined

      41 |   lazyLoad(dom);
      42 |   // Deal with "Enter fullscreen mode Exit fullscreen mode" text from dev.to bug
    > 43 |   removeDevToFullscreen(dom);
         |   ^
      44 |
      45 |   // Return the resulting HTML
      46 |   return dom.window.document.body.innerHTML;

      at process (src/backend/utils/html/index.js:43:3)
      at Object.<anonymous> (test/process-html.test.js:35:18)

  ● Process HTML › https://github.com/Seneca-CDOT/telescope/issues/1091

    ReferenceError: removeDevToFullscreen is not defined

      41 |   lazyLoad(dom);
      42 |   // Deal with "Enter fullscreen mode Exit fullscreen mode" text from dev.to bug
    > 43 |   removeDevToFullscreen(dom);
         |   ^
      44 |
      45 |   // Return the resulting HTML
      46 |   return dom.window.document.body.innerHTML;

      at process (src/backend/utils/html/index.js:43:3)
      at Object.<anonymous> (test/process-html.test.js:41:18)

Kevan-Y · 2021-10-24T18:19:28Z

Did you rebase onto the right commit? Looks like you've got issues with removeDevToFullscreen, which I think we ripped out quite a while ago.

Summary of all failing tests
FAIL test/feed.test.js
  ● Post data class tests › Get and Set Feed objects from database › Removing feeds should also remove posts

    ReferenceError: removeDevToFullscreen is not defined

      41 |   lazyLoad(dom);
      42 |   // Deal with "Enter fullscreen mode Exit fullscreen mode" text from dev.to bug
    > 43 |   removeDevToFullscreen(dom);
         |   ^
      44 |
      45 |   // Return the resulting HTML
      46 |   return dom.window.document.body.innerHTML;

      at processHTML (src/backend/utils/html/index.js:43:3)
      at _callee$ (src/backend/data/post.js:134:14)
      at tryCatch (node_modules/regenerator-runtime/runtime.js:63:40)
      at Generator.invoke [as _invoke] (node_modules/regenerator-runtime/runtime.js:294:22)
      at Generator.next (node_modules/regenerator-runtime/runtime.js:119:21)
      at asyncGeneratorStep (node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:24)
      at _next (node_modules/@babel/runtime/helpers/asyncToGenerator.js:25:9)
      at node_modules/@babel/runtime/helpers/asyncToGenerator.js:32:7
      at Function.<anonymous> (node_modules/@babel/runtime/helpers/asyncToGenerator.js:21:12)

FAIL test/post.test.js
  ● Post data class tests › Post.createFromArticle() tests › should work with real world RSS

    ReferenceError: removeDevToFullscreen is not defined

      41 |   lazyLoad(dom);
      42 |   // Deal with "Enter fullscreen mode Exit fullscreen mode" text from dev.to bug
    > 43 |   removeDevToFullscreen(dom);
         |   ^
      44 |
      45 |   // Return the resulting HTML
      46 |   return dom.window.document.body.innerHTML;

      at processHTML (src/backend/utils/html/index.js:43:3)
      at _callee$ (src/backend/data/post.js:134:14)
      at tryCatch (node_modules/regenerator-runtime/runtime.js:63:40)
      at Generator.invoke [as _invoke] (node_modules/regenerator-runtime/runtime.js:294:22)
      at Generator.next (node_modules/regenerator-runtime/runtime.js:119:21)
      at asyncGeneratorStep (node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:24)
      at _next (node_modules/@babel/runtime/helpers/asyncToGenerator.js:25:9)
      at node_modules/@babel/runtime/helpers/asyncToGenerator.js:32:7
      at Function.<anonymous> (node_modules/@babel/runtime/helpers/asyncToGenerator.js:21:12)

  ● Post data class tests › Post.createFromArticle() tests › Post.createFromArticle() with missing date should not throw

    ReferenceError: removeDevToFullscreen is not defined

      41 |   lazyLoad(dom);
      42 |   // Deal with "Enter fullscreen mode Exit fullscreen mode" text from dev.to bug
    > 43 |   removeDevToFullscreen(dom);
         |   ^
      44 |
      45 |   // Return the resulting HTML
      46 |   return dom.window.document.body.innerHTML;

      at processHTML (src/backend/utils/html/index.js:43:3)
      at _callee$ (src/backend/data/post.js:134:14)
      at tryCatch (node_modules/regenerator-runtime/runtime.js:63:40)
      at Generator.invoke [as _invoke] (node_modules/regenerator-runtime/runtime.js:294:22)
      at Generator.next (node_modules/regenerator-runtime/runtime.js:119:21)
      at asyncGeneratorStep (node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:24)
      at _next (node_modules/@babel/runtime/helpers/asyncToGenerator.js:25:9)
      at node_modules/@babel/runtime/helpers/asyncToGenerator.js:32:7
      at Function.<anonymous> (node_modules/@babel/runtime/helpers/asyncToGenerator.js:21:12)

  ● Post data class tests › Post.createFromArticle() tests › Post.createFromArticle() with missing title should use Untitled

    ReferenceError: removeDevToFullscreen is not defined

      41 |   lazyLoad(dom);
      42 |   // Deal with "Enter fullscreen mode Exit fullscreen mode" text from dev.to bug
    > 43 |   removeDevToFullscreen(dom);
         |   ^
      44 |
      45 |   // Return the resulting HTML
      46 |   return dom.window.document.body.innerHTML;

      at processHTML (src/backend/utils/html/index.js:43:3)
      at _callee$ (src/backend/data/post.js:134:14)
      at tryCatch (node_modules/regenerator-runtime/runtime.js:63:40)
      at Generator.invoke [as _invoke] (node_modules/regenerator-runtime/runtime.js:294:22)
      at Generator.next (node_modules/regenerator-runtime/runtime.js:119:21)
      at asyncGeneratorStep (node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:24)
      at _next (node_modules/@babel/runtime/helpers/asyncToGenerator.js:25:9)
      at node_modules/@babel/runtime/helpers/asyncToGenerator.js:32:7
      at Function.<anonymous> (node_modules/@babel/runtime/helpers/asyncToGenerator.js:21:12)

FAIL test/process-html.test.js
  ● Process HTML › Unknown tags within code tag should be stay the same

    ReferenceError: removeDevToFullscreen is not defined

      41 |   lazyLoad(dom);
      42 |   // Deal with "Enter fullscreen mode Exit fullscreen mode" text from dev.to bug
    > 43 |   removeDevToFullscreen(dom);
         |   ^
      44 |
      45 |   // Return the resulting HTML
      46 |   return dom.window.document.body.innerHTML;

      at process (src/backend/utils/html/index.js:43:3)
      at Object.<anonymous> (test/process-html.test.js:7:18)

  ● Process HTML › https://github.com/Seneca-CDOT/telescope/issues/2313

    ReferenceError: removeDevToFullscreen is not defined

      41 |   lazyLoad(dom);
      42 |   // Deal with "Enter fullscreen mode Exit fullscreen mode" text from dev.to bug
    > 43 |   removeDevToFullscreen(dom);
         |   ^
      44 |
      45 |   // Return the resulting HTML
      46 |   return dom.window.document.body.innerHTML;

      at process (src/backend/utils/html/index.js:43:3)
      at Object.<anonymous> (test/process-html.test.js:12:18)

  ● Process HTML › https://github.com/Seneca-CDOT/telescope/issues/2220

    ReferenceError: removeDevToFullscreen is not defined

      41 |   lazyLoad(dom);
      42 |   // Deal with "Enter fullscreen mode Exit fullscreen mode" text from dev.to bug
    > 43 |   removeDevToFullscreen(dom);
         |   ^
      44 |
      45 |   // Return the resulting HTML
      46 |   return dom.window.document.body.innerHTML;

      at process (src/backend/utils/html/index.js:43:3)
      at Object.<anonymous> (test/process-html.test.js:20:18)

  ● Process HTML › Double escape character stay the same

    ReferenceError: removeDevToFullscreen is not defined

      41 |   lazyLoad(dom);
      42 |   // Deal with "Enter fullscreen mode Exit fullscreen mode" text from dev.to bug
    > 43 |   removeDevToFullscreen(dom);
         |   ^
      44 |
      45 |   // Return the resulting HTML
      46 |   return dom.window.document.body.innerHTML;

      at process (src/backend/utils/html/index.js:43:3)
      at Object.<anonymous> (test/process-html.test.js:29:18)

  ● Process HTML › Escape character stay the same

    ReferenceError: removeDevToFullscreen is not defined

      41 |   lazyLoad(dom);
      42 |   // Deal with "Enter fullscreen mode Exit fullscreen mode" text from dev.to bug
    > 43 |   removeDevToFullscreen(dom);
         |   ^
      44 |
      45 |   // Return the resulting HTML
      46 |   return dom.window.document.body.innerHTML;

      at process (src/backend/utils/html/index.js:43:3)
      at Object.<anonymous> (test/process-html.test.js:35:18)

  ● Process HTML › https://github.com/Seneca-CDOT/telescope/issues/1091

    ReferenceError: removeDevToFullscreen is not defined

      41 |   lazyLoad(dom);
      42 |   // Deal with "Enter fullscreen mode Exit fullscreen mode" text from dev.to bug
    > 43 |   removeDevToFullscreen(dom);
         |   ^
      44 |
      45 |   // Return the resulting HTML
      46 |   return dom.window.document.body.innerHTML;

      at process (src/backend/utils/html/index.js:43:3)
      at Object.<anonymous> (test/process-html.test.js:41:18)

Sorry about that. I fixed and ran the test. All should be good.

humphd

I'm tempted to try this and see what happens, but I'd like to think on it a bit more.

humphd

I'm going to approve this and we'll test/fix based on what we see on staging.

Grommers00 · 2021-10-27T22:27:45Z

Okay, so, after reading the original issue, the point of this PR is to fix up why things in a < code > tag were not previously loading in. And part of the problem before was our sanitization.

The fix is to change some of the 'problematic' values which occur inparticular & > <?

humphd · 2021-10-28T00:21:10Z

@Kevan-Y can give more details

Kevan-Y · 2021-10-28T00:53:39Z

@manekenpix The issue was our current sanitization decode escape character inside of a < code > tag. Those escape characters will turn into actual characters, so when we render the page, it will convert into an actual HTML tag.

The issue came from a code fix introduced here:

telescope/src/backend/utils/html/replace-entities.js

Lines 5 to 6 in bce0b21

    
           const result = entities.decodeHTML(codeElement.innerHTML); 
        
           return entities.decodeHTML(result);

This was to fix this issue #1091

So if we remove replace-entitie.js. It will technically fix our current issue. But will re-introduce this bug #1091. So I used another way to fix this (#1091) bug.
#1091 bug was that passing code.innerHTML to hljs.highlightAuto() is changing for example > to &gt;.
So to fix it having a regex at the end of

telescope/src/backend/utils/html/syntax-highlight.js

Line 58 in bce0b21

code.innerHTML = value;

to change it back should be enough to fix that bug.

Grommers00 · 2021-10-28T02:40:16Z

Okay awesome.
I think we can see if run into problems down the line and re-address them as we move forward.

Grommers00 · 2021-10-29T20:02:45Z

If you can just rebase it onto master. So we can merge it should be good to go :).

humphd · 2021-10-30T01:50:56Z

OK, let's try this. @Kevan-Y we'll open further issues to tweak it when we see what happens on staging.

vercel bot deployed to Preview October 5, 2021 15:02 View deployment

humphd requested changes Oct 5, 2021

View reviewed changes

Kevan-Y requested a review from humphd October 5, 2021 15:19

manekenpix assigned Kevan-Y Oct 10, 2021

vercel bot deployed to Preview October 22, 2021 14:52 View deployment

humphd requested changes Oct 22, 2021

View reviewed changes

vercel bot deployed to Preview October 22, 2021 15:24 View deployment

Kevan-Y requested a review from humphd October 22, 2021 15:27

humphd requested changes Oct 22, 2021

View reviewed changes

vercel bot deployed to Preview October 22, 2021 17:16 View deployment

humphd requested a review from manekenpix October 22, 2021 17:27

humphd added the hacktoberfest-accepted label Oct 24, 2021

humphd marked this pull request as ready for review October 24, 2021 15:12

humphd self-requested a review October 24, 2021 15:13

Kevan-Y force-pushed the issue/issue-2322 branch from bde3c64 to 6e6be2f Compare October 24, 2021 16:39

vercel bot deployed to Preview October 24, 2021 16:39 View deployment

Kevan-Y force-pushed the issue/issue-2322 branch from 6e6be2f to 6ba81f6 Compare October 24, 2021 16:49

vercel bot deployed to Preview October 24, 2021 16:49 View deployment

Fix unknown tags within code tag

70ee609

Kevan-Y force-pushed the issue/issue-2322 branch from 6ba81f6 to 70ee609 Compare October 24, 2021 18:19

vercel bot deployed to Preview October 24, 2021 18:19 View deployment

humphd reviewed Oct 24, 2021

View reviewed changes

humphd approved these changes Oct 26, 2021

View reviewed changes

humphd requested review from cindyorangis and Grommers00 October 27, 2021 18:30

Grommers00 approved these changes Oct 28, 2021

View reviewed changes

humphd merged commit 8639cbe into Seneca-CDOT:master Oct 30, 2021

Kevan-Y mentioned this pull request Nov 9, 2021

[BUG] - && not displaying correctly in post #2457

Closed

This was referenced Nov 22, 2021

Code escaping/highlighting bug #1801

Closed

HTML encoded text in posts being removed/rendered incorrectly #2131

Closed

Syntax highlighting issues with code blocks #2313

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

escape thing inside of code tag #2337

escape thing inside of code tag #2337

Kevan-Y commented Oct 5, 2021 •

edited

Loading

humphd left a comment

Kevan-Y commented Oct 5, 2021 •

edited

Loading

humphd commented Oct 5, 2021

manekenpix commented Oct 12, 2021

humphd commented Oct 12, 2021

humphd commented Oct 13, 2021

Kevan-Y commented Oct 22, 2021

humphd Oct 22, 2021

humphd Oct 22, 2021

Kevan-Y Oct 22, 2021 •

edited

Loading

humphd Oct 22, 2021

Kevan-Y Oct 22, 2021

humphd Oct 22, 2021

humphd Oct 22, 2021

Kevan-Y Oct 22, 2021

humphd commented Oct 24, 2021

Kevan-Y commented Oct 24, 2021

humphd commented Oct 24, 2021

Kevan-Y commented Oct 24, 2021

humphd left a comment

humphd left a comment

Grommers00 commented Oct 27, 2021 •

edited

Loading

humphd commented Oct 28, 2021

Kevan-Y commented Oct 28, 2021

Grommers00 commented Oct 28, 2021

Grommers00 commented Oct 29, 2021

humphd commented Oct 30, 2021

escape thing inside of code tag #2337

escape thing inside of code tag #2337

Conversation

Kevan-Y commented Oct 5, 2021 • edited Loading

Issue This PR Addresses

Type of Change

Description

Checklist

humphd left a comment

Choose a reason for hiding this comment

Kevan-Y commented Oct 5, 2021 • edited Loading

humphd commented Oct 5, 2021

manekenpix commented Oct 12, 2021

humphd commented Oct 12, 2021

humphd commented Oct 13, 2021

Kevan-Y commented Oct 22, 2021

humphd Oct 22, 2021

Choose a reason for hiding this comment

humphd Oct 22, 2021

Choose a reason for hiding this comment

Kevan-Y Oct 22, 2021 • edited Loading

Choose a reason for hiding this comment

humphd Oct 22, 2021

Choose a reason for hiding this comment

Kevan-Y Oct 22, 2021

Choose a reason for hiding this comment

humphd Oct 22, 2021

Choose a reason for hiding this comment

humphd Oct 22, 2021

Choose a reason for hiding this comment

Kevan-Y Oct 22, 2021

Choose a reason for hiding this comment

humphd commented Oct 24, 2021

Kevan-Y commented Oct 24, 2021

humphd commented Oct 24, 2021

Kevan-Y commented Oct 24, 2021

humphd left a comment

Choose a reason for hiding this comment

humphd left a comment

Choose a reason for hiding this comment

Grommers00 commented Oct 27, 2021 • edited Loading

humphd commented Oct 28, 2021

Kevan-Y commented Oct 28, 2021

Grommers00 commented Oct 28, 2021

Grommers00 commented Oct 29, 2021

humphd commented Oct 30, 2021

Kevan-Y commented Oct 5, 2021 •

edited

Loading

Kevan-Y commented Oct 5, 2021 •

edited

Loading

Kevan-Y Oct 22, 2021 •

edited

Loading

Grommers00 commented Oct 27, 2021 •

edited

Loading