You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by zwoop <gi...@git.apache.org> on 2016/06/23 18:40:33 UTC
[GitHub] trafficserver pull request #735: TS-4449 header_rewrite: Better errors and d...
GitHub user zwoop opened a pull request:
https://github.com/apache/trafficserver/pull/735
TS-4449 header_rewrite: Better errors and debug output
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zwoop/trafficserver TS-4449
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafficserver/pull/735.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 #735
----
commit aa67ba46f6e6bfe0193128eae12f8ee2c954770b
Author: Leif Hedstrom <zw...@apache.org>
Date: 2016-05-17T18:20:32Z
TS-4449 Cleanup of coding style
commit 857278154f62043bbecb1fd59b8c898a823aa65f
Author: Leif Hedstrom <zw...@apache.org>
Date: 2016-05-17T19:12:19Z
TS-4449 Better errors and debug output
This does a few things:
1. Better error reports when parsing a config file
2. Better diagnostics when debugging expressions
I also added an example to the docs, for a common use case that
turns out to be non-obvious.
----
---
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 #735: TS-4449 header_rewrite: Better errors and d...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/735#discussion_r68300125
--- Diff: plugins/header_rewrite/conditions.cc ---
@@ -584,16 +585,17 @@ bool
ConditionIncomingPort::eval(const Resources &res)
{
uint16_t port = getPort(TSHttpTxnIncomingAddrGet(res.txnp));
- bool rval = static_cast<const Matchers<uint16_t> *>(_matcher)->test(port);
- TSDebug(PLUGIN_NAME, "Evaluating INCOMING-PORT(): %d: rval: %d", port, rval);
- return rval;
+
+ TSDebug(PLUGIN_NAME, "Evaluating INCOMING-PORT()");
--- End diff --
Print the port?
---
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 #735: TS-4449 header_rewrite: Better errors and debug ou...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/735
Linux build *failed*! See https://ci.trafficserver.apache.org/job/Github-Linux/239/ 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 #735: TS-4449 header_rewrite: Better errors and d...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/735#discussion_r68299556
--- Diff: plugins/header_rewrite/conditions.cc ---
@@ -86,9 +86,8 @@ ConditionMethod::eval(const Resources &res)
std::string s;
append_value(s, res);
- bool rval = static_cast<const Matchers<std::string> *>(_matcher)->test(s);
- TSDebug(PLUGIN_NAME, "Evaluating METHOD(): %s - rval: %d", s.c_str(), rval);
- return rval;
+ TSDebug(PLUGIN_NAME, "Evaluating METHOD()");
--- End diff --
Why did you drop the value from this debug message? The one below in ``ConditionHeader`` still has it ...
---
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 #735: TS-4449 header_rewrite: Better errors and debug ou...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/735
Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/225/ 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 #735: TS-4449 header_rewrite: Better errors and debug ou...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/735
I cleaned up a few of the missing pieces, made a get() method better (less STL copy on use), and added some typedef's as introduced in one of the operators (it makes sense and cleans up the usage in other places).
---
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 #735: TS-4449 header_rewrite: Better errors and debug ou...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on the issue:
https://github.com/apache/trafficserver/pull/735
Looks good; I just had a couple of trivial suggestions.
\U0001f44d
---
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 #735: TS-4449 header_rewrite: Better errors and d...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/735#discussion_r68301583
--- Diff: plugins/header_rewrite/regex_helper.h ---
@@ -36,11 +36,13 @@ class regexHelper
regexHelper() : regex(NULL), regexExtra(NULL), regexCcount(0) {}
~regexHelper()
{
- if (regex)
+ if (regex) {
pcre_free(regex);
+ }
- if (regexExtra)
+ if (regexExtra) {
pcre_free(regexExtra);
+ }
--- End diff --
FWIW, ``pcre_free`` should have the same semantics as ``free(3)`` so passing ``NULL`` is a no-op.
---
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 #735: TS-4449 header_rewrite: Better errors and debug ou...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/735
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/331/ 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 #735: TS-4449 header_rewrite: Better errors and d...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/735#discussion_r68303567
--- Diff: plugins/header_rewrite/conditions.cc ---
@@ -316,12 +314,12 @@ ConditionQuery::eval(const Resources &res)
{
std::string s;
+ TSDebug(PLUGIN_NAME, "Evaluating QUERY()");
if (NULL == res._rri) {
- TSDebug(PLUGIN_NAME, "QUERY requires remap initialization! Evaluating to false!");
+ TSDebug(PLUGIN_NAME, "\tQUERY requires remap initialization! Evaluating to false!");
--- End diff --
Maybe, but that might be a different bug to fix honestly. File a Jira? We shouldn't error out here I don't think.
---
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 #735: TS-4449 header_rewrite: Better errors and debug ou...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/735
Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/240/ 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 #735: TS-4449 header_rewrite: Better errors and debug ou...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/735
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/345/ 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 #735: TS-4449 header_rewrite: Better errors and d...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/735#discussion_r68303176
--- Diff: plugins/header_rewrite/conditions.cc ---
@@ -584,16 +585,17 @@ bool
ConditionIncomingPort::eval(const Resources &res)
{
uint16_t port = getPort(TSHttpTxnIncomingAddrGet(res.txnp));
- bool rval = static_cast<const Matchers<uint16_t> *>(_matcher)->test(port);
- TSDebug(PLUGIN_NAME, "Evaluating INCOMING-PORT(): %d: rval: %d", port, rval);
- return rval;
+
+ TSDebug(PLUGIN_NAME, "Evaluating INCOMING-PORT()");
--- End diff --
I think it prints that as part of the other code, when doing the matching.
---
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 #735: TS-4449 header_rewrite: Better errors and d...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop closed the pull request at:
https://github.com/apache/trafficserver/pull/735
---
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 #735: TS-4449 header_rewrite: Better errors and d...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/735#discussion_r68299677
--- Diff: plugins/header_rewrite/conditions.cc ---
@@ -316,12 +314,12 @@ ConditionQuery::eval(const Resources &res)
{
std::string s;
+ TSDebug(PLUGIN_NAME, "Evaluating QUERY()");
if (NULL == res._rri) {
- TSDebug(PLUGIN_NAME, "QUERY requires remap initialization! Evaluating to false!");
+ TSDebug(PLUGIN_NAME, "\tQUERY requires remap initialization! Evaluating to false!");
--- End diff --
This seems like a configuration problem. Maybe it should be a ``TSError``.
---
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.
---