Skip to content
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

Serializing object that has line breaks or other control characters in it #25

Closed
sebastiancarlsson opened this issue Aug 16, 2016 · 16 comments

Comments

@sebastiancarlsson
Copy link

Not sure if this is the right place to ask this. I'm building a universal React app and I'm using this module to serialize my immutable Redux store, which is then printed as a javascript variable and picked up by my client code:

Server:

const serializedStore = transit.toJSON(store.getState());
// later:
window.INITIAL_STATE = serializedStore;

Client:

const initialState = window.INITIAL_STATE ? transit.fromJSON( window.INITIAL_STATE ) : new Map();

However, if there are any line breaks or other control characters the browser will complain about "unexpected token in JSON". Do I also have to escape the string before printing it to the browser? All examples I've found of using this library doesn't do this but also doesn't seem to take into account the possibility of line breaks etc. in the JSON content.

@glenjamin
Copy link
Owner

Can you give a concrete example which fails? This seems similar to #20 which got reported upstream as cognitect/transit-js#40 but I was unable to reproduce.

I've just added a2bb3b6 to the testsuite which shows that this appears to work fine in a small case.

@nkbt
Copy link

nkbt commented Aug 17, 2016

It might be related to double-encoding... Like if it is

"Maps": Immutable.Map({"abc": JSON.stringify({x: "def\nghi"})}),

Though I am not sure.

So far I had no time to play with it and find some super simple reproduceable example, but the issue is still there for me (we just do not put any funky stuff into URL anymore ;) )

@sebastiancarlsson
Copy link
Author

I solved the issue by adding an extra set of slashes:

let serializedStore = transit.toJSON(store.getState());
serializedStore = serializedStore
    .replace(/\\/g, '\\\\')
    .replace(/\'/g, '\\\'')
    .replace(/\"/g, '\\"')
    .replace(/\0/g, '\\0')

Picked it up from here: https://magp.ie/2011/01/20/addslashes-and-stripslashes-in-javascript/

@glenjamin
Copy link
Owner

I still can't seem to find a way to get those characters to appear in the serialised output. If you could provide a small example that does it would be really helpful.

@nkbt
Copy link

nkbt commented Aug 18, 2016

I guess it is hard to find an example since this only happens when we transfer redux state from server to client this way:

import {toJSON} from 'transit-immutable-js';

const Data = ({data}) => (
  <script
    dangerouslySetInnerHTML={{__html: `window.__data='${toJSON(data)}';`}}
    charSet="UTF-8" />
);

const Html = ({app, store}) => (
  <html>
    <body>
      <div id="app" dangerouslySetInnerHTML={{__html: component ? ReactDOM.renderToString(app) : ''}} />
      <Data data={store.getState()} />
    </body>
  </html>
);

I've just thought that it might be as well a problem somewhere in React's dangerouslySetInnerHTML...

@sebastiancarlsson
Copy link
Author

Yes you are exactly right, @glenjamin the example provided by @nkbt sums it up nicely. What simply seems to happen is that the string is stripped of slashes automatically. Consider this: open the console (in a new tab of your browser!) and simply run:

document.write("This is a \"quote\"")

The output will be This is a "quote", so we did not manually have to strip the slashes, this happens automagically when the string is printed to the DOM. So subsequently if I have a JSON string with double quotes somewhere, properly escaped, that I'm trying to print to the browser as a JavaScript variable enclosed in double quotes, it will be unescaped and the page will break. So the solution to this is to "double-slash" it as I said above. Somewhere deep down in the React core they must obviously be using some native JavaScript methods to render things to the browser so I don't think it's a problem with any of the libraries I use, it's simply how JavaScript works.

Now of course it is out of scope for this library to solve this because it has nothing to do with serialization (and it's obviously not limited to serializing immutable.js objects), but on the other hand, I would assume that this is a very common real world example of how people would use transit and transit-immutable-js. So maybe the question is how and where we should present this information and provide examples on how to solve it?

@glenjamin
Copy link
Owner

Ah, ok I see the problem now - it's because the underlying transit lib mostly deals with strings rather than objects (rather annoyingly, see cognitect/transit-js#23).

It's not safe to spit a string out onto the page by simply wrapping it in quotes, as you need to deal with escaping. The safest way to get JS to encode a string value as a string you can place on the page is actually to use JSON.stringify again, so the result should look like this:

const Data = ({data}) => (
  <script
    dangerouslySetInnerHTML={{__html: `window.__data=${JSON.stringify(toJSON(data))};`}}
    charSet="UTF-8" />
);

We use transit.toJSON to give us a string, and then we use JSON.stringify to encode that string into a string literal we can place in a javascript context.

If we added transit.decode which accepted an object instead of a string, we could skip the extra string wrapping and probably speed things up a bit when parsing.

@sebastiancarlsson
Copy link
Author

@glenjamin Ok, I think this makes sense, thank you. I will subscribe to the issue you referred to.

@coreymaher
Copy link

+1 for the transit.decode idea

@nkbt
Copy link

nkbt commented Aug 18, 2016

I see! In this case double-encoding is not critical and it will work for us
just fine. Though would be nice to have it fixed at the core level.

I will check if it works reliably and let you know
On Thu, 18 Aug 2016 at 20:02, Glen Mailer [email protected] wrote:

Ah, ok I see the problem now - it's because the underlying transit lib
mostly deals with strings rather than objects (rather annoyingly, see
cognitect/transit-js#23
cognitect/transit-js#23).

It's not safe to spit a string out onto the page by simply wrapping it in
quotes, as you need to deal with escaping. The safest way to get JS to
encode a string value as a string you can place on the page is actually to
use JSON.stringify again, so the result should look like this:

const Data = ({data}) => (

<script dangerouslySetInnerHTML={{__html: `window.__data=${JSON.stringify(toJSON(data))};`}} charSet="UTF-8" /> ); We use transit.toJSON to give us a string, and then we use JSON.stringify to encode that string into a string literal we can place in a javascript context. If we added transit.decode which accepted an object instead of a string, we could skip the extra string wrapping and probably speed things up a bit when parsing. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com//issues/25#issuecomment-240679696, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKsoBuWwyNgrhWLZE9URobmSl-npQK2ks5qhC2zgaJpZM4Jlk8t .

@nkbt
Copy link

nkbt commented Aug 19, 2016

Unfortunately this does not help

    dangerouslySetInnerHTML={{__html: `window.__data='${JSON.stringify(toJSON(data))}';`}}

20160819-172701

@glenjamin
Copy link
Owner

Too many quotes there, leave out the single quotes.

On 19 Aug 2016, at 08:27, Nik Butenko [email protected] wrote:

Unfortunately this does not help

dangerouslySetInnerHTML={{__html: `window.__data='${JSON.stringify(toJSON(data))}';`}}


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@nkbt
Copy link

nkbt commented Aug 19, 2016

My fix for the moment, encode JSON safely with base64 (unicode): https://github.com/mathiasbynens/base64

Server

const Data = ({data}) => (
  <script
    dangerouslySetInnerHTML={{__html: `window.__data='${base64.encode(toJSON(data))}';`}}
    charSet="UTF-8" />
);

Client

    this.store = createReduxStore(fromJSON(base64.decode(window.__data)));

Works without issues.

@nkbt
Copy link

nkbt commented Aug 19, 2016

Too many quotes there, leave out the single quotes.

Hm, yeah, that worked as well. I reckon it is still better then base64 ;)

@nkbt
Copy link

nkbt commented Aug 19, 2016

Thanks, for now it works with an extra JSON.stringify

@glenjamin
Copy link
Owner

Closing in favour of #27 which has a nice short summary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants