apache / apache/trafficserver

header_rewrite: set-destination path corruption

Open
#12,173 1 comment 0 reactions 1 assignee Assigned to @zwoop View on GitHub
Bug header_rewrite
Dominant language
C++
Stars
2k
Forks
874
Avg merge
6d 15h
Merged PRs (30d)
46

Description

@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](https://github.com/bneradt/trafficserver/tree/header_rewrite_path_issue) and will next investigate the cause of this behavior and whether there is a tractible solution.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.