[ 
https://issues.apache.org/jira/browse/TS-4993?focusedWorklogId=30983&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-30983
 ]

ASF GitHub Bot logged work on TS-4993:
--------------------------------------

                Author: ASF GitHub Bot
            Created on: 24/Oct/16 16:27
            Start Date: 24/Oct/16 16:27
    Worklog Time Spent: 10m 
      Work Description: 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?


Issue Time Tracking
-------------------

    Worklog Id:     (was: 30983)
    Time Spent: 50m  (was: 40m)

> backslash/escape removed from header_rewrite rule when unquoted
> ---------------------------------------------------------------
>
>                 Key: TS-4993
>                 URL: https://issues.apache.org/jira/browse/TS-4993
>             Project: Traffic Server
>          Issue Type: Bug
>          Components: Plugins
>    Affects Versions: 7.0.0
>            Reporter: Randall Meyer
>            Assignee: Leif Hedstrom
>             Fix For: 7.1.0
>
>          Time Spent: 50m
>  Remaining Estimate: 0h
>
> Loading up a configuration with a header_rewrite rule of:
> {noformat}
> cond %{READ_RESPONSE_HDR_HOOK} [AND]
> cond %{PATH} /(\.html|\.js|\.png)(?:\?(.*))*$/ [AND]
> cond %{STATUS} >199 [AND]
> cond %{STATUS} <300
>   set-header Cache-Control "max-age=31536000, public
> {noformat}
> results in an call to abort() in matcher.h under ATS 7.0.0. This worked fine 
> under ATS 6.x (and probably 5.3.x)
> {noformat}
> (gdb) where
> #0  0x00007ffff4f64625 in *__GI_raise (sig=<optimized out>) at 
> ../nptl/sysdeps/unix/sysv/linux/raise.c:64
> #1  0x00007ffff4f65e05 in *__GI_abort () at abort.c:92
> #2  0x00007fffe71dd3ff in Matchers<std::string>::setRegex 
> (this=this@entry=0x7fffe5ac0f80) at matcher.h:98
> #3  0x00007fffe71d7baa in Matchers<std::string>::set (d=..., 
> this=0x7fffe5ac0f80) at matcher.h:119
> #4  ConditionPath::initialize (this=0x7fffe59a6d00, p=...) at 
> conditions.cc:260
> {noformat}
> The string comes to matcher with the escapes removed:
> {noformat}
> Adding condition: %{PATH} with arg: /(.html|.js|.png)(?:?(.*))*$/
> {noformat}
> If I add quotes around the regex, this regex is passed through correctly 
> escaped.
> Not sure if this is expected behavior or not. 
> This also seems related to TS-4797 and TS-4940. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to