Change div tags to script tags and put viewContext/props inside

[martyphee]

this is exacerbated because HTML encoded JSON is very bloated as " -> &quot when rendered into an HTML attribute and there are a great deal of quote marks in JSON!

On pages with large amounts of data the time to usability is dramatically increased on lower bandwidth connections as megabytes of html encoded JSON needs to be downloaded before the HTML that displays the page can be retrieved.

[martyphee]

I'm working on a PR right now to address this.

[justin808]

Turbo links is the tough one if we do that. Script tag is how I originally had it. Other issue is escaping the data.

[martyphee]

What issues does this cause with TurboLinks?

If the data is put into the script tag as proper JSON what issue's might we still have?

[justin808]

The page scripts don't run with TurboLinks, without some special configuration, and I'm not even sure on that.

So TurboLinks will just insert the HTML tags on the existing page (so CSS and JS are not reloaded), and we have a hook from TurboLinks to parse the special dom elements and kick start the React parts. See https://github.com/turbolinks/turbolinks#working-with-script-elements

[martyphee]

According to the link

Turbolinks evaluates script elements in a page’s each time it renders the page.

So that should be ok. The JSON on one of our largest menu pages is about 1mb as a string (JSON.stringify). We still have a number of people who access the menu on mobile devices instead our our apps so we need to make sure we can keep the page size as small as possible.

[justin808]

Let's try it out! It's possible that the older turbo links didn't.

[martyphee]

I'll try to do some work on this over the weekend. I'm headed back to
London Sunday and probably won't have much time during the week, but I'll
try to fit some work in on it.

On Wed, May 4, 2016 at 5:34 PM, Justin Gordon [email protected]
wrote:

Let's try it out! It's possible that the older turbo links didn't.

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#412 (comment)

[justin808]

@martyphee Any info on this? Be particularly careful with escaping issues. I.e., suppose you have application data sent in the JavasScript, and it can close the script tag and open a new one.

[martyphee]

I'll try to take a look at this today.

On Fri, May 20, 2016 at 8:05 PM, Justin Gordon [email protected]
wrote:

@martyphee https://github.com/martyphee Any info on this? Be
particularly careful with escaping issues. I.e., suppose you have
application data sent in the JavasScript, and it can close the script tag
and open a new one.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#412 (comment)

[martyphee]

I got some work done on it, but haven't finished.

On Sat, May 21, 2016 at 11:58 AM, Martin Phee [email protected] wrote:

I'll try to take a look at this today.

On Fri, May 20, 2016 at 8:05 PM, Justin Gordon [email protected]
wrote:

@martyphee https://github.com/martyphee Any info on this? Be
particularly careful with escaping issues. I.e., suppose you have
application data sent in the JavasScript, and it can close the script tag
and open a new one.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#412 (comment)

[jbhatab]

@martyphee sounds like a great improvement. Is the difference noticeable on phone load time?

[martyphee]

@jbhatab I have most of the test passing now. I'll hopefully get the rest done in today or tomorrow. Mainly the ones which just check the rendering of the div's.

[martyphee]

Ok, down to two test failures.

[justin808]

@martyphee The most important thing is that we super, super carefully consider the escaping and that we have good tests. If we incorrectly put the data in the generated JavaScript, this would allow for script injection hack.

[martyphee]

Won't that be more of the server making sure it doesn't except or removes all XSS attempts? Not sure the best way of escaping the data on the server without causing other side effects.

[squadette]

@justin808: just in case: http://apidock.com/rails/ERB/Util/json_escape properly escapes JSON-encoded strings for further inclusion to HTML code (inside the tags).

It handles things like { "foo": "<script>window.alert(1);</script>"}. It does JSON-level escaping, so that HTML-dangerous chars do not leak to the HTML code.

You can have a code like this:

  def json_safe_and_pretty(something)
    if Rails.env.development?
      json_escape(JSON.pretty_generate(something.as_json))
    else
      json_escape(something.to_json)
    end
  end

and use it in your ERB template like this:

<script type="application/json" id="json-entries"><%= raw json_safe_and_pretty(@foobar) %></script>

[gregoryStarr]

Hey Guys, I have a project I would love to use this on but we have a hard requirement of not being able to mount our state into the hidden divs the way this project does.. Has there been any progress on this or is there any way I can jump in and give a hand?

[justin808]

@gregoryStarr it would be awesome to get your help. Both @martyphee and I would appreciate it.

Please see the full message thread above. The main hazard is an XSS injection in the data.

[justin808]

@gregoryStarr Please work off of #411.

[justin808]

Closed by #775.