headers: Add support for deleting all headers as one operation

[heimoshuiyu]

Related issue #5451

This PR recognizes the syntax header_up -* and removes all headers. After removing all headers, any desired headers can be added.

Here is an example Caddyfile:

:9000 {
    reverse_proxy * http://127.0.0.1:9001 {
        header_up -*
        header_up authorization {http.request.header.authorization}
    }
}

This example removes all headers sent by the client except for authorization.

As I am not familiar with the code of Caddy, I am concerned that changing the order of the header manipulation could affect other programs. Please help me review this PR :)

[CLAassistant]

All committers have signed the CLA.

[francislavoie]

Hmm, this'll also affect the header directive since the same code is used outside of the proxy. This would likely break a usecase like this:

header {
	-Foo
	FooBar {header.Foo}
}

Previously, it would take the value from Foo before deleting Foo, but now it will delete Foo first which would make the placeholder return an empty value (I didn't run it though so that might need confirming).

[heimoshuiyu]

Hmm, this'll also affect the header directive since the same code is used outside of the proxy. This would likely break a usecase like this:

header {
	-Foo
	FooBar {header.Foo}
}

Previously, it would take the value from Foo before deleting Foo, but now it will delete Foo first which would make the placeholder return an empty value (I didn't run it though so that might need confirming).

@francislavoie Thank you for providing the test example, but surprisingly, it still works as expected in this case. This seems to be because the - symbol removes the headers in the response, but it can still retrieve headers from the request. Here is the Caddyfile I used:

:9000 {
    reverse_proxy * http://127.0.0.1:9001 {
        header_up -Foo
        header_up FooBar {header.Foo}
    }
}

Then I used nc -l 9001 to listen on port 9001. I executed curl http://127.0.0.1:9000 -H "foo: value". The nc (as upstream of caddy) output the following information:

GET / HTTP/1.1
Host: localhost:9000
User-Agent: curl/7.88.1
Accept: */*
Foobar: value
X-Forwarded-For: 127.0.0.1
X-Forwarded-Host: localhost:9000
X-Forwarded-Proto: http
Accept-Encoding: gzip

---

Then, I tested another case

:9000 {
    root * /usr/share/caddy
    file_server
    header {
        -Foo
        FooBar {header.Foo}
    }
}

The output of curl http://127.0.0.1:9000 -I -H "foo: value" is:

HTTP/1.1 200 OK
Accept-Ranges: bytes
Content-Length: 12214
Content-Type: text/html; charset=utf-8
Etag: "rqlgyd9fa"
Foobar: value
Last-Modified: Fri, 24 Feb 2023 17:08:37 GMT
Server: Caddy
Date: Mon, 27 Mar 2023 13:19:17 GMT

[francislavoie]

Oh sorry, yeah. I meant the request_header directive, but that's a less common usecase. I wrote that too late last night 😶

[heimoshuiyu]

Oh sorry, yeah. I meant the request_header directive, but that's a less common usecase. I wrote that too late last night no_mouth

I tested the request_header directive and obtained the same results using both v2.6.4 and the version with this PR modification 🤔.

Caddyfile

:9000 {
    request_header -Foo
    request_header FooBar {header.Foo}
    reverse_proxy * http://127.0.0.1:9001
}

Command

nc -l 9001
curl http:127.0.0.1:9000/ -H 'Foo: value' -v

Result from nc -l 9001

HEAD / HTTP/1.1
Host: localhost:9000
User-Agent: curl/7.88.1
Accept: */*
Foobar: 
X-Forwarded-For: 127.0.0.1
X-Forwarded-Host: localhost:9000
X-Forwarded-Proto: http

By switching the order it works 🤔

:9000 {
    request_header FooBar {header.Foo}
    request_header -Foo
    reverse_proxy * http://127.0.0.1:9001
}

Result

HEAD / HTTP/1.1
Host: localhost:9000
User-Agent: curl/7.88.1
Accept: */*
Foobar: value
X-Forwarded-For: 127.0.0.1
X-Forwarded-Host: localhost:9000
X-Forwarded-Proto: http

It appears that the execution order of request_header is determined by the order in which it is wroted in the configuration file.

[heimoshuiyu]

However, I am still concerned about potentially breaking something else. I have come up with a better approach: treating -* as a special case that is executed before adding any headers, but without changing the overall order of header execution.

This should not break anything, as the -* syntax was not parseable by Caddy before this change.

[francislavoie]

request_header -Foo
request_header FooBar {header.Foo}

That's cause that's two separate handler invocations instead of one handler with two operations.

Did we not implement multiple operations for request headers in the Caddyfile? 🤷‍♂️

This might be fine.

[mholt]

Thanks for the patch!

This LGTM -- I appreciate the careful consideration. Initially I too was thinking we just move the Delete logic up to be first, but I can see how this is probably safer and more relevant.

I just submitted suggestions to improve the comments. If you agree let's go ahead and accept those changes before merging. Thanks!