Implement safe filter as SafeString and handle SafeString in filters, functions and expressions #394
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Joeoh
changed the title
|
This PR is ready for review when you get a chance. This simplifies the implementation a great deal and gives us more confidence that the behaviour will remain consistent as the individual filters have been changed minimally. This resolves this issue replicated in this PR: #392 eg: https://github.com/HubSpot/jinjava/pull/394/files#diff-aa9fe1818d891da524d556e9ed0da866R30 |
boulter
approved these changes
Feb 3, 2020
Joeoh
added a commit
that referenced
this pull request
Feb 10, 2020
* First draft of deferring from tag including macros * Checkstyle * Add more tests * Remove incorrect assert * Add test that checks deferring macros in depth and update DeferredValue & MacroFunction * Checkstyle * Make constructor private and overload instance method * Fixes #159: Adds dict support for DefaultFilter * removes undesired code from Filter.java * removes whitespaces from DateTimeFormatFilter.java * Fixes Style Check Errors * Removes stringArgs from DateTimeFormatFilter * Revert "Fixes #159: Adds dict support for DefaultFilter" * Changes DefaultFilter to extend AdvancedFilter * adds PyList support to ForTag * adds tests for ForTag * removes escape filter from fortag test * Fix resoncstruct end to honor trim tags * Add reconstructImage to MacroFunction * Add test for reconstructing macro with no trim tags * Whitespace fix * Implement safe filter as SafeString and handle SafeString in filters, functions and expressions (#385) * Start implementing safe filter * Remove comment about pass-through implementation * Return var if it's not instance of string instead of throwing * Add test for pass-through * Add support for SafeString to all filters which handle Strings * Remove utils for string reverse filter * Handle SafeString in truncate function * Formatting fix * Add SafeStringFilter interface * Handle safe strings in filters * rm trailing space * Formatting fixes * Move safeFilter method to Filter IF and remove SafeFilter IF * rm space * Change behaviour of Urlize filter to not always return a SafeString * Code style changes * Remove unnecessary call to safeString * Style fix * Add tests to handle Urlize string being escaped and made safe * rm hardcoded string * rm uneeded getValue * Add SafeString type as str * Handle SafeString in expressions Co-authored-by: Joe <[email protected]> * Fix template error line numbers (#380) * Fix line numbers * add to some more places * two levels deep test * Fix case with child interpreter * Add deprecation * Add another test * Update error messages * Fix up error messages and tests * Fix case with scopes * Add check for inherit * Put everything on the path stack * always push parent * remove callstack crud * Set back path management * Fix extend lineNo/position and keep track for each block * cleanup * Add test for extends * Add more tests * Check parent call stack for emtpy and line numbers * Fix test * Reorient line numbers when evaluating macros, make sure to pop import path off of stack * Add test for imported macros * Reorient line numbers when resolving blocks * Reorient line numbers when processing extend parents * Add tests for extends + includes * Revert "Implement safe filter as SafeString and handle SafeString in filters, functions and expressions (#385)" This reverts commit a6bea47. * Implement safe filter as SafeString and handle SafeString in filters, functions and expressions (#394) * Start implementing safe filter * Remove comment about pass-through implementation * Return var if it's not instance of string instead of throwing * Add test for pass-through * Handle safestrings and call implementor * Add tests * Handle SafeString in filter using kwargs * Handle safe strings in most simple expressions * Handle SafeString in IsUpperExp * Handle SafeString in filters. Allow overriding from within filter * Formatting fixes Co-authored-by: jkollmann <[email protected]> * First draft of deferring from tag including macros * Checkstyle * Add more tests * Remove incorrect assert * Add test that checks deferring macros in depth and update DeferredValue & MacroFunction * Checkstyle * Make constructor private and overload instance method * Fix resoncstruct end to honor trim tags * Add reconstructImage to MacroFunction * Add test for reconstructing macro with no trim tags * Whitespace fix * rm duplicate statement after merge Co-authored-by: Manish Devgan <[email protected]> Co-authored-by: Jeff Boulter <[email protected]> Co-authored-by: Joe <[email protected]> Co-authored-by: Matt Coley <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Safe filter:
Replacement for reverted PR: #385
Implemenetation is much simplier than previous PR. Any filter which is receiving a SafeString object will now be handled without any changes to the underlying filter. If a function filter receives a SafeString and returns a String it will be wrapped in a SafeString. This is undesirable in some cases such as the ipAddr filter which does not return some direct version of the input. An override has been added to disable this wrapping for such filters.
preserveSafeStringcan be set to false to disable the wrapping.Documentation
Jinja docs: http://jinja.palletsprojects.com/en/2.10.x/templates/#working-with-automatic-escaping
Approach:
Wrap String with new class
SafeStringwhen the|safefilter is applied to the value. This way we can check the type inExpressionNodeand not escape it. I overwrotetoStringto make it work as expected here:jinjava/src/main/java/com/hubspot/jinjava/tree/ExpressionNode.java
Line 52 in 1667977
Considerations:
Since I introduce a new type here we might need to add some implementation for this in
ExpressionResolverorJinjavaInterpreterResolver. The implementation is currently only tested with|safebeing used at the end of an expression.