header_rewrite: set-destination path corruption

[bneradt]

@djcarlin noticed something interesting about the header_rewrite plugin with respect to its set-destination directive. He was working with a config file like this:

cond %{REMAP_PSEUDO_HOOK}
set-destination PATH /prefix/this/%{CLIENT-URL:PATH}

The documentation is not clear about whether the set-destination's path argument should or should not take the / prefix:

https://docs.trafficserver.apache.org/en/10.0.x/admin-guide/plugins/header_rewrite.en.html#header-rewrite-plugin

It does say, "URL substring beginning with (but not including) the first / after the hostname ", but how should that be applied? Does that mean the initial / should or should not be provided for the set-destination directive. All the cond PATH flavors include the initial /. In any case, he tried with the initial / first. The result of the above config is that a mysterious space gets inserted in the path. Thus, if the client had a path of /original/path, the result above the above set-destination directive is this:

[Apr  8 21:44:15.441] [ET_NET 0] DIAG: (header_rewrite) Building resources, hook=TS_HTTP_LAST_HOOK
[Apr  8 21:44:15.441] [ET_NET 0] DIAG: (header_rewrite)         Adding TXN client request header buffers
[Apr  8 21:44:15.441] [ET_NET 0] DIAG: (header_rewrite)         Adding TXN client request header buffers for remap instance
[Apr  8 21:44:15.441] [ET_NET 0] DIAG: (header_rewrite) Evaluating GROUP()
[Apr  8 21:44:15.441] [ET_NET 0] DIAG: (header_rewrite) Appending '/prepend/ this/' to evaluation value
[Apr  8 21:44:15.441] [ET_NET 0] DIAG: (header_rewrite)    Using the pristine url
[Apr  8 21:44:15.441] [ET_NET 0] DIAG: (header_rewrite)    Path to match is: original/path
[Apr  8 21:44:15.441] [ET_NET 0] DIAG: (header_rewrite) OperatorSetDestination::exec() invoked with PATH: /prepend/ this/original/path

Note the space inserted in the final path: /prepend/ this/original/path

This is obviously problematic. @djcarlin found that leaving off the initial / in the config file resolves the issue:

cond %{REMAP_PSEUDO_HOOK}
set-destination PATH prefix/this/%{CLIENT-URL:PATH}

The result is now the expected result:

[Apr  8 21:54:40.627] [ET_NET 0] DIAG: (header_rewrite) Building resources, hook=TS_HTTP_LAST_HOOK
[Apr  8 21:54:40.627] [ET_NET 0] DIAG: (header_rewrite)         Adding TXN client request header buffers
[Apr  8 21:54:40.627] [ET_NET 0] DIAG: (header_rewrite)         Adding TXN client request header buffers for remap instance
[Apr  8 21:54:40.627] [ET_NET 0] DIAG: (header_rewrite) Evaluating GROUP()
[Apr  8 21:54:40.627] [ET_NET 0] DIAG: (header_rewrite) Appending 'prepend/this/' to evaluation value
[Apr  8 21:54:40.627] [ET_NET 0] DIAG: (header_rewrite)    Using the pristine url
[Apr  8 21:54:40.627] [ET_NET 0] DIAG: (header_rewrite)    Path to match is: original/path
[Apr  8 21:54:40.627] [ET_NET 0] DIAG: (header_rewrite) OperatorSetDestination::exec() invoked with PATH: prepend/this/original/path

I have reproduced this in an autest and will next investigate the cause of this behavior and whether there is a tractible solution.

[bneradt]

I'm going to switch the assignee to @zwoop because my PR to address this was obsoleted by the improvements he made to header_rewrite. It is possible that his refactors addressed this issue.