-
-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Hydration fails with nested props arrays/objects #150
Comments
The only general solution is base64 (or some other safe serialisation). Otherwise e.g. strings that contain HTML will be rendered by the browser rather than treated as part of the JSON regardless of how the JS parses it. |
The other solutions are viable. The special token solution is used in multipart payloads (eg every time you submit an HTML form with file upload or open an HTML email, so ~every day for regular internet users), and the multiline string solution for most shells and many shell inspired languages. Separating the markers from the props is just a hash map reference, the marker referencing a key in the hydration props payload. They all have tradeoffs. Base-64 has the most costly tradeoff but it’s the least impact to implement. |
Another solution that’s probably a much bigger lift would be a huge win if possible: don’t mount with props at all. Effectively memoize on the server. The big cost (for users) here is second render because you need a comparison algorithm that’s efficient, and JS doesn’t have one. The big cost (for devs) is accounting for that. But if you squint a little bit it’s actually not a big deal. Just mount noop on load and render on first lifecycle check and don’t worry about the diff. Or async trigger the second render on idle/visible. It’s the same expense with no payload at all. And dang it feels good to |
Sorry I should have been more clear. I agree that the other solutions would work for the JSON parsing case, but my point is that it's worth doing the safe serialisation one because of a different problem (which the other methods can't solve): The problem is that hydration props like: {
"key": "<script>alert()</script>"
} without safe serialisation will cause the script to be executed by the browser. EDIT: Memoizing on the server effectively embeds the payload into JS, which also sidesteps this problem, so that's viable too. |
That wouldn’t be a problem if the data is actually JSON treated as such, either:
|
Actually you’re right there is a potential XSS (self own or not), JSON serialization doesn’t account for it. So some change in that regard is definitely warranted as well. |
I think serializing into JS is the common theme here and regardless of the rest of the specifics it’s a clear win. |
Props which reproduce this:
These props produce this error:
I suspect this would be solved by changing this regex to:
But I'm hesitant to open a PR because I don't fully grasp what's expected in the hydration markers. That said, I'd also probably recommend not using a regex for this purpose in the first place. A few options I've thought of:
The text was updated successfully, but these errors were encountered: