React: dangerouslySetInnerHTML should NOT remove script tag

[rajaraodv]

Do you want to request a feature or report a bug?
Bug
What is the current behavior?
There are many cases in the real-world where we need to inject external 3rd party script. For example, adding google-analytics code, adding stripe button and so on. Currently dangerouslySetInnerHTML removes the script tag making it hard to simply inject JS code to the page.
Because of this issue, people are writing bloated components like react-ga (google analytics) that's 12KB(minimized) and all it does it to add a script tag! There are other similar libs like: react-scripts and stripe-checkout (8KB minimized to add the script).

I believe that at least when using dangerouslySetInnerHTML, we should not remove script tag and instead run the script.

For example, adding stripe's button is as follows.
https://stripe.com/docs/checkout/tutorial#embedding

<form action="/your-server-side-code" method="POST">
  <script
    src="https://checkout.stripe.com/checkout.js" class="stripe-button"
    data-key="pk_test_5qV78InO5XtnYvFRZ2VKnIjy"
    data-amount="999"
    data-name="Demo Site"
    data-description="Widget"
    data-image="https://stripe.com/img/documentation/checkout/marketplace.png"
    data-locale="auto">
  </script>
</form>

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://jsfiddle.net or similar (template: https://jsfiddle.net/reactjs/69z2wepo/).

var StripeCheckout = React.createClass({
  createMarkup: function() {
    return {__html: 
      `Below script should create a Stripe button:
      <form action="/your-server-side-code" method="POST">
        <script
          src="https://checkout.stripe.com/checkout.js" class="stripe-button"
          data-key="pk_test_5qV78InO5XtnYvFRZ2VKnIjy"
          data-amount="999"
          data-name="Demo Site"
          data-description="Widget"
          data-image="https://stripe.com/img/documentation/checkout/marketplace.png"
          data-locale="auto">
        </script>
      </form>`
    };
  },
  render: function() {
    return  <span dangerouslySetInnerHTML={this.createMarkup()} />;
  }
})

ReactDOM.render(
  <StripeCheckout />,
  document.getElementById('container')
);

What is the expected behavior?
It should display a "Stripe" button button.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
15.4.2. I believe that this is how React worked for a long time.

[niole]

A workaround could be to make a script DOM element and append it to the appropriate parent. This stack overflow question also seems to have some answers:
http://stackoverflow.com/questions/35614809/react-script-tag-not-working-when-inserted-using-dangerouslysetinnerhtml

[rajaraodv]

I think that dangerouslySetInnerHTML should NOT strip script tag. When we are using dangerouslySetInnerHTML, we know that it could be 'dangerous'. Because of this issue, people are writing bloated components like react-ga (google analytics) that's 10KB(minimized) and all it does it to add a script tag! There are other similar libs like: react-scripts andstripe-checkout
CC @gaearon

[abhirathore2006]

I use a very basic component to load scripts, you can customize it to load any script you wish for.
it gives you a callback, so you can continue doing stuff once script get loaded.

    export interface mapWrapperProps{
      asyncScriptOnLoad?:()=>void;
      libraries?:string;
    }

    class MapWrapper extends React.Component<mapWrapperProps,{}>{

      componentWillMount(){
         let scriptlibraries = this.props.libraries?"libraries="+this.props.libraries:"";
         this.loadScript("https://maps.googleapis.com/maps/api/js?key=apikey&" + scriptlibraries, this.props.asyncScriptOnLoad);
      }

      loadScript(src,callback){
        let len = $('script').filter(function () {
                    return ($(this).attr('src') == src);
                  }).length;
        if(len){
          if(callback)callback();
        }else { 
          var script = document.createElement("script");
          script.type = "text/javascript";
          if(callback)script.onload=callback;
          document.getElementsByTagName("head")[0].appendChild(script);
          script.src = src;
        }
      }
      render(){
        return (<div></div>);
      }
    }
    export default MapWrapper;

[brigand]

I don't think this is a React issue, it's just how setting innerHTML works.

[jakearchibald]

createContextualFragment will run scripts http://jsbin.com/pajubuz/edit?js,output.

I don't know how many people want this, but React could add dangerouslySetHtmlContent which used createContextualFragment rather than innerHTML.

[nhunzaker]

Nice! Sorry, I kept meaning to respond here. DOM parsing/serialization is really neat stuff!

Still, I'm not sure how much steam there would be behind adding something like that, but I don't want to kill anyone's enthusiasm! If anyone is particularly interested in making a case for it, React now has a formal RFC repo. Personally, I'd want to know:

1. What advantages exist for a dedicated property over using createContextualFragment in a lifecycle hook?
2. Is this safe? How do we help users avoid injecting "live" script tags unintentionally?
3. Does this behave consistently across every browser, is the behavior intentional, and is IE11+ support okay?

[webdeveloperpr]

Running into issues with draft-js and dangerouslySetInnerHTML property. I need to embed tweets and script tags need to run in order to do that. A way around make it work is to use an image's onload property to run scripts.

