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

Fix escaping/unescaping of text and attributes in SSR #747

Merged
merged 5 commits into from
Aug 4, 2017
Merged

Conversation

Rich-Harris
Copy link
Member

@Rich-Harris Rich-Harris commented Aug 4, 2017

Fixes #741

@Rich-Harris Rich-Harris changed the title [WIP] failing tests for #741 Fix escaping/unescaping of text and attributes in SSR Aug 4, 2017
@Rich-Harris
Copy link
Member Author

Alright, hopefully this is fixed once and for all! @Conduitry would appreciate a once-over if you get a chance, you seem to have a knack for these bugs...

@@ -1,3 +1,7 @@
export default function stringify(data: string) {
export function stringify(data: string) {
return JSON.stringify(data.replace(/([^\\@#])?([@#])/g, '$1\\$2'));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stringify should probably call escape instead of having its own copy of the regex.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch! for a brief moment while I was bashing my head against a wall they had different regexes, hence the (now unnecessary) duplication. fixed

@@ -178,7 +178,12 @@ export default function ssr(
function __escape ( html ) {
return String( html ).replace( /["'&<>]/g, match => escaped[ match ] );
}
`.replace(/(\\)?@(\w*)/g, (match: string, escaped: string, name: string) => escaped ? match.slice(1) : generator.alias(name));
`.replace(/(\\)?([@#])(\w*)/g, (match: string, escaped: string, sigil: string, name: string) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see similar (but not identical) code to this in the DOM generator, but I'm not entirely sure what it all does. Have all #-sigiled identifiers already been replaced, because we're at the top level?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no #-sigiled identifiers in SSR code, because that means 'create a unique name inside this block', and there are no 'blocks' to speak of (because everything is inline). But because it uses the same escape helper, # characters are escaped anyway, and therefore need to be unescaped. A bit circuitous, admittedly

@Rich-Harris Rich-Harris merged commit 207c599 into master Aug 4, 2017
@Rich-Harris Rich-Harris deleted the gh-741 branch August 4, 2017 18:24
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

Successfully merging this pull request may close these issues.

2 participants