TS-4797: Allow backslash-escape in header_rewrite rules

[masaori335]

• TS-4797

[atsci]

FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/660/ for details.

[atsci]

Linux build successful! See https://ci.trafficserver.apache.org/job/Github-Linux/555/ for details.

[zwoop]

👍

[jpeach]

Why the inquote condition? Why can't backquotes work outside of a quoted string?

[masaori335]

My commit message might be not suitable. What I want to fix is we can't use double quote inside of quoted string.

We can make this more general, but it makes header_rewrite rules and parser code more complicated.
Do you have any specific use cases to escape something outside of quoted string?

[maskit]

Without this change, the test below passes.

{                                                                                 
    ParserTest p("add-header foo \\var\\");                                         
    CHECK_EQ(p.getTokens().size(), 3);                                              
    CHECK_EQ(p.getTokens()[0], "add-header");                                                                                                                                        
    CHECK_EQ(p.getTokens()[1], "foo");                                              
    CHECK_EQ(p.getTokens()[2], "\\var\\");                                          
}

But with this change, it fails.
The test case will need to be changed like:

CHECK_EQ(p.getTokens()[2], "var\\");

This seems very odd. Even if there is no actual use case, it's hard to understand the behavior.

However, it seems we don't support backslash-escaping completely outside of a quoted string string now, actually.

// This doesn't work but it's OK, maybe.
{
  ParserTest p("add-header foo <bar>");
  CHECK_EQ(p.getTokens().size(), 3);
}

// This doesn't work even without Masaori's change. I guess this is the case James pointed out.
{
  ParserTest p("add-header foo \\<bar\\>");
  CHECK_EQ(p.getTokens().size(), 3);
}

// This works either way.
{                                                                              
  ParserTest p("add-header foo \"<bar>\"");
  CHECK_EQ(p.getTokens().size(), 3);
  CHECK_EQ(p.getTokens()[0], "add-header");
  CHECK_EQ(p.getTokens()[1], "foo");
  CHECK_EQ(p.getTokens()[2], "<bar>");
}

I don't know whether this is a regression.

Anyway, more conservative codes would be:

if (!extracting_token) {
  extracting_token = true;
  cur_token_start  = i;
}
if (inquote) {
    line.erase(i, 1);
    continue;
}

I think we should add all test cases above to clarify the cases now we support.

[maskit]

As I commented inline, this introduces an unexpected behavior. I suggest using the conservative change I wrote on the comment.

Also, it's a trivial thing but I'd suggest using "bar" instead of "var".

[atsci]

FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/696/ for details.

[atsci]

Linux build successful! See https://ci.trafficserver.apache.org/job/Github-Linux/592/ for details.

[masaori335]

Please take another look.
I added test cases which @maskit pointed out. And allow backslash-escape outside of quoted-string too.

[atsci]

Linux build successful! See https://ci.trafficserver.apache.org/job/Github-Linux/593/ for details.

[atsci]

FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/697/ for details.

[maskit]

👍