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

feat(sanitization): refactored, fixed and extended sanitization #993 #1014

Closed
wants to merge 1 commit into from

Conversation

marklagendijk
Copy link
Contributor

  • Added $translateSanitization
  • Removed sanitization logic from interpolators, and implemented usage of $translateSanitization
  • Changed $translate to use $translateSanitizationProvider for setting the strategy
  • Added support for using multiple sanitization strategies, which will be executed as a chain.
  • Throw an error when an unknown strategy name is specified. Failing silently would be a security issue.
  • Added methods for adding / removing strategies to $translateSanitizationProvider.

@knalli
Copy link
Member

knalli commented Apr 23, 2015

Thank you. Looks great. It would be great if you could apply some coding styles (if (expr) { or } else { like the existing ones. There are hint/lint errors, however, that's actually a regression made some months ago in the build chain I'd just figured out. Not your fault. ;(

Anyway.. first look promises a great feature. Everything is tested, even the case w/o the ngSanitize module. I'll have a depper look tomorrow I thing..

@marklagendijk
Copy link
Contributor Author

I adjusted the formatting settings in WebStorm and reformatted the code. Apart from that I fixed a small bug, and improved the sanitization tests in the interpolators.

…ar-translate#993

- Added $translateSanitization
- Removed sanitization logic from interpolators, and implemented usage of  $translateSanitization
- Changed $translate to use $translateSanitizationProvider for setting the strategy
- Added support for using multiple sanitization strategies, which will be executed as a chain.
- Throw an error when an unknown strategy name is specified. Failing silently would be a security issue.
- Added methods for adding / removing strategies to $translateSanitizationProvider.
@knalli
Copy link
Member

knalli commented Apr 25, 2015

@marklagendijk Thank you very much. Landed as 12dbc57 with some fixups (additional info in log for changelog, and a few updates in the code)

@knalli knalli closed this Apr 25, 2015
@marklagendijk
Copy link
Contributor Author

@knalli you're welcome. Thanks for the way you helped with designing and reviewing this. I think we made an huge improvement to the security of angular-translate.

@knalli
Copy link
Member

knalli commented Apr 25, 2015

Yes, you are absolutely right. That one has long been overdue.

@marklagendijk
Copy link
Contributor Author

@knalli question: when do you expect to be the new version (2.7.0?), with this feature in it to be released?

@knalli
Copy link
Member

knalli commented Apr 28, 2015

@marklagendijk We have currently 1-2 open feedbacks in queue. After this, we are ready.

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

Successfully merging this pull request may close these issues.

None yet

2 participants