Skip to content
This repository was archived by the owner on Oct 9, 2019. It is now read-only.

suppress handling of special characters in replacement text#20

Closed
chesles wants to merge 3 commits intomrtazz:masterfrom
chesles:dollar-sign
Closed

suppress handling of special characters in replacement text#20
chesles wants to merge 3 commits intomrtazz:masterfrom
chesles:dollar-sign

Conversation

@chesles
Copy link
Contributor

@chesles chesles commented Aug 18, 2015

Given replacement text with special characters like $, (, ), \, and probably others, the call to regex_replace was handling these - lines 107-117 seem to be a workaround for \, but using the boost::regex_constants::format_literal to suppress handling of all special characters in the replacement text seems like a better solution.

See also #17 and boost replace options documentation.

@mrtazz
Copy link
Owner

mrtazz commented Sep 3, 2015

Oh good catch, thanks for this! Do you know if there is a corresponding flag for C++11? I'm planning to merge #19 and remove the boost dependency so it would be nice to not regress on this. But I'll anyway make sure to merge this before I remove boost, as there will be a last release depending on boost anyways. That way we have this fixed at least.

@chesles
Copy link
Contributor Author

chesles commented Sep 3, 2015

I'm not sure about a C++11 flag, I'll look into it and see if I can find one.

Doing a release with this fix before #19 gets merged sounds like a good approach to me.

@mrtazz mrtazz added this to the v0.4.0 milestone Sep 3, 2015
@mrtazz mrtazz self-assigned this Sep 3, 2015
@chesles
Copy link
Contributor Author

chesles commented Sep 4, 2015

It looks like there's not a corresponding flag for this in C++11 - the format string is controlled by the format_* flags described here, and none of them appear to be what we want for this.

However, by default regex_replace uses ECMAScript rules to evaluate the format string, and the only special character is $ (syntax reference here), so replacing $ with $$ on input strings should be all that's needed to get the correct behavior.

Here's a diff against #19 that implements this (and also removes code that seems to replace \\ with \\ - probably not needed unless I'm missing something there). I confirmed the tests added in this PR pass with this patch (only the dollar sign test fails without it, which is expected). @kylealanhale can you add this to your PR (or, what's the best way for me to add it?)

diff --git a/src/template.cpp b/src/template.cpp
index 7c43d03..23c49cb 100644
--- a/src/template.cpp
+++ b/src/template.cpp
@@ -154,24 +154,13 @@ std::string template_t::render_tags(const std::string& tmplate,
         trim(modifier);
         std::string text(start, matches[0].second);
         std::string repl;
+        std::string escaped_special_chars("$$$&");
+        std::regex special_chars("\\$");
+
         // don't html escape this
         if (modifier == "&" || modifier == "{")
         {
-            try
-            {
-                // get value
-                std::string s = ctx.get(key)[0][key];
-                // escape backslash in std::string
-                const std::string f = "\\";
-                size_t found = s.find(f);
-                while(found != std::string::npos)
-                {
-                    s.replace(found,f.length(),"\\");
-                    found = s.find(f, found+2);
-                }
-                repl.assign(s);
-            }
-            catch(int i) { repl.assign(""); }
+          repl.assign(ctx.get(key)[0][key]);
         }
         // this is a comment
         else if (modifier == "!")
@@ -193,6 +182,7 @@ std::string template_t::render_tags(const std::string& tmplate,
             }
             catch(int i) { repl.assign(""); }
         }
+        repl.assign(regex_replace(repl, special_chars, escaped_special_chars));

         // replace
         ret += regex_replace(text, tag, repl,

@mrtazz mrtazz closed this in 6922191 May 21, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants