2
\$\begingroup\$

I have been using a particular pattern over and over again when I want to keep my jQuery apps more safe from XSS. I do something like this. (It uses ES6 template strings)

function doSomething ($el, user) {
  var $html = $(`
  <div>
    <img src="${user.img}">
    <div class="username-text"></div>
    <div class="description"></div>
  </div>
`);
  $html.appendTo($el);
  $html.find('.username-text').append(document.createTextNode(user.name));
  $html.find('.description').append(document.createTextNode(user.description));

  return $html;
}

var user = {
  name: "John", 
  description: "<script>alert('XSS vulnerability');</script>", 
  img: 'http://placehold.it/100x100'
};

doSomething($('body'), user);

JS Bin

User defined fields become text nodes which means I need to create a jQuery element or some sort of DOM structure to hold them. I also use find which means at least in jQuery I have to append it to the actual DOM itself. So I was thinking it would be nice to have a more general way of nesting jQuery elements (I guess you could say "components") inside the template itself kind of like React's JSX but for jQuery.

This is what I came up with. (It uses a tagged template literal which returns a jQuery object).

var jQTemplate = (function ($) {

  var tempFiller = function (key) {
    return `<div id='jQT-jQTemplate-tempFiller-${key}'></div>`;
  };

  var jQTemplate = function (strings, ...values) {
      var $docFrag = $(document.createDocumentFragment());
      var nodeIndexes = [];
      var html = "";
    for(let i = 0; i < strings.length; ++i) {
      html += strings[i];

      var value = values[i];
      var allNodes = Array.isArray(value) && value.every(el=>el instanceof Node || el instanceof jQuery)
      if(value) {
        if(value instanceof Node || value instanceof jQuery || allNodes) {
          nodeIndexes.push(i);
          html += tempFiller(i);
        } else if(value) {
          html += value;// || '';
        }
      }
    }
    $docFrag.append(html);
    nodeIndexes.forEach(function (i) {
      var value = values[i];
      $($docFrag[0].getElementById(`jQT-jQTemplate-tempFiller-${i}`))
    .replaceWith(value);
    });

      return $docFrag;
    };

  return {
    template: jQTemplate,
    textNode: (text) => document.createTextNode(text)
  };

}($));

Here is some example usage: (not the best "app" but shows the different ways I intended it to be used)

/*
* Example usage
*
*/

var {template, textNode} =  jQTemplate;


console.log("started");

function bootstrapButton (type, text) {
  return $(`<a class="btn btn-${type || "default"} btn-lg" href="#" role="button">${text || ""}</a>`)
}

function jumbotron () {
  var $learnMore = bootstrapButton("primary", "Add Content");

  $learnMore.on('click', function (el) {
    $("body").append('<p>Lorem Ipsum. <i class="close fa fa-times js-deleteParent"></i></p>');
  })

  return template`
<div class="jumbotron">
  <h1>Hello, world!</h1>
  <p>...</p>
  <p>${$learnMore}</p>
</div>
`;
}

$("body").on("click", '.js-deleteParent', function () {
  $(this).parent().remove();
});

var userInput = ['<script>alert("hello world")</script>', "more user input"];

var trustedHtmlContent = [`<h1>Header</h1>`, `<h3>Sub Header</h3>`];

var extraClass ="container";

template`
  <div class="${extraClass}">
    <div>
      ${trustedHtmlContent.map(el=>$(el))}
      ${userInput.map(el=>textNode(el))}
     </div>
     ${jumbotron()}
  </div>
`.appendTo("body");

JS Bin

Any thoughts, suggestions, or comments? Is there any reason why this shouldn't be used in production?

\$\endgroup\$
1
\$\begingroup\$

I was reading through your code and I got to

  if(value) {
    if(value instanceof Node || value instanceof jQuery || allNodes) {
      nodeIndexes.push(i);
      html += tempFiller(i);
    } else if(value) {
      html += value;// || '';
    }
  }

and I had to read over it a few times, but I think that you can do away with the outside if statement or at least not check value a second time.

either one of these look like they are going to do about the same thing

if (value instanceof Node || value instanceof jQuery || allNodes) {
    nodeIndexes.push(i);
    html += tempFiller(i);
} else if (value) {
    html += value;
}

this would go through both checks, much the same way that the original code did, except you don't check the existence of value first, this might cause some errors, so I would probably do this instead

if (value) {
    if (value instanceof Node || value instanceof jQuery || allNodes) {
      nodeIndexes.push(i);
      html += tempFiller(i);
    } else {
      html += value;
    }
}

turned the else if into an else, because we already know that value is going to pass a truthy test, no need to check it a second time.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.