Preloaded Data Changes (PR #10129)

!!! This is just a draft for discussion !!!

This draft moves the big preloaded data payload we send on the HTML from a data-attribute to a Javascript variable.

In doing this, it greatly reduces the size of the HTML (around ~50KB depending on site, before gzip) and reduces the number of necessary JSON.parse on the browser boot. To work with CSP it adds a nonce to secure the inline script, keeping this payload inline with the main document.

On the Ruby side, we are still generating/storing JSONs everywhere, so this PR hacks around by doing parsing each part back into a Hash so it can create a proper JSON only in the last step. If this approach is deemed worth to follow up we will need to fix this by adding Hash versions of each method so we don’t do double the work on the backend.

I would like the review on this refactor to see if it is something worth pursuing.

One unexpected change from this PR is that it fix the “View Source” breaking on Firefox :smiley: . I believe the 350KB single HTML tag highlight was the culprit.


What happens if you use script type="text/discourse-preload" instead?

Premature feedback, perhaps, but we should use a much more complex variable name here to avoid conflicts since it’s a global namespace.

In theory, I like it. It is much simpler. However, does it make us potential victims of this slow path?

It might be faster to emit a variable that is a JSON.parse even though that seems counter intuitive.

The nonce-based CSP option is only secure if the nonce is unique on every request. I think that’s mostly true with this implementation, except when the anon cache comes into play. When that happens, the same nonce will get sent to multiple clients. An attacker could then take that nonce and potentially use it to perform an XSS attack.

Maybe we could use the Hash method instead?

That even bypasses the need for a nonce! Which also fixed the perf concern from @eviltrout and the nonce cache from @davidtaylorhq

On the other hand means I need to escape script close tags :thinking:

I am curious why we need the sub thing… I think having < or > will never be included in json. For example:

pry(main)> puts ({test: "</script>h4cked"}.to_json)

But there must be a reason it was there before :thinking:

Check if the existing Rails html escaper takes care of the angle brackets, yeah.