You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficserver.apache.org by zwoop <gi...@git.apache.org> on 2016/10/23 18:41:12 UTC
[GitHub] trafficserver pull request #1133: TS-4993: Disables escaping and quotes insi...
GitHub user zwoop opened a pull request:
https://github.com/apache/trafficserver/pull/1133
TS-4993: Disables escaping and quotes inside regexes
In addition, this cleans up the unit tests a bit, to make it more useful from
the command line when testing/debugging. New tests are also added for regular
expression parse testing.
Note: This restores 6.1.x functionality in 7.x, however, it's not compatible with 5.3.x. I do feel that this is the right approach, because it retains PCRE compatibility within the // that indicates a regex.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zwoop/trafficserver TS-4993
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafficserver/pull/1133.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1133
----
commit 75757f3eb4a50d1c80997356e6df2d001e32d360
Author: Leif Hedstrom <zw...@apache.org>
Date: 2016-10-22T01:18:43Z
TS-4993: Disables escaping and quotes inside regexes
In addition, this cleans up the unit tests a bit, to make it more useful from
the command line when testing/debugging. New tests are also added for regular
expression parse testing
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1133: TS-4993: Disables escaping and quotes insi...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1133#discussion_r84770706
--- Diff: plugins/header_rewrite/parser.cc ---
@@ -133,14 +152,15 @@ Parser::preprocess(std::vector<std::string> tokens)
std::string s = tokens[0].substr(2, tokens[0].size() - 3);
_op = s;
- if (tokens.size() > 2 && (tokens[1][0] == '=' || tokens[1][0] == '>' || tokens[1][0] == '<')) { // cond + (=/</>) + argument
+ if (tokens.size() > 2 && (tokens[1][0] == '=' || tokens[1][0] == '>' || tokens[1][0] == '<')) {
+ // cond + [=<>] + argument
_arg = tokens[1] + tokens[2];
} else if (tokens.size() > 1) {
+ // This is for the regular expression, which for some reason has its own handling?? ToDo: Why ?
--- End diff --
The comment? It's what it says, that case is for the regular expression case, I kinda wish we had treated / as the other operators (=, < and >), but we do not. And I don't think this is the time to change that.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1133: TS-4993: Disables escaping and quotes inside rege...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1133
Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/966/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1133: TS-4993: Disables escaping and quotes inside rege...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1133
Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/959/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1133: TS-4993: Disables escaping and quotes insi...
Posted by bryancall <gi...@git.apache.org>.
Github user bryancall commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1133#discussion_r84722717
--- Diff: plugins/header_rewrite/header_rewrite_test.cc ---
@@ -31,274 +33,395 @@ const char PLUGIN_NAME_DBG[] = "TEST_dbg_header_rewrite";
extern "C" void
TSError(const char *fmt, ...)
{
- char buf[2048];
- int bytes = 0;
- va_list args;
- va_start(args, fmt);
- if ((bytes = vsnprintf(buf, sizeof(buf), fmt, args)) > 0) {
- fprintf(stderr, "TSError: %s: %.*s\n", PLUGIN_NAME, bytes, buf);
- }
- va_end(args);
}
-extern "C" void
-TSDebug(const char *tag, const char *fmt, ...)
-{
- char buf[2048];
- int bytes = 0;
- va_list args;
- va_start(args, fmt);
- if ((bytes = vsnprintf(buf, sizeof(buf), fmt, args)) > 0) {
- fprintf(stdout, "TSDebug: %s: %.*s\n", PLUGIN_NAME, bytes, buf);
- }
- va_end(args);
-}
-
-#define CHECK_EQ(x, y) \
- do { \
- if ((x) != (y)) { \
- fprintf(stderr, "CHECK FAILED\n"); \
- return 1; \
- } \
- } while (false);
-
class ParserTest : public Parser
{
public:
- ParserTest(std::string line) : Parser(line) {}
+ ParserTest(std::string line) : Parser(line), res(true) { std::cout << "Starting parser test: " << line << std::endl; }
std::vector<std::string>
getTokens()
{
return _tokens;
}
+
+ template <typename T, typename U>
+ void
+ do_parser_check(T x, U y, int line = 0)
+ {
+ if (x != y) {
+ std::cerr << "CHECK FAILED on line " << line << ": " << x << " != " << y << std::endl;
+ res = false;
+ }
+ }
+
+ bool res;
--- End diff --
What is this for?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1133: TS-4993: Disables escaping and quotes insi...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1133#discussion_r84729282
--- Diff: plugins/header_rewrite/header_rewrite_test.cc ---
@@ -31,274 +33,395 @@ const char PLUGIN_NAME_DBG[] = "TEST_dbg_header_rewrite";
extern "C" void
TSError(const char *fmt, ...)
{
- char buf[2048];
- int bytes = 0;
- va_list args;
- va_start(args, fmt);
- if ((bytes = vsnprintf(buf, sizeof(buf), fmt, args)) > 0) {
- fprintf(stderr, "TSError: %s: %.*s\n", PLUGIN_NAME, bytes, buf);
- }
- va_end(args);
}
-extern "C" void
-TSDebug(const char *tag, const char *fmt, ...)
-{
- char buf[2048];
- int bytes = 0;
- va_list args;
- va_start(args, fmt);
- if ((bytes = vsnprintf(buf, sizeof(buf), fmt, args)) > 0) {
- fprintf(stdout, "TSDebug: %s: %.*s\n", PLUGIN_NAME, bytes, buf);
- }
- va_end(args);
-}
-
-#define CHECK_EQ(x, y) \
- do { \
- if ((x) != (y)) { \
- fprintf(stderr, "CHECK FAILED\n"); \
- return 1; \
- } \
- } while (false);
-
class ParserTest : public Parser
{
public:
- ParserTest(std::string line) : Parser(line) {}
+ ParserTest(std::string line) : Parser(line), res(true) { std::cout << "Starting parser test: " << line << std::endl; }
std::vector<std::string>
getTokens()
{
return _tokens;
}
+
+ template <typename T, typename U>
+ void
+ do_parser_check(T x, U y, int line = 0)
+ {
+ if (x != y) {
+ std::cerr << "CHECK FAILED on line " << line << ": " << x << " != " << y << std::endl;
+ res = false;
+ }
+ }
+
+ bool res;
--- End diff --
That's the check :). This is what DO_CHECK() used to do as a macro, but I turned it into a templetized function so that DO_CHECK can produce output about the check for the various types of input parameters.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1133: TS-4993: Disables escaping and quotes insi...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1133#discussion_r84770458
--- Diff: plugins/header_rewrite/header_rewrite_test.cc ---
@@ -31,274 +33,395 @@ const char PLUGIN_NAME_DBG[] = "TEST_dbg_header_rewrite";
extern "C" void
TSError(const char *fmt, ...)
{
- char buf[2048];
- int bytes = 0;
- va_list args;
- va_start(args, fmt);
- if ((bytes = vsnprintf(buf, sizeof(buf), fmt, args)) > 0) {
- fprintf(stderr, "TSError: %s: %.*s\n", PLUGIN_NAME, bytes, buf);
- }
- va_end(args);
}
-extern "C" void
-TSDebug(const char *tag, const char *fmt, ...)
-{
- char buf[2048];
- int bytes = 0;
- va_list args;
- va_start(args, fmt);
- if ((bytes = vsnprintf(buf, sizeof(buf), fmt, args)) > 0) {
- fprintf(stdout, "TSDebug: %s: %.*s\n", PLUGIN_NAME, bytes, buf);
- }
- va_end(args);
-}
-
-#define CHECK_EQ(x, y) \
- do { \
- if ((x) != (y)) { \
- fprintf(stderr, "CHECK FAILED\n"); \
- return 1; \
- } \
- } while (false);
-
class ParserTest : public Parser
{
public:
- ParserTest(std::string line) : Parser(line) {}
+ ParserTest(std::string line) : Parser(line), res(true) { std::cout << "Starting parser test: " << line << std::endl; }
std::vector<std::string>
getTokens()
{
return _tokens;
}
+
+ template <typename T, typename U>
+ void
+ do_parser_check(T x, U y, int line = 0)
+ {
+ if (x != y) {
+ std::cerr << "CHECK FAILED on line " << line << ": " << x << " != " << y << std::endl;
+ res = false;
+ }
+ }
+
+ bool res;
};
+#define CHECK_EQ(x, y) \
+ do { \
+ p.do_parser_check((x), (y), __LINE__); \
+ } while (false);
+
+#define END_TEST(s) \
+ do { \
+ if (!p.res) { \
+ ++errors; \
+ } \
+ } while (false);
+
int
test_parsing()
{
+ int errors = 0;
+
{
ParserTest p("cond %{READ_REQUEST_HDR_HOOK}");
- CHECK_EQ(p.getTokens().size(), 2);
+
+ CHECK_EQ(p.getTokens().size(), 2U);
CHECK_EQ(p.getTokens()[0], "cond");
CHECK_EQ(p.getTokens()[1], "%{READ_REQUEST_HDR_HOOK}");
+
+ END_TEST();
}
{
ParserTest p("cond %{CLIENT-HEADER:Host} =a");
- CHECK_EQ(p.getTokens().size(), 4);
+
+ CHECK_EQ(p.getTokens().size(), 4UL);
CHECK_EQ(p.getTokens()[0], "cond");
CHECK_EQ(p.getTokens()[1], "%{CLIENT-HEADER:Host}");
CHECK_EQ(p.getTokens()[2], "=");
CHECK_EQ(p.getTokens()[3], "a");
+
+ END_TEST();
}
{
ParserTest p(" # COMMENT!");
- CHECK_EQ(p.getTokens().size(), 0);
+
+ CHECK_EQ(p.getTokens().size(), 0UL);
CHECK_EQ(p.empty(), true);
+
+ END_TEST();
}
{
ParserTest p("# COMMENT");
- CHECK_EQ(p.getTokens().size(), 0);
+
+ CHECK_EQ(p.getTokens().size(), 0UL);
CHECK_EQ(p.empty(), true);
+
+ END_TEST();
}
{
ParserTest p("cond %{Client-HEADER:Foo} =b");
- CHECK_EQ(p.getTokens().size(), 4);
+
+ CHECK_EQ(p.getTokens().size(), 4UL);
CHECK_EQ(p.getTokens()[0], "cond");
CHECK_EQ(p.getTokens()[1], "%{Client-HEADER:Foo}");
CHECK_EQ(p.getTokens()[2], "=");
CHECK_EQ(p.getTokens()[3], "b");
+
+ END_TEST();
}
{
ParserTest p("cond %{Client-HEADER:Blah} = x");
- CHECK_EQ(p.getTokens().size(), 4);
+
+ CHECK_EQ(p.getTokens().size(), 4UL);
CHECK_EQ(p.getTokens()[0], "cond");
CHECK_EQ(p.getTokens()[1], "%{Client-HEADER:Blah}");
CHECK_EQ(p.getTokens()[2], "=");
CHECK_EQ(p.getTokens()[3], "x");
+
+ END_TEST();
}
{
ParserTest p("cond %{CLIENT-HEADER:non_existent_header} = \"shouldnt_ exist _anyway\" [AND]");
- CHECK_EQ(p.getTokens().size(), 5);
+
+ CHECK_EQ(p.getTokens().size(), 5UL);
CHECK_EQ(p.getTokens()[0], "cond");
CHECK_EQ(p.getTokens()[1], "%{CLIENT-HEADER:non_existent_header}");
CHECK_EQ(p.getTokens()[2], "=");
CHECK_EQ(p.getTokens()[3], "shouldnt_ exist _anyway");
CHECK_EQ(p.getTokens()[4], "[AND]");
+
+ END_TEST();
}
{
ParserTest p("cond %{CLIENT-HEADER:non_existent_header} = \"shouldnt_ = _anyway\" [AND]");
- CHECK_EQ(p.getTokens().size(), 5);
+
+ CHECK_EQ(p.getTokens().size(), 5UL);
CHECK_EQ(p.getTokens()[0], "cond");
CHECK_EQ(p.getTokens()[1], "%{CLIENT-HEADER:non_existent_header}");
CHECK_EQ(p.getTokens()[2], "=");
CHECK_EQ(p.getTokens()[3], "shouldnt_ = _anyway");
CHECK_EQ(p.getTokens()[4], "[AND]");
+
+ END_TEST();
}
{
ParserTest p("cond %{CLIENT-HEADER:non_existent_header} =\"=\" [AND]");
- CHECK_EQ(p.getTokens().size(), 5);
+
+ CHECK_EQ(p.getTokens().size(), 5UL);
CHECK_EQ(p.getTokens()[0], "cond");
CHECK_EQ(p.getTokens()[1], "%{CLIENT-HEADER:non_existent_header}");
CHECK_EQ(p.getTokens()[2], "=");
CHECK_EQ(p.getTokens()[3], "=");
CHECK_EQ(p.getTokens()[4], "[AND]");
+
+ END_TEST();
}
{
ParserTest p("cond %{CLIENT-HEADER:non_existent_header} =\"\" [AND]");
- CHECK_EQ(p.getTokens().size(), 5);
+
+ CHECK_EQ(p.getTokens().size(), 5UL);
CHECK_EQ(p.getTokens()[0], "cond");
CHECK_EQ(p.getTokens()[1], "%{CLIENT-HEADER:non_existent_header}");
CHECK_EQ(p.getTokens()[2], "=");
CHECK_EQ(p.getTokens()[3], "");
CHECK_EQ(p.getTokens()[4], "[AND]");
+
+ END_TEST();
}
{
ParserTest p("add-header X-HeaderRewriteApplied true");
- CHECK_EQ(p.getTokens().size(), 3);
+
+ CHECK_EQ(p.getTokens().size(), 3UL);
CHECK_EQ(p.getTokens()[0], "add-header");
CHECK_EQ(p.getTokens()[1], "X-HeaderRewriteApplied");
CHECK_EQ(p.getTokens()[2], "true");
+
+ END_TEST();
}
/* backslash-escape */
{
ParserTest p("add-header foo \\ \\=\\<\\>\\\"\\#\\\\");
- CHECK_EQ(p.getTokens().size(), 3);
+
+ CHECK_EQ(p.getTokens().size(), 3UL);
CHECK_EQ(p.getTokens()[0], "add-header");
CHECK_EQ(p.getTokens()[1], "foo");
CHECK_EQ(p.getTokens()[2], " =<>\"#\\");
+
+ END_TEST();
}
{
ParserTest p("add-header foo \\<bar\\>");
- CHECK_EQ(p.getTokens().size(), 3);
+
+ CHECK_EQ(p.getTokens().size(), 3UL);
CHECK_EQ(p.getTokens()[0], "add-header");
CHECK_EQ(p.getTokens()[1], "foo");
CHECK_EQ(p.getTokens()[2], "<bar>");
+
+ END_TEST();
}
{
ParserTest p("add-header foo \\bar\\");
- CHECK_EQ(p.getTokens().size(), 3);
+
+ CHECK_EQ(p.getTokens().size(), 3UL);
CHECK_EQ(p.getTokens()[0], "add-header");
CHECK_EQ(p.getTokens()[1], "foo");
CHECK_EQ(p.getTokens()[2], "bar");
+
+ END_TEST();
}
{
ParserTest p("add-header foo \"bar\"");
- CHECK_EQ(p.getTokens().size(), 3);
+
+ CHECK_EQ(p.getTokens().size(), 3UL);
CHECK_EQ(p.getTokens()[0], "add-header");
CHECK_EQ(p.getTokens()[1], "foo");
CHECK_EQ(p.getTokens()[2], "bar");
+
+ END_TEST();
}
{
ParserTest p("add-header foo \"\\\"bar\\\"\"");
- CHECK_EQ(p.getTokens().size(), 3);
+
+ CHECK_EQ(p.getTokens().size(), 3UL);
CHECK_EQ(p.getTokens()[0], "add-header");
CHECK_EQ(p.getTokens()[1], "foo");
CHECK_EQ(p.getTokens()[2], "\"bar\"");
+
+ END_TEST();
}
{
ParserTest p("add-header foo \"\\\"\\\\\\\"bar\\\\\\\"\\\"\"");
- CHECK_EQ(p.getTokens().size(), 3);
+
+ CHECK_EQ(p.getTokens().size(), 3UL);
CHECK_EQ(p.getTokens()[0], "add-header");
CHECK_EQ(p.getTokens()[1], "foo");
CHECK_EQ(p.getTokens()[2], "\"\\\"bar\\\"\"");
+
+ END_TEST();
}
{
ParserTest p("add-header Public-Key-Pins \"max-age=3000; pin-sha256=\\\"d6qzRu9zOECb90Uez27xWltNsj0e1Md7GkYYkVoZWmM=\\\"\"");
- CHECK_EQ(p.getTokens().size(), 3);
+
+ CHECK_EQ(p.getTokens().size(), 3UL);
CHECK_EQ(p.getTokens()[0], "add-header");
CHECK_EQ(p.getTokens()[1], "Public-Key-Pins");
CHECK_EQ(p.getTokens()[2], "max-age=3000; pin-sha256=\"d6qzRu9zOECb90Uez27xWltNsj0e1Md7GkYYkVoZWmM=\"");
+
+ END_TEST();
}
{
- ParserTest p(
- "add-header Public-Key-Pins max-age\\=3000;\\ pin-sha256\\=\\\"d6qzRu9zOECb90Uez27xWltNsj0e1Md7GkYYkVoZWmM\\=\\\"");
- CHECK_EQ(p.getTokens().size(), 3);
+ ParserTest p("add-header Public-Key-Pins max-age\\=3000;"
+ "\\ pin-sha256\\=\\\"d6qzRu9zOECb90Uez27xWltNsj0e1Md7GkYYkVoZWmM\\=\\\"");
+
+ CHECK_EQ(p.getTokens().size(), 3UL);
CHECK_EQ(p.getTokens()[0], "add-header");
CHECK_EQ(p.getTokens()[1], "Public-Key-Pins");
CHECK_EQ(p.getTokens()[2], "max-age=3000; pin-sha256=\"d6qzRu9zOECb90Uez27xWltNsj0e1Md7GkYYkVoZWmM=\"");
+
+ END_TEST();
+ }
+
+ {
+ ParserTest p("add-header Client-IP \"%<chi>\"");
+
+ CHECK_EQ(p.getTokens().size(), 3UL);
+ CHECK_EQ(p.getTokens()[0], "add-header");
+ CHECK_EQ(p.getTokens()[1], "Client-IP");
+ CHECK_EQ(p.getTokens()[2], "\%<chi>");
+
+ END_TEST();
+ }
+
+// This can not succeed as is now, have to quote the "%<proto>"
--- End diff --
Not sure I understand, this check breaks now? There is an incompatible change since 6.x, where just %<chi> no longer works. I can remove this check.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1133: TS-4993: Disables escaping and quotes insi...
Posted by bryancall <gi...@git.apache.org>.
Github user bryancall commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1133#discussion_r84724447
--- Diff: plugins/header_rewrite/header_rewrite_test.cc ---
@@ -31,274 +33,395 @@ const char PLUGIN_NAME_DBG[] = "TEST_dbg_header_rewrite";
extern "C" void
TSError(const char *fmt, ...)
{
- char buf[2048];
- int bytes = 0;
- va_list args;
- va_start(args, fmt);
- if ((bytes = vsnprintf(buf, sizeof(buf), fmt, args)) > 0) {
- fprintf(stderr, "TSError: %s: %.*s\n", PLUGIN_NAME, bytes, buf);
- }
- va_end(args);
}
-extern "C" void
-TSDebug(const char *tag, const char *fmt, ...)
-{
- char buf[2048];
- int bytes = 0;
- va_list args;
- va_start(args, fmt);
- if ((bytes = vsnprintf(buf, sizeof(buf), fmt, args)) > 0) {
- fprintf(stdout, "TSDebug: %s: %.*s\n", PLUGIN_NAME, bytes, buf);
- }
- va_end(args);
-}
-
-#define CHECK_EQ(x, y) \
- do { \
- if ((x) != (y)) { \
- fprintf(stderr, "CHECK FAILED\n"); \
- return 1; \
- } \
- } while (false);
-
class ParserTest : public Parser
{
public:
- ParserTest(std::string line) : Parser(line) {}
+ ParserTest(std::string line) : Parser(line), res(true) { std::cout << "Starting parser test: " << line << std::endl; }
std::vector<std::string>
getTokens()
{
return _tokens;
}
+
+ template <typename T, typename U>
+ void
+ do_parser_check(T x, U y, int line = 0)
+ {
+ if (x != y) {
+ std::cerr << "CHECK FAILED on line " << line << ": " << x << " != " << y << std::endl;
+ res = false;
+ }
+ }
+
+ bool res;
};
+#define CHECK_EQ(x, y) \
+ do { \
+ p.do_parser_check((x), (y), __LINE__); \
+ } while (false);
+
+#define END_TEST(s) \
+ do { \
+ if (!p.res) { \
+ ++errors; \
+ } \
+ } while (false);
+
int
test_parsing()
{
+ int errors = 0;
+
{
ParserTest p("cond %{READ_REQUEST_HDR_HOOK}");
- CHECK_EQ(p.getTokens().size(), 2);
+
+ CHECK_EQ(p.getTokens().size(), 2U);
CHECK_EQ(p.getTokens()[0], "cond");
CHECK_EQ(p.getTokens()[1], "%{READ_REQUEST_HDR_HOOK}");
+
+ END_TEST();
}
{
ParserTest p("cond %{CLIENT-HEADER:Host} =a");
- CHECK_EQ(p.getTokens().size(), 4);
+
+ CHECK_EQ(p.getTokens().size(), 4UL);
CHECK_EQ(p.getTokens()[0], "cond");
CHECK_EQ(p.getTokens()[1], "%{CLIENT-HEADER:Host}");
CHECK_EQ(p.getTokens()[2], "=");
CHECK_EQ(p.getTokens()[3], "a");
+
+ END_TEST();
}
{
ParserTest p(" # COMMENT!");
- CHECK_EQ(p.getTokens().size(), 0);
+
+ CHECK_EQ(p.getTokens().size(), 0UL);
CHECK_EQ(p.empty(), true);
+
+ END_TEST();
}
{
ParserTest p("# COMMENT");
- CHECK_EQ(p.getTokens().size(), 0);
+
+ CHECK_EQ(p.getTokens().size(), 0UL);
CHECK_EQ(p.empty(), true);
+
+ END_TEST();
}
{
ParserTest p("cond %{Client-HEADER:Foo} =b");
- CHECK_EQ(p.getTokens().size(), 4);
+
+ CHECK_EQ(p.getTokens().size(), 4UL);
CHECK_EQ(p.getTokens()[0], "cond");
CHECK_EQ(p.getTokens()[1], "%{Client-HEADER:Foo}");
CHECK_EQ(p.getTokens()[2], "=");
CHECK_EQ(p.getTokens()[3], "b");
+
+ END_TEST();
}
{
ParserTest p("cond %{Client-HEADER:Blah} = x");
- CHECK_EQ(p.getTokens().size(), 4);
+
+ CHECK_EQ(p.getTokens().size(), 4UL);
CHECK_EQ(p.getTokens()[0], "cond");
CHECK_EQ(p.getTokens()[1], "%{Client-HEADER:Blah}");
CHECK_EQ(p.getTokens()[2], "=");
CHECK_EQ(p.getTokens()[3], "x");
+
+ END_TEST();
}
{
ParserTest p("cond %{CLIENT-HEADER:non_existent_header} = \"shouldnt_ exist _anyway\" [AND]");
- CHECK_EQ(p.getTokens().size(), 5);
+
+ CHECK_EQ(p.getTokens().size(), 5UL);
CHECK_EQ(p.getTokens()[0], "cond");
CHECK_EQ(p.getTokens()[1], "%{CLIENT-HEADER:non_existent_header}");
CHECK_EQ(p.getTokens()[2], "=");
CHECK_EQ(p.getTokens()[3], "shouldnt_ exist _anyway");
CHECK_EQ(p.getTokens()[4], "[AND]");
+
+ END_TEST();
}
{
ParserTest p("cond %{CLIENT-HEADER:non_existent_header} = \"shouldnt_ = _anyway\" [AND]");
- CHECK_EQ(p.getTokens().size(), 5);
+
+ CHECK_EQ(p.getTokens().size(), 5UL);
CHECK_EQ(p.getTokens()[0], "cond");
CHECK_EQ(p.getTokens()[1], "%{CLIENT-HEADER:non_existent_header}");
CHECK_EQ(p.getTokens()[2], "=");
CHECK_EQ(p.getTokens()[3], "shouldnt_ = _anyway");
CHECK_EQ(p.getTokens()[4], "[AND]");
+
+ END_TEST();
}
{
ParserTest p("cond %{CLIENT-HEADER:non_existent_header} =\"=\" [AND]");
- CHECK_EQ(p.getTokens().size(), 5);
+
+ CHECK_EQ(p.getTokens().size(), 5UL);
CHECK_EQ(p.getTokens()[0], "cond");
CHECK_EQ(p.getTokens()[1], "%{CLIENT-HEADER:non_existent_header}");
CHECK_EQ(p.getTokens()[2], "=");
CHECK_EQ(p.getTokens()[3], "=");
CHECK_EQ(p.getTokens()[4], "[AND]");
+
+ END_TEST();
}
{
ParserTest p("cond %{CLIENT-HEADER:non_existent_header} =\"\" [AND]");
- CHECK_EQ(p.getTokens().size(), 5);
+
+ CHECK_EQ(p.getTokens().size(), 5UL);
CHECK_EQ(p.getTokens()[0], "cond");
CHECK_EQ(p.getTokens()[1], "%{CLIENT-HEADER:non_existent_header}");
CHECK_EQ(p.getTokens()[2], "=");
CHECK_EQ(p.getTokens()[3], "");
CHECK_EQ(p.getTokens()[4], "[AND]");
+
+ END_TEST();
}
{
ParserTest p("add-header X-HeaderRewriteApplied true");
- CHECK_EQ(p.getTokens().size(), 3);
+
+ CHECK_EQ(p.getTokens().size(), 3UL);
CHECK_EQ(p.getTokens()[0], "add-header");
CHECK_EQ(p.getTokens()[1], "X-HeaderRewriteApplied");
CHECK_EQ(p.getTokens()[2], "true");
+
+ END_TEST();
}
/* backslash-escape */
{
ParserTest p("add-header foo \\ \\=\\<\\>\\\"\\#\\\\");
- CHECK_EQ(p.getTokens().size(), 3);
+
+ CHECK_EQ(p.getTokens().size(), 3UL);
CHECK_EQ(p.getTokens()[0], "add-header");
CHECK_EQ(p.getTokens()[1], "foo");
CHECK_EQ(p.getTokens()[2], " =<>\"#\\");
+
+ END_TEST();
}
{
ParserTest p("add-header foo \\<bar\\>");
- CHECK_EQ(p.getTokens().size(), 3);
+
+ CHECK_EQ(p.getTokens().size(), 3UL);
CHECK_EQ(p.getTokens()[0], "add-header");
CHECK_EQ(p.getTokens()[1], "foo");
CHECK_EQ(p.getTokens()[2], "<bar>");
+
+ END_TEST();
}
{
ParserTest p("add-header foo \\bar\\");
- CHECK_EQ(p.getTokens().size(), 3);
+
+ CHECK_EQ(p.getTokens().size(), 3UL);
CHECK_EQ(p.getTokens()[0], "add-header");
CHECK_EQ(p.getTokens()[1], "foo");
CHECK_EQ(p.getTokens()[2], "bar");
+
+ END_TEST();
}
{
ParserTest p("add-header foo \"bar\"");
- CHECK_EQ(p.getTokens().size(), 3);
+
+ CHECK_EQ(p.getTokens().size(), 3UL);
CHECK_EQ(p.getTokens()[0], "add-header");
CHECK_EQ(p.getTokens()[1], "foo");
CHECK_EQ(p.getTokens()[2], "bar");
+
+ END_TEST();
}
{
ParserTest p("add-header foo \"\\\"bar\\\"\"");
- CHECK_EQ(p.getTokens().size(), 3);
+
+ CHECK_EQ(p.getTokens().size(), 3UL);
CHECK_EQ(p.getTokens()[0], "add-header");
CHECK_EQ(p.getTokens()[1], "foo");
CHECK_EQ(p.getTokens()[2], "\"bar\"");
+
+ END_TEST();
}
{
ParserTest p("add-header foo \"\\\"\\\\\\\"bar\\\\\\\"\\\"\"");
- CHECK_EQ(p.getTokens().size(), 3);
+
+ CHECK_EQ(p.getTokens().size(), 3UL);
CHECK_EQ(p.getTokens()[0], "add-header");
CHECK_EQ(p.getTokens()[1], "foo");
CHECK_EQ(p.getTokens()[2], "\"\\\"bar\\\"\"");
+
+ END_TEST();
}
{
ParserTest p("add-header Public-Key-Pins \"max-age=3000; pin-sha256=\\\"d6qzRu9zOECb90Uez27xWltNsj0e1Md7GkYYkVoZWmM=\\\"\"");
- CHECK_EQ(p.getTokens().size(), 3);
+
+ CHECK_EQ(p.getTokens().size(), 3UL);
CHECK_EQ(p.getTokens()[0], "add-header");
CHECK_EQ(p.getTokens()[1], "Public-Key-Pins");
CHECK_EQ(p.getTokens()[2], "max-age=3000; pin-sha256=\"d6qzRu9zOECb90Uez27xWltNsj0e1Md7GkYYkVoZWmM=\"");
+
+ END_TEST();
}
{
- ParserTest p(
- "add-header Public-Key-Pins max-age\\=3000;\\ pin-sha256\\=\\\"d6qzRu9zOECb90Uez27xWltNsj0e1Md7GkYYkVoZWmM\\=\\\"");
- CHECK_EQ(p.getTokens().size(), 3);
+ ParserTest p("add-header Public-Key-Pins max-age\\=3000;"
+ "\\ pin-sha256\\=\\\"d6qzRu9zOECb90Uez27xWltNsj0e1Md7GkYYkVoZWmM\\=\\\"");
+
+ CHECK_EQ(p.getTokens().size(), 3UL);
CHECK_EQ(p.getTokens()[0], "add-header");
CHECK_EQ(p.getTokens()[1], "Public-Key-Pins");
CHECK_EQ(p.getTokens()[2], "max-age=3000; pin-sha256=\"d6qzRu9zOECb90Uez27xWltNsj0e1Md7GkYYkVoZWmM=\"");
+
+ END_TEST();
+ }
+
+ {
+ ParserTest p("add-header Client-IP \"%<chi>\"");
+
+ CHECK_EQ(p.getTokens().size(), 3UL);
+ CHECK_EQ(p.getTokens()[0], "add-header");
+ CHECK_EQ(p.getTokens()[1], "Client-IP");
+ CHECK_EQ(p.getTokens()[2], "\%<chi>");
+
+ END_TEST();
+ }
+
+// This can not succeed as is now, have to quote the "%<proto>"
--- End diff --
Is breaking this test going to cause an issue?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1133: TS-4993: Disables escaping and quotes inside rege...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/1133
Randall and I ran into another issue with this patch, so doing some more debugging :-/.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1133: TS-4993: Disables escaping and quotes insi...
Posted by bryancall <gi...@git.apache.org>.
Github user bryancall commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1133#discussion_r84726802
--- Diff: plugins/header_rewrite/parser.cc ---
@@ -133,14 +152,15 @@ Parser::preprocess(std::vector<std::string> tokens)
std::string s = tokens[0].substr(2, tokens[0].size() - 3);
_op = s;
- if (tokens.size() > 2 && (tokens[1][0] == '=' || tokens[1][0] == '>' || tokens[1][0] == '<')) { // cond + (=/</>) + argument
+ if (tokens.size() > 2 && (tokens[1][0] == '=' || tokens[1][0] == '>' || tokens[1][0] == '<')) {
+ // cond + [=<>] + argument
--- End diff --
Remove this?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1133: TS-4993: Disables escaping and quotes insi...
Posted by igalic <gi...@git.apache.org>.
Github user igalic commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1133#discussion_r84887560
--- Diff: plugins/header_rewrite/header_rewrite_test.cc ---
@@ -31,274 +33,395 @@ const char PLUGIN_NAME_DBG[] = "TEST_dbg_header_rewrite";
extern "C" void
TSError(const char *fmt, ...)
{
- char buf[2048];
- int bytes = 0;
- va_list args;
- va_start(args, fmt);
- if ((bytes = vsnprintf(buf, sizeof(buf), fmt, args)) > 0) {
- fprintf(stderr, "TSError: %s: %.*s\n", PLUGIN_NAME, bytes, buf);
- }
- va_end(args);
--- End diff --
you may want to replace the functions content then with this ^ `/* comment */`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1133: TS-4993: Disables escaping and quotes insi...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1133#discussion_r84770538
--- Diff: plugins/header_rewrite/parser.cc ---
@@ -133,14 +152,15 @@ Parser::preprocess(std::vector<std::string> tokens)
std::string s = tokens[0].substr(2, tokens[0].size() - 3);
_op = s;
- if (tokens.size() > 2 && (tokens[1][0] == '=' || tokens[1][0] == '>' || tokens[1][0] == '<')) { // cond + (=/</>) + argument
+ if (tokens.size() > 2 && (tokens[1][0] == '=' || tokens[1][0] == '>' || tokens[1][0] == '<')) {
+ // cond + [=<>] + argument
--- End diff --
Remove what?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1133: TS-4993: Disables escaping and quotes inside rege...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1133
Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/967/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1133: TS-4993: Disables escaping and quotes insi...
Posted by bryancall <gi...@git.apache.org>.
Github user bryancall commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1133#discussion_r84726964
--- Diff: plugins/header_rewrite/parser.cc ---
@@ -133,14 +152,15 @@ Parser::preprocess(std::vector<std::string> tokens)
std::string s = tokens[0].substr(2, tokens[0].size() - 3);
_op = s;
- if (tokens.size() > 2 && (tokens[1][0] == '=' || tokens[1][0] == '>' || tokens[1][0] == '<')) { // cond + (=/</>) + argument
+ if (tokens.size() > 2 && (tokens[1][0] == '=' || tokens[1][0] == '>' || tokens[1][0] == '<')) {
+ // cond + [=<>] + argument
_arg = tokens[1] + tokens[2];
} else if (tokens.size() > 1) {
+ // This is for the regular expression, which for some reason has its own handling?? ToDo: Why ?
--- End diff --
Don't understand and have to catch a flight.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1133: TS-4993: Disables escaping and quotes inside rege...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1133
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1075/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1133: TS-4993: Disables escaping and quotes inside rege...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1133
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1067/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1133: TS-4993: Disables escaping and quotes insi...
Posted by igalic <gi...@git.apache.org>.
Github user igalic commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1133#discussion_r84601766
--- Diff: plugins/header_rewrite/header_rewrite_test.cc ---
@@ -31,274 +33,395 @@ const char PLUGIN_NAME_DBG[] = "TEST_dbg_header_rewrite";
extern "C" void
TSError(const char *fmt, ...)
{
- char buf[2048];
- int bytes = 0;
- va_list args;
- va_start(args, fmt);
- if ((bytes = vsnprintf(buf, sizeof(buf), fmt, args)) > 0) {
- fprintf(stderr, "TSError: %s: %.*s\n", PLUGIN_NAME, bytes, buf);
- }
- va_end(args);
--- End diff --
how does this function work now?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1133: TS-4993: Disables escaping and quotes inside rege...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1133
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1074/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1133: TS-4993: Disables escaping and quotes insi...
Posted by bryancall <gi...@git.apache.org>.
Github user bryancall closed the pull request at:
https://github.com/apache/trafficserver/pull/1133
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1133: TS-4993: Disables escaping and quotes insi...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1133#discussion_r84723539
--- Diff: plugins/header_rewrite/header_rewrite_test.cc ---
@@ -31,274 +33,395 @@ const char PLUGIN_NAME_DBG[] = "TEST_dbg_header_rewrite";
extern "C" void
TSError(const char *fmt, ...)
{
- char buf[2048];
- int bytes = 0;
- va_list args;
- va_start(args, fmt);
- if ((bytes = vsnprintf(buf, sizeof(buf), fmt, args)) > 0) {
- fprintf(stderr, "TSError: %s: %.*s\n", PLUGIN_NAME, bytes, buf);
- }
- va_end(args);
--- End diff --
So, the issue was that there are two test cases that are intended to fail. When running this from command line (to verify / analyze tests), it would therefore always produce two errors on the output. I found this confusing as hell, thinking that the parser was actually failing (which it was not). So, I removed this, so it produces no output, and instead the test now properly detect the errors, and produce more useful information when they do fail unexpectedly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1133: TS-4993: Disables escaping and quotes inside rege...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on the issue:
https://github.com/apache/trafficserver/pull/1133
@zwoop This would have been better if it was split into multiple commits to separate the test changes from the actual bug fixes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---