<img
  style="display: none;"
  class="load-script-img"
  src="http://placehold.it/1x1"
  onload="(function() {
    var script = document.createElement('script');
      script.setAttribute('src', 'https://platform.twitter.com/widgets.js');
      script.setAttribute('charset', 'utf-8');
      script.setAttribute('async', 'true');
      document.body.append(script);
  })()"
>
 <!--  Tweeter embed  -->
${html}
/>

I know it's not the best solution but is a way to go around the limitations.

[rickschott]

This feature is needed. If you have encapsulated html code with script you have to separate it out. In a pure React environment it's not normally needed, but in a large enterprise setting you don't always have control over the remote content you want to use. JQuery does this easily and I can't believe more people don't complain about this.

[sonhanguyen]

what does this have anything to do with react though, why should react have code to strip script from what's already expected to be a "dangerous" (hence should behave like a normal) html string? There are plenty of ways xss can be performed without a script tag if that's the intention.

[christo-pr]

I created a React component that executes all the js code that it finds on the html string, check it out, it might help you:

https://www.npmjs.com/package/dangerously-set-html-content

[nightpool]

@nhunzaker sorry to necro, but just found this thread today, and I think this is absolutely a really useful change for React to implement

What advantages exist for a dedicated property over using createContextualFragment in a lifecycle hook?

Getting the lifecycle hook right is very hard. The one @christo-pr linked doesn't work properly when the html prop can change, it doesn't work when server-side rendering, and it doesn't work for tests. If you add html to the dependency of the useEffect hook, then you also need to make sure to clear out all of the children that are already there, otherwise append will "double" your content every time the html changes. These are all problems that React is uniquely positioned to solve—it knows about server side rendering, it can clone the parent DOM node whenever the children need to change, etc etc.

Is this safe? How do we help users avoid injecting "live" script tags unintentionally?

It is no less safe then the existing dangerouslySetInnerHTML. It is trivial for an attacker to get around the "safety" feature of not executing script tags (for example, add an onload attribute to an img), the benefit is in allowing

Here's a concrete use-case. I'm writing a CMS, and I want to allow my users to embed rich content from a variety of providers. Twitter, like many providers, uses a combination of semantic HTML + a trusted <script> to progressively enhance the display. Here's an example of Twitter's supported embed format:

<blockquote class="twitter-tweet">
  <p lang="en" dir="ltr">
    Sunsets don&#39;t get much better than this one over
    <a href="https://twitter.com/GrandTetonNPS?ref_src=twsrc%5Etfw">@GrandTetonNPS</a>.
    <a href="https://twitter.com/hashtag/nature?src=hash&amp;ref_src=twsrc%5Etfw">#nature</a>
    <a href="https://twitter.com/hashtag/sunset?src=hash&amp;ref_src=twsrc%5Etfw">#sunset</a>
    <a href="http://t.co/YuKy2rcjyU">pic.twitter.com/YuKy2rcjyU</a>
  </p>&mdash; US Department of the Interior (@Interior)
  <a href="https://twitter.com/Interior/status/463440424141459456?ref_src=twsrc%5Etfw">May 5, 2014</a>
</blockquote>
<script async src="https://platform.twitter.com/widgets.js" charset="utf-8"></script>

If I naively used setDangerousInnerHTML to render this embed code, it wouldn't work. Embedding trusted rich context like this seems like a pretty common use-case, and it's really frustrating to have to hack around in weird DOM internals to get it to work.

Does this behave consistently across every browser, is the behavior intentional, and is IE11+ support okay?

Yes, this is an intentional interface exposed by the DOM Parsing spec, which specifically calls out that scripts should be executed when using it (step 4, here). I can't speak to the specifics of IE support, but the fact that createContextualFragments respects script tags is both per-spec and consistent across modern browsers.

(some details for those that are curious: innerHTML has legacy behavior with respect to scripts not for safety reasons, but to maintain compatibility with a once-common pattern of updating innerHTML in place, like so: element.innerHTML += '<h2>a new element</h2>'. For that reason, scripts inserted using innerHTML are marked as "already executed". Nodes created using createDocumentFragment, on the other hand, should be basically identical to parsing the HTML normally. https://lists.whatwg.org/pipermail/whatwg-whatwg.org/2010-November/071276.html is a whatwg thread from 2010 lays out the whole situation, including several statements from browser vendors and spec authors that this is intentional behavior.)

[nightpool]

Frankly, as a user this is surprising enough behavior that I would consider it a bug in dangerouslySetInnerHTML, but I can also see the argument for making React's concepts as close to the DOM's concepts as possible, and implementing something new like dangerouslySetHtmlContent as the "recommended" way of adding trusted HTML instead.

[StevenBarquet]

I created a React component that executes all the js code that it finds on the html string, check it out, it might help you:

https://www.npmjs.com/package/dangerously-set-html-content

It worked awesome for me!