BUG: Incorrect unescaping of inlined props in hydrated component
dmitrysmagin opened this issue · 1 comments
@nickreese
I've found an error when elder can't parse props for the hydrated component.
Let's see the use case when we have a prop which contains a string with escaped double quotes:
{ prop: "This is a string with \"escaped\" quotes" }
In src/partialHydration/inlineSvelteComponent.ts there's a function to escape unwanted characters:
export function escapeHtml(text: string): string {
return text
.replace(/&/g, '&')
.replace(/</g, '<')
.replace(/>/g, '>')
.replace(/"/g, '"')
.replace(/'/g, ''');
}
...
const replacementAttrs = {
class: '"ejs-component"',
'data-ejs-component': `"${name}"`,
'data-ejs-props': `"${escapeHtml(JSON.stringify(props))}"`,
'data-ejs-options': `"${escapeHtml(JSON.stringify(hydrationOptions))}"`,
};
After JSON.stringify() the prop looks like this:
'{"prop":"This is a string with \\"escaped\\" quotes"}'
After applying escapeHtml():
'{"prop":"This is a string with \\"escaped\\" quotes"}'
Then in src/partialHydration/mountComponentsInHtml.ts there's a code to unescape characters and parse the prop back to JSON:
export const replaceSpecialCharacters = (str) =>
str
.replace(/\\\\n/gim, '\\n') // <-- ???
.replace(/"/gim, '"')
.replace(/</gim, '<')
.replace(/>/gim, '>')
.replace(/'/gim, "'")
.replace(/\\"/gim, '"') // <-- This breaks the unescaping
.replace(/&/gim, '&');
...
try {
hydrateComponentProps = JSON.parse(replaceSpecialCharacters(match[3]));
} catch (e) {
throw new Error(`Failed to JSON.parse props for ${hydrateComponentName} ${match[3]}`);
}
Parsing back to an object with JSON.parse will trigger an error, because replaceSpecialCharacters () incorrectly reverts the prop string:
{"prop":"This is a string with "escaped" quotes"}
BANG! Escaped quotes are gone, JSON.parse throws an exception.
I think both .replace(/\"/gim, '"') and .replace(/\\n/gim, '\n') should be removed. Both escapeHtml() and replaceSpecialCharacters() should be revertable, i.e. convert the same set of charactes to escaped ones and back.
@dmitrysmagin Somehow GitHub didn't notify me of this issue or I missed the notification. 👼
Interestingly I encountered this on ElderGuide.com this past week and on my private branch of Elder.js it is fixed.
I've added robust tests to make sure the input and output are the same of these two functions and pushed in 1.6.14
let me know how you get on with them.