Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

feat($interpolate): escape interpolated expressions #7496

Closed
wants to merge 1 commit into from
Closed

feat($interpolate): escape interpolated expressions #7496

wants to merge 1 commit into from

Conversation

shahata
Copy link
Contributor

@shahata shahata commented May 17, 2014

the default escaped interpolation signs are {{{{ and }}}}. those symbols will be ignored when parsing the interpolated string and will only be replaced by {{ and }} in the result string. this allows servers that put unsafe strings inside html templates to replace {{ with {{{{ and optionally }} with }}}} in order to prevent XSS attacks.

Closes #5601

@mary-poppins
Copy link

Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.

  • Uses the issue template (#7496)

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

@shahata
Copy link
Contributor Author

shahata commented May 17, 2014

This is another approach to escaping interpolation signs in a more simplistic fashion than #5628. I think it is better since all it does is ignore {{{{ and }}}} when looking for expressions in the interpolated string and replace them with {{ and }} in the result string.

@caitp - what do you think about this?

@@ -316,6 +335,11 @@ function $InterpolateProvider() {
return endSymbol;
};

function unescape(text) {
return text.split(escapedStartSymbol).join(startSymbol)
.split(escapedEndSymbol).join(endSymbol);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm pretty frustrated with the split/join thing. The alternative is to either write a simple replaceAll function or use some ugly regexp escaping function so that we can replace(new RegExp(escapedStartRegexp, 'g'), startSymbol)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, I think escapedEndSymbol is pretty useless, maybe we can just remove it... (we can have a escapedStartSymbol method instead of adding the escaped param to the startSymbol method, seems more elegant in my opinion since currently startSymbol method is a kind of weird "set two things or get one thing")

Copy link
Contributor

Choose a reason for hiding this comment

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

then you'd render the wrong thing, anyways whatever, there's enough comments, take a break

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I mean we could say that the server needs to escape only the start symbol. What do you mean by there's enough comments?

the default escaped interpolation signs are `{{{{` and `}}}}`. those symbols will be ignored when parsing the interpolated string and will only be replaced by `{{` and `}}` in the result string. this allows servers that put unsafe strings inside html templates to replace `{{` with `{{{{` and optionally `}}` with `}}}}` in order to prevent XSS attacks.

Closes #5601
@shahata
Copy link
Contributor Author

shahata commented May 20, 2014

Closing in favour of #7517

@shahata shahata closed this May 20, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

XSS issues with server-rendered Angular templates
3 participants