keystonejs / keystone-classic

Node.js CMS and web app framework
http://v4.keystonejs.com
MIT License
14.63k stars 2.21k forks source link

Unicode "Line separator" in a Types.Text field stop javascript from working in the list page #735

Closed kumo closed 9 years ago

kumo commented 9 years ago

I was copying text from a PDF into a Types.Text field and I realised that the filtering and searching wasn't working. Using Safari I had an "unexpected EOF" error. After checking the strings I realised that there was a unicode "line separator" symbol in one of the titles. Is there an easy way of filtering these symbols?

morenoh149 commented 9 years ago

is this related to keystone? sounds like you just want to filter the text in your pdf from odd characters correct? maybe a quick js script or some vim-foo would do it

kumo commented 9 years ago

Well it is sort of related, because I coped text into my keystone app and afterwards I was unable to use the filtering and searching for that model; the javascript that was created was incorrect. Of course I can check the text that I write, but since my users could write anything, I thought that the search/filter javascript code should be ensuring that the javascript is valid.

morenoh149 commented 9 years ago

I'm reviewing this https://github.com/keystonejs/keystone/blob/master/lib/fieldTypes/text.js Which method do you think is blowing it up? also what search/filtering features are you using? I didn't even know keystone had that. Is this user facing or the admin UI?

kumo commented 9 years ago

In the admin UI. The moment in which a field contains invalid text, I was unable to add/remove columns or add filters. I am not entirely sure, but it would seem that the error is caused by the attempt to create JSON in the line Keystone.items = !{ JSON.stringify(items) }; in https://github.com/keystonejs/keystone/blob/master/templates/views/list.jade

kumo commented 9 years ago

Ok, it looks like the problem is simple: JavaScript strings are not allowed to contain U+2028 (line separator) and U+2029 (paragraph separator). I suppose at this point the easiest thing to do is filter them out in text and textarea fields (see kumo/keystone@b7674e8b32e519a2b21293700db956ac22a8fe63)

kumo commented 9 years ago

Alternative, list.jade could be replaced with Keystone.items = !{ JSON.stringify(items).replace(/\u2028/g, '').replace(/\u2029/g, '') };

sebmck commented 9 years ago

There are actually 4 valid JSON unicode characters that aren't valid in JS \u000A, \u000D, \u2028 and \u2029. Proper way to escape them would be:

JSON.stringify(items).replace(/[\u000A\u000D\u2028\u2029]/g, function (c) {
  return "\\u" + ("0000" + c.charCodeAt(0).toString(16)).slice(-4);
});
morenoh149 commented 9 years ago

So this issue was resolved? @sebmck @JedWatson