Question concerning practical alternatives to innerHTML and insertAdjacent HTML & passing AMO review


(Gary) #1

I suspect that this is a rather novice question but have been struggling to find useful information on it.

The use of innerHTML is not permitted in an extension according to the pink warning box just above this example. https://developer.mozilla.org/en-US/docs/Web/API/Element/innerHTML#Example.

Is the same true of insertAdjacentHTML? I read one stackoverflow contributor’s comment that they received a “warning” to be sure to sanitize any user input before inserting it but couldn’t tell if that meant the extension failed to pass review.

I don’t want to use jQuery in the extension but, if one were to do so, would $(element).html(string) pass the AMO review?

If neither passes review, are there alternatives? For example, if one wants to capture text in a contenteditable division and then display it as html in another division within the page–similar to what happens here as questions are typed–can it be done without innerHTML or insertAdjacentHTML? ’

I realize that each individual element can be created, text content inserted into it, and appended to a fragment and then the fragment inserted into the DOM, but that seems rather cumbersome for the above scenario.

Also, is it possible to import a document node and its descendents from a background page to a web page, apart from using innerHTML? It appears that it cannot be passed as an object because the result of JSON.stringify is empty. Is there another way?

Thank you.


(Martin Giger) #2

You can use things like innerHTML or insertAdjacentHTML if it’s clear that the value you set is safe. So either it’s a literal in your code or it is sanitized by a means that is trusted by AMO. There’s some lib that’s usually suggested around this and you may find it by searching this forum.

So in short, you can use string serialized HTML if you must, but you have to prove that it is safe to be inserted by either sanitizing it or by having it included directly in your JS.


(Gary) #3

Thank you for responding to my question. I’m glad to learn that.

Most of the string serialized HTML is in the JS but there is some user input that is sanitized before being inserted. When I first started learning about the need to sanitize user input, I came across this simple approach on a web site called http://shebang.brandonmintern.com/foolproof-html-escaping-in-javascript/. It’s from 2012 but it still seems to work well since it relies on the browser to determine what needs to be escaped; but I know very little about this area.

function sanitize(input_str)
{
var e = document.createElement(“div”);
e.appendChild( document.createTextNode(input_str) );
return e.innerHTML;
}

After sanitizing the input, the string is examined and only a few pre-defined custom “tags” are “unescaped” for styling purposes only. Accepting user input through contenteditable divisions only and getting the text content only appears to give the same sanitized string before unescaping the selected custom tags.

I’ll search for that library for something previously trusted by AMO nonetheless.

Do you know if it is true, though, that the occurrence of innerHTML or insertAdjacentHTML in any of the extension code will cause the automatic review to fail.

Thank you.


(Martin Giger) #4

If you use that method to sanitize you may as well use e.textContent, which does the same as adding a textNode as a child. Essentially, that would let you avoid using innerHTML etc. over all.

Occurences of those things don’t auto fail the review, but it’s very likely to be rejected, still.


(Gary) #5

Ok, thank you.

When we type questions and responses into the box in this discourse area and use what I think is called the tilde character on both ends of e.textContent, to get e.textContent in the rendered window, is there any other way to accomplish such a task without using innerHTML to insert an adjusted version of the typed text as string serialized HTML?

I’m just trying to understand if there is anyway in which I can code a similar feature into an extension and not have it be rejected for that alone.


(Martin Giger) #6

The easiest solution is to not use innerHTML and use textContent instead. It ensures that the string you set is rendered as text to the user and not parsed as HTML. This is actually the recommended way to set content in extensions (and websites too, to avoid XSS and all that). Since you usually don’t want to actually do HTML parsing on whatever you assign as the content of an element. See https://developer.mozilla.org/en-US/docs/Web/API/Node/textContent

The code tags in discord, as you can see in the source also use <code> html tags, though you should only use these when semantically accurate and still properly escape anything you put as contents.

There is also https://developer.mozilla.org/en-US/docs/Web/HTML/Element/pre which displays text between the two tags completely unaltered, but if you set arbitrary contents you could still build a payload to escape from the <pre> sandbox. So use with extreme care.


(Gary) #7

Ok thank you.

I’m probably not getting something here because I don’t see how textContent can accomplish what happens on this web site.

If I type <b>bold</b> in one box on this page, the other box renders bold.

Can that take place using textContent alone?

I undersatnd that I can get the text from the first box usingtextContent and then sanitize that string, but when I want to render it with a permissible bold style, isn’t innerHTML necessary to get that sanitized HTML string into the second box?


(Martin Giger) #8

Ah, you want to do formatting of HTML. Well, you’d have to ensure that you are the one inserting the formatting. And ensure what you insert in between the formatting is sanitized. Howe you do the formatting is up to you, if you manually construct the dom tree and use textContent or use your sanitization method etc. (also, in HTML5 you should use <strong> for bold text, or CSS).


(Gary) #9

Thanks. I think I understand now.

I likely have to manually build each element in the DOM tree and set its text using textContent.

If I attempt to use a sanitized string of HTML and innerHTML or insertAdjacentHTML to insert HTML into a DOM element, which would be quicker and possibly more efficient, the extension will likely be rejected unless I can demonstrate a trusted method of ensuring safe HTML.

So, it is unlikely that I could have a similar feature in an extension as takes place here between the box we type in and the box that renders the typed text because there’s no way to get the typed text into the rendering box without using one of those two undesired methods/properties.

Thanks.


(jscher2000) #10

If you are building regularly structured output, you could consider an HTML template:

This is a messy example, but you can see that the amount of code in these forms would make for cumbersome and difficult to maintain insertAdjacentHTML lines:

Templates for different forms: https://github.com/jscher2000/Content-Type-Fixer-extension/blob/master/logaddnew.html#L27

Script that clones the template, populates in various data, and adds it to the document: https://github.com/jscher2000/Content-Type-Fixer-extension/blob/master/logaddnew.js#L67


(Gary) #11

Thank you for the new information. I was unaware of the template tag.

I’m glad to learn about it because it may likely make some of my other work easier. However, I don’t understand how it can be used to render an HTML string of sanitized user input without somewhere involving innerHTML or insertAdjacentHTML.

I noticed that in the logaddnew.js script in a few lines above line 67, there are occurrences of insertAdjacentHTML. I assume your extension passed review, but I didn’t observe any user input being accepted there.

In building my test extension, in a contenteditable division, user input is accepted and the user can click a button to toggle between text and their text rendered as HTML. That button click retrieves the textContent of the division, sanitizes it, changes the division to not be editable, and then uses insertAdjacentHTML to insert the sanitized HTML string.

This, of course, happens quickly. But if, instead, each segment of the text had to be broken down into either a text node or a DOM element and then set the textContent of each element, assemble all of that in a document fragment to replace what was in the division upon each toggle between text and rendered HTML, that would be rather inefficient. It would have to be done with each click because the user would be adjusting the text and number and types of DOM elements.

I thought, perhaps, I was missing something since it appears that the same(using innerHTML of insertAdjacentHTML) must be taking place between the two boxes on this site; and that isn’t considered a risk to the extent it is not used.

Thank you.


(jscher2000) #12

Oh, I didn’t realize the user controlled the HTML tags. In that case, I don’t think templates are relevant.


(Gary) #13

Maybe not for this specifc part but the information you provided was helpful and I found a use for it. It seems that I pick up valuable pieces of information that I didn’t know were available that improve code I thought I was done with, while trying to figure out a nearly unrelated coding issue. So, I’m glad to learn about it. Thanks.


#14

Have you thought about parsing the HTML into a DOM and then just removing all of the elements you don’t allow?


(Gary) #15

Thank you for the suggestion. I don’t understand what you mean by “parse.”

My concern was that any extension that uses innerHTML or insertAdjacentHTML would be rejected. How else can sanitized user input ever be inserted back into the DOM as HTML without using either of these two statments?

Thank you.


#16

Something like

let htmlString = "..."
let parser = new DOMParser;
let html = parser.parseFromString(htmlString, "text/html"); // returns a DOM

From there

let scriptEls = html.querySelectorAll('script');
for(let script of scriptEls) {
    script.remove();
}

(Gary) #17

Thank you for the explanation/example. I’m a novice and did not know about the DOMparser.

I suspect knowing this will be quite useful in the future, in terms of cleaning/sanitizing input.

For the scenario of simply accepting typed text, including some styling tags, from a user in a contenteditable division, how would this work? ( These user-typed “tags” don’t even have to be true HTML tags, just something that the extension code will convert to HTML tags if acceptable.)

  1. Retrieve the textContent from the contenteditable division.

  2. Parse into a DOM by first wrapping the user text inside a complete HTML document string, and then clean further if necessary, as your example removed all script elements.

  3. Remove the desired element in the DOM created from the user’s text and insert it into the web page’s DOM using importNode() or insertAdjacentElement() to avoid using insertAdjacentHTML() or innerHTML().

Is this the correct approach?

It is very interesting and I am glad to learn of it. However, it seems like a lot of work for the browser for my specific scenario, especially when I am taking the user text and inserting into it only the tags I allow. But if innerHTML and insertAdjacentHTML won’t be accepted, this appears to be an alternative.

Thank you.


#18

Maybe try using a library

Really just kind of depends on what you’re doing. Since you’re getting back a DOM it’ll have a body that has the elements that were parsed at the top level, you can just loop through that DOM body’s childNodes.

There are various ways to go about this. It really depends on the complexity that you’re going for. You may simply want to look into using something like CKEditor or TinyMCE if you’re going for a WYSIWYG editor.


(Gary) #19

Thank you for the information and links. I’d like areas of the extension to work offline; so, I don’t think the editors will work for that, but it is interesting to see that someone has built that sort of thing.