You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by GitBox <gi...@apache.org> on 2021/12/13 15:43:29 UTC
[GitHub] [logging-log4cxx] coldtobi opened a new pull request #83: Fix constructions of sed filter in corner cases
coldtobi opened a new pull request #83:
URL: https://github.com/apache/logging-log4cxx/pull/83
Some tests use sed to generate predictable outputs and employ sed files for
that. In some circumstances, the generated sed file syntax is incorrect, if
the subsitions data contains the sed expresssion seperator. The patch escapes
the data if needed. The bug manifested in (local) autopkgtest runs, when the
temporary directory contained the separator ("Q").
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] ams-tschoening commented on a change in pull request #83: Fix constructions of sed filter in corner cases
Posted by GitBox <gi...@apache.org>.
ams-tschoening commented on a change in pull request #83:
URL: https://github.com/apache/logging-log4cxx/pull/83#discussion_r769845917
##########
File path: src/test/cpp/util/transformer.cpp
##########
@@ -116,14 +116,29 @@ void Transformer::createSedCommandFile(const std::string& regexName,
std::string tmp;
+ auto sedSanitizer = [] (const std::string& in, const std::string& sedSeparator = "Q")
+ {
+ std::string ret = in;
+ std::string replaceTo = "\\" + sedSeparator;
+ size_t pos = 0;
+
+ while((pos = ret.find(sedSeparator, pos)) != std::string::npos)
+ {
+ ret.replace(pos, sedSeparator.length(), replaceTo);
+ pos += replaceTo.length();
+ }
+
+ return ret;
+ };
+
for (log4cxx::Filter::PatternList::const_iterator iter = patterns.begin();
iter != patterns.end();
iter++)
{
tmp = "sQ";
Review comment:
For my own thoughts: This results in something like `sQ...Q...Qg`, which is pretty much the same like `s/.../.../g` or `s!...!...!g` or `s#...#...#g` etc. The important point is that at least the delimiter needs to be escaped when available in the input, like was the case for this PR.
OTOH, in theory each and every character would need to be escaped because it might have special reg exp meaning? SED documents things like `&` and `\1` already. Though, might not be that important in this context, as it's only about tests with the only variable input being some paths in the file system.
Modern versions of SED support the argument `--regexp-extended`, which should support `\Q...\E` to escape arbitrary input. So something like `sQ\Q...\EQ\Q...\EQg` should work in the future as well.
https://www.boost.org/doc/libs/1_50_0/libs/regex/doc/html/boost_regex/syntax/basic_extended.html#boost_regex.syntax.basic_extended.quoting_escape
Though, a different delimiter should be chosen than to keep things readable. But on my system I have a version of SED using `UnxUtils` for Windows which doesn't support that. Instead, I would need to use SED bundled with GIT, but sadly wasn't able to get that work as expected as well. So am simply keeping things as they are right now.
> C:\Program Files (x86)\UnxUtils\usr\local\wbin
vs.
> C:\Program Files\Git\usr\bin
[ghpr_83_full_escape.diff.txt](https://github.com/apache/logging-log4cxx/files/7721553/ghpr_83_full_escape.diff.txt)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] ams-tschoening commented on a change in pull request #83: Fix constructions of sed filter in corner cases
Posted by GitBox <gi...@apache.org>.
ams-tschoening commented on a change in pull request #83:
URL: https://github.com/apache/logging-log4cxx/pull/83#discussion_r768790290
##########
File path: src/test/cpp/util/transformer.cpp
##########
@@ -116,14 +116,25 @@ void Transformer::createSedCommandFile(const std::string& regexName,
std::string tmp;
+ auto sedsaniziter = [] (const std::string &in, const std::string &sedseperator = "Q") {
+ std::string ret = in;
+ std::string replace_to = "\\" + sedseperator;
+ size_t pos = 0;
+ while((pos = ret.find(sedseperator, pos)) != std::string::npos) {
Review comment:
Please add a newline before and after the `while`, make things better readable.
##########
File path: src/test/cpp/util/transformer.cpp
##########
@@ -116,14 +116,25 @@ void Transformer::createSedCommandFile(const std::string& regexName,
std::string tmp;
+ auto sedsaniziter = [] (const std::string &in, const std::string &sedseperator = "Q") {
Review comment:
`sedsaniziter` contains a spelling error most likely, prefer `sedSanitizer`. `sedseperator` should follow that naming with `sedSeperator` as well. We mostly follow Java naming conventions.
Please change the references to be `const std::string& ...`, because most other code does so already.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] rm5248 commented on a change in pull request #83: Fix constructions of sed filter in corner cases
Posted by GitBox <gi...@apache.org>.
rm5248 commented on a change in pull request #83:
URL: https://github.com/apache/logging-log4cxx/pull/83#discussion_r770518357
##########
File path: src/test/cpp/util/transformer.cpp
##########
@@ -116,14 +116,29 @@ void Transformer::createSedCommandFile(const std::string& regexName,
std::string tmp;
+ auto sedSanitizer = [] (const std::string& in, const std::string& sedSeparator = "Q")
+ {
+ std::string ret = in;
+ std::string replaceTo = "\\" + sedSeparator;
+ size_t pos = 0;
+
+ while((pos = ret.find(sedSeparator, pos)) != std::string::npos)
+ {
+ ret.replace(pos, sedSeparator.length(), replaceTo);
+ pos += replaceTo.length();
+ }
+
+ return ret;
+ };
+
for (log4cxx::Filter::PatternList::const_iterator iter = patterns.begin();
iter != patterns.end();
iter++)
{
tmp = "sQ";
Review comment:
> Of course, if wanted, I can extend sedSanitizer to escape more characters, so that it `-E` (aka `--rexep-extended` is not needed), (Edit: Won't work for the same reason as in the last paragraph) or look into boost::regexp to avoid the invocation to sed alltogether. Though this feels for me as overkill just for the tests (having to have boost I mean)
FWIW since we're now on C++11, we have `std::regex`, which is basically the same as `boost::regexp`. If your compiler doesn't support C++17, boost is currently required anyway.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] ams-tschoening commented on a change in pull request #83: Fix constructions of sed filter in corner cases
Posted by GitBox <gi...@apache.org>.
ams-tschoening commented on a change in pull request #83:
URL: https://github.com/apache/logging-log4cxx/pull/83#discussion_r770285461
##########
File path: src/test/cpp/util/transformer.cpp
##########
@@ -116,14 +116,29 @@ void Transformer::createSedCommandFile(const std::string& regexName,
std::string tmp;
+ auto sedSanitizer = [] (const std::string& in, const std::string& sedSeparator = "Q")
+ {
+ std::string ret = in;
+ std::string replaceTo = "\\" + sedSeparator;
+ size_t pos = 0;
+
+ while((pos = ret.find(sedSeparator, pos)) != std::string::npos)
+ {
+ ret.replace(pos, sedSeparator.length(), replaceTo);
+ pos += replaceTo.length();
+ }
+
+ return ret;
+ };
+
for (log4cxx::Filter::PatternList::const_iterator iter = patterns.begin();
iter != patterns.end();
iter++)
{
tmp = "sQ";
Review comment:
I've tested and `--regexp-extend` is supported by my GIT-SED. Though, it seems I have missed again that regular expression are used and need to be supported, making my thoughts somewhat useless. :-) So let's simply keep things as they are now and change when needed as needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] coldtobi commented on a change in pull request #83: Fix constructions of sed filter in corner cases
Posted by GitBox <gi...@apache.org>.
coldtobi commented on a change in pull request #83:
URL: https://github.com/apache/logging-log4cxx/pull/83#discussion_r770247583
##########
File path: src/test/cpp/util/transformer.cpp
##########
@@ -116,14 +116,29 @@ void Transformer::createSedCommandFile(const std::string& regexName,
std::string tmp;
+ auto sedSanitizer = [] (const std::string& in, const std::string& sedSeparator = "Q")
+ {
+ std::string ret = in;
+ std::string replaceTo = "\\" + sedSeparator;
+ size_t pos = 0;
+
+ while((pos = ret.find(sedSeparator, pos)) != std::string::npos)
+ {
+ ret.replace(pos, sedSeparator.length(), replaceTo);
+ pos += replaceTo.length();
+ }
+
+ return ret;
+ };
+
for (log4cxx::Filter::PatternList::const_iterator iter = patterns.begin();
iter != patterns.end();
iter++)
{
tmp = "sQ";
Review comment:
Yes, this PR is only focusing on the delimiter only -- to fix the test issues I saw when packaging...
Regarding your sed: Did you try the switch `-E` instead? `--regexp-extend` is not POSIX, but `-E` would be, so maybe it understands only that switch (TIL: the feature is indeed POSIX; before, I thought this was not a portable feature)
<strike>Of course, if wanted, I can extend sedSanitizer to escape more characters, so that it `-E` (aka `--rexep-extended` is not needed),</strike> (Edit: Won't work for the same reason as in the last paragraph)
or look into boost::regexp to avoid the invocation to sed alltogether. Though this feels for me as overkill just for the tests (having to have boost I mean)
> Though, a different delimiter should be chosen than to keep things readable.
I agree :)
> ghpr_83_full_escape.diff.txt
Unconditonally wrapping won't work.. sed will treat them as literal, so any regexp in the strings will be NOPs...
So possibly the RegExps in patternlayouttest.cpp would be needed to be tweaked instead...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] coldtobi commented on a change in pull request #83: Fix constructions of sed filter in corner cases
Posted by GitBox <gi...@apache.org>.
coldtobi commented on a change in pull request #83:
URL: https://github.com/apache/logging-log4cxx/pull/83#discussion_r770247583
##########
File path: src/test/cpp/util/transformer.cpp
##########
@@ -116,14 +116,29 @@ void Transformer::createSedCommandFile(const std::string& regexName,
std::string tmp;
+ auto sedSanitizer = [] (const std::string& in, const std::string& sedSeparator = "Q")
+ {
+ std::string ret = in;
+ std::string replaceTo = "\\" + sedSeparator;
+ size_t pos = 0;
+
+ while((pos = ret.find(sedSeparator, pos)) != std::string::npos)
+ {
+ ret.replace(pos, sedSeparator.length(), replaceTo);
+ pos += replaceTo.length();
+ }
+
+ return ret;
+ };
+
for (log4cxx::Filter::PatternList::const_iterator iter = patterns.begin();
iter != patterns.end();
iter++)
{
tmp = "sQ";
Review comment:
Yes, this PR is only focusing on the delimiter only -- to fix the test issues I saw when packaging...
Regarding your sed: Did you try the switch `-E` instead? `--regexp-extend` is not POSIX, but `-E` would be, so maybe it understands only that switch (TIL: the feature is indeed POSIX; before, I thought this was not a portable feature)
Of course, if wanted, I can extend sedSanitizer to escape more characters, so that it `-E` (aka `--rexep-extended` is not needed), or look into boost::regexp to avoid the invocation to sed alltogether. Though this feels for me as overkill just for the tests (having to have boost I mean)
> Though, a different delimiter should be chosen than to keep things readable.
I agree :)
> ghpr_83_full_escape.diff.txt
Unconditonally wrapping won't work.. sed will treat them as literal, so any regexp in the strings will be NOPs...
So possibly the RegExps in patternlayouttest.cpp would be needed to be tweaked instead...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] ams-tschoening commented on a change in pull request #83: Fix constructions of sed filter in corner cases
Posted by GitBox <gi...@apache.org>.
ams-tschoening commented on a change in pull request #83:
URL: https://github.com/apache/logging-log4cxx/pull/83#discussion_r767901515
##########
File path: src/test/cpp/util/transformer.cpp
##########
@@ -116,14 +116,25 @@ void Transformer::createSedCommandFile(const std::string& regexName,
std::string tmp;
+ auto sedsaniziter = [] (const std::string in, const std::string sedseperator = "Q") {
Review comment:
Shouldn't the arguments better be `const std::string&`? Especially as you are copying `in` again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] coldtobi commented on a change in pull request #83: Fix constructions of sed filter in corner cases
Posted by GitBox <gi...@apache.org>.
coldtobi commented on a change in pull request #83:
URL: https://github.com/apache/logging-log4cxx/pull/83#discussion_r768849479
##########
File path: src/test/cpp/util/transformer.cpp
##########
@@ -116,14 +116,25 @@ void Transformer::createSedCommandFile(const std::string& regexName,
std::string tmp;
+ auto sedsaniziter = [] (const std::string &in, const std::string &sedseperator = "Q") {
Review comment:
should be fixed now by 38729af.
##########
File path: src/test/cpp/util/transformer.cpp
##########
@@ -116,14 +116,25 @@ void Transformer::createSedCommandFile(const std::string& regexName,
std::string tmp;
+ auto sedsaniziter = [] (const std::string &in, const std::string &sedseperator = "Q") {
+ std::string ret = in;
+ std::string replace_to = "\\" + sedseperator;
+ size_t pos = 0;
+ while((pos = ret.find(sedseperator, pos)) != std::string::npos) {
Review comment:
should be fixed now by 38729af.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] coldtobi commented on a change in pull request #83: Fix constructions of sed filter in corner cases
Posted by GitBox <gi...@apache.org>.
coldtobi commented on a change in pull request #83:
URL: https://github.com/apache/logging-log4cxx/pull/83#discussion_r770247583
##########
File path: src/test/cpp/util/transformer.cpp
##########
@@ -116,14 +116,29 @@ void Transformer::createSedCommandFile(const std::string& regexName,
std::string tmp;
+ auto sedSanitizer = [] (const std::string& in, const std::string& sedSeparator = "Q")
+ {
+ std::string ret = in;
+ std::string replaceTo = "\\" + sedSeparator;
+ size_t pos = 0;
+
+ while((pos = ret.find(sedSeparator, pos)) != std::string::npos)
+ {
+ ret.replace(pos, sedSeparator.length(), replaceTo);
+ pos += replaceTo.length();
+ }
+
+ return ret;
+ };
+
for (log4cxx::Filter::PatternList::const_iterator iter = patterns.begin();
iter != patterns.end();
iter++)
{
tmp = "sQ";
Review comment:
Yes, this PR is only focusing on the delimiter only -- to fix the test issues I saw when packaging...
Regarding your sed: Did you try the switch `-E` instead? `--regexp-extend` is not POSIX, but `-E` would be, so maybe it understands only that switch (TIL: the feature is indeed POSIX; before, I thought this was not a portable feature)
<strike>Of course, if wanted, I can extend sedSanitizer to escape more characters, so that it `-E` (aka `--rexep-extended` is not needed),</strike> (Edit: Won't work for the same reason, see last paragraph)
or look into boost::regexp to avoid the invocation to sed alltogether. Though this feels for me as overkill just for the tests (having to have boost I mean)
> Though, a different delimiter should be chosen than to keep things readable.
I agree :)
> ghpr_83_full_escape.diff.txt
Unconditonally wrapping won't work.. sed will treat them as literal, so any regexp in the strings will be NOPs...
So possibly the RegExps in patternlayouttest.cpp would be needed to be tweaked instead...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] coldtobi commented on a change in pull request #83: Fix constructions of sed filter in corner cases
Posted by GitBox <gi...@apache.org>.
coldtobi commented on a change in pull request #83:
URL: https://github.com/apache/logging-log4cxx/pull/83#discussion_r771167930
##########
File path: src/test/cpp/util/transformer.cpp
##########
@@ -116,14 +116,29 @@ void Transformer::createSedCommandFile(const std::string& regexName,
std::string tmp;
+ auto sedSanitizer = [] (const std::string& in, const std::string& sedSeparator = "Q")
+ {
+ std::string ret = in;
+ std::string replaceTo = "\\" + sedSeparator;
+ size_t pos = 0;
+
+ while((pos = ret.find(sedSeparator, pos)) != std::string::npos)
+ {
+ ret.replace(pos, sedSeparator.length(), replaceTo);
+ pos += replaceTo.length();
+ }
+
+ return ret;
+ };
+
for (log4cxx::Filter::PatternList::const_iterator iter = patterns.begin();
iter != patterns.end();
iter++)
{
tmp = "sQ";
Review comment:
Gosh, I thought `std::regex` was C++17 ... I stand corrected.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] coldtobi commented on a change in pull request #83: Fix constructions of sed filter in corner cases
Posted by GitBox <gi...@apache.org>.
coldtobi commented on a change in pull request #83:
URL: https://github.com/apache/logging-log4cxx/pull/83#discussion_r771167930
##########
File path: src/test/cpp/util/transformer.cpp
##########
@@ -116,14 +116,29 @@ void Transformer::createSedCommandFile(const std::string& regexName,
std::string tmp;
+ auto sedSanitizer = [] (const std::string& in, const std::string& sedSeparator = "Q")
+ {
+ std::string ret = in;
+ std::string replaceTo = "\\" + sedSeparator;
+ size_t pos = 0;
+
+ while((pos = ret.find(sedSeparator, pos)) != std::string::npos)
+ {
+ ret.replace(pos, sedSeparator.length(), replaceTo);
+ pos += replaceTo.length();
+ }
+
+ return ret;
+ };
+
for (log4cxx::Filter::PatternList::const_iterator iter = patterns.begin();
iter != patterns.end();
iter++)
{
tmp = "sQ";
Review comment:
Gosh, I thought std.:regex was C++17 ... I stand corrected.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] ams-tschoening commented on a change in pull request #83: Fix constructions of sed filter in corner cases
Posted by GitBox <gi...@apache.org>.
ams-tschoening commented on a change in pull request #83:
URL: https://github.com/apache/logging-log4cxx/pull/83#discussion_r769355155
##########
File path: src/test/cpp/util/transformer.cpp
##########
@@ -116,14 +116,18 @@ void Transformer::createSedCommandFile(const std::string& regexName,
std::string tmp;
- auto sedsaniziter = [] (const std::string &in, const std::string &sedseperator = "Q") {
+ auto sedSanitizer = [] (const std::string& in, const std::string& sedSeperator = "Q")
Review comment:
Sorry, missed that as well: Needs to be `sedSeparator` using an `a`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] coldtobi commented on a change in pull request #83: Fix constructions of sed filter in corner cases
Posted by GitBox <gi...@apache.org>.
coldtobi commented on a change in pull request #83:
URL: https://github.com/apache/logging-log4cxx/pull/83#discussion_r770247583
##########
File path: src/test/cpp/util/transformer.cpp
##########
@@ -116,14 +116,29 @@ void Transformer::createSedCommandFile(const std::string& regexName,
std::string tmp;
+ auto sedSanitizer = [] (const std::string& in, const std::string& sedSeparator = "Q")
+ {
+ std::string ret = in;
+ std::string replaceTo = "\\" + sedSeparator;
+ size_t pos = 0;
+
+ while((pos = ret.find(sedSeparator, pos)) != std::string::npos)
+ {
+ ret.replace(pos, sedSeparator.length(), replaceTo);
+ pos += replaceTo.length();
+ }
+
+ return ret;
+ };
+
for (log4cxx::Filter::PatternList::const_iterator iter = patterns.begin();
iter != patterns.end();
iter++)
{
tmp = "sQ";
Review comment:
Yes, this PR is only focusing on the delimiter only -- to fix the test issues I saw when packaging...
Regarding your sed: Did you try the switch `-E` instead? `--regexp-extend` is not POSIX, but `-E` would be, so maybe it understands only that switch (TIL: the feature is indeed POSIX; before, I thought this was not a portable feature)
Of course, if wanted, I can extend sedSanitizer to escape more characters, so that it `-E` (aka `--rexep-extended` is not needed), or look into boost::regexp to avoid the invocation to sed alltogether. Though this feels for me as overkill just for the tests (having to have boost I mean)
> Though, a different delimiter should be chosen than to keep things readable.
I agree
> ghpr_83_full_escape.diff.txt
Unconditonally wrapping won't work.. sed will treat them as literal, so any regexp in the strings will be NOPs...
So possibly the RegExps in patternlayouttest.cpp would be needed to be tweaked instead...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] ams-tschoening merged pull request #83: Fix constructions of sed filter in corner cases
Posted by GitBox <gi...@apache.org>.
ams-tschoening merged pull request #83:
URL: https://github.com/apache/logging-log4cxx/pull/83
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] coldtobi commented on a change in pull request #83: Fix constructions of sed filter in corner cases
Posted by GitBox <gi...@apache.org>.
coldtobi commented on a change in pull request #83:
URL: https://github.com/apache/logging-log4cxx/pull/83#discussion_r767947596
##########
File path: src/test/cpp/util/transformer.cpp
##########
@@ -116,14 +116,25 @@ void Transformer::createSedCommandFile(const std::string& regexName,
std::string tmp;
+ auto sedsaniziter = [] (const std::string in, const std::string sedseperator = "Q") {
Review comment:
you're right, fixing ...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] coldtobi commented on a change in pull request #83: Fix constructions of sed filter in corner cases
Posted by GitBox <gi...@apache.org>.
coldtobi commented on a change in pull request #83:
URL: https://github.com/apache/logging-log4cxx/pull/83#discussion_r770247583
##########
File path: src/test/cpp/util/transformer.cpp
##########
@@ -116,14 +116,29 @@ void Transformer::createSedCommandFile(const std::string& regexName,
std::string tmp;
+ auto sedSanitizer = [] (const std::string& in, const std::string& sedSeparator = "Q")
+ {
+ std::string ret = in;
+ std::string replaceTo = "\\" + sedSeparator;
+ size_t pos = 0;
+
+ while((pos = ret.find(sedSeparator, pos)) != std::string::npos)
+ {
+ ret.replace(pos, sedSeparator.length(), replaceTo);
+ pos += replaceTo.length();
+ }
+
+ return ret;
+ };
+
for (log4cxx::Filter::PatternList::const_iterator iter = patterns.begin();
iter != patterns.end();
iter++)
{
tmp = "sQ";
Review comment:
Yes, this PR is only focusing on the delimiter only -- to fix the test issues I saw when packaging...
Regarding your sed: Did you try the switch `-E` instead? `--regexp-extend` is not POSIX, but `-E` would be, so maybe it understands only that switch (TIL: the feature is indeed POSIX; before, I thought this was not a portable feature)
Of course, if wanted, I can extend sedSanitizer to escape more characters, so that it `-E` (aka `--rexep-extended` is not needed), or look into boost::regexp to avoid the invocation to sed alltogether. Though this feels for me as overkill just for the tests (having to have boost I mean)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org