Repository: caddyserver/caddy (language: go)

Issue:
Weird Interactions between 103 Early Hints, Headers, and Reverse Proxy
I started playing around with the `103 Early Hints` implementation in Caddy 2.6.1 and found some odd behavior.

**Summary**

If the `103 Early Hints` is generated by Caddy:

- Caddy doesn’t remove the headers that it should remove from the reverse proxy response.
- Caddy adds the `Link` headers that pertain to the `103 Early Hints` response even to the reverse proxy response.

If the `103 Early Hints` is generated by the underlying application:

- Caddy processes headers only to the `103 Early Hints` early response, not to the main response.

**Details**

_Base Case (No Headers; No `103 Early Hints`)_

Let the underlying application be a “Hello World.” Without loss of generality I’ll use Express.js. If you wish to follow along:

```console
$ npm install @leafac/caddy express
```

```javascript
// index.mjs
import express from "express";
const app = express();
app.get("/", (req, res) => {
  res.send("Hello World");
});
app.listen(3000);
```

Configure Caddy as a reverse proxy:

```caddyfile
# Caddyfile
localhost
reverse_proxy localhost:3000
```

And run the following in separate terminals:

```console
$ node index.mjs
$ npx @leafac/caddy run
$ curl -sv "https://localhost/" > /dev/null
```

```
< HTTP/2 200
< alt-svc: h3=":443"; ma=2592000
< content-type: text/html; charset=utf-8
< date: Fri, 23 Sep 2022 17:44:24 GMT
< etag: W/"b-Ck1VqNd45QIvq3AZd8XYQLvEhtA"
< server: Caddy
< x-powered-by: Express
< content-length: 11
```

Note how Express.js & Caddy both generated identifying headers:

```
server: Caddy
x-powered-by: Express
```

---

_Adding & Removing Headers (Still no `103 Early Hints`)_

Change both the application & the Caddy configuration to add/remove custom headers:

```javascript
// index.mjs
import express from "express";
const app = express();
app.get("/", (req, res) => {
  res.header("Hello-From-Express", "true");
  res.send("Hello World");
});
app.listen(3000);
```

```caddyfile
# Caddyfile
localhost
header Hello-From-Caddy true
header -Server
header -X-Powered-By
reverse_proxy localhost:3000
```

Repeat the commands in different terminals:

```console
$ node index.mjs
$ npx @leafac/caddy run
$ curl -sv "https://localhost/" > /dev/null
```

```
< HTTP/2 200
< alt-svc: h3=":443"; ma=2592000
< content-type: text/html; charset=utf-8
< date: Fri, 23 Sep 2022 17:45:06 GMT
< etag: W/"b-Ck1VqNd45QIvq3AZd8XYQLvEhtA"
< hello-from-caddy: true
< hello-from-express: true
< content-length: 11
```

Note how Caddy successfully added the `Hello-From-Express` & `Hello-From-Caddy`, and successfully removed the `Server` & `X-Powered-By` headers.

So far so good.

---

_Adding `103 Early Hints` with Caddy_

Change the Caddy configuration to include a `103 Early Hints`:

```caddyfile
# Caddyfile
localhost
header Hello-From-Caddy true
header -Server
header -X-Powered-By
route {
  header  Link "</style.css>; rel=preload; as=style"
  header  Link "</script.js>; rel=preload; as=script"
  respond 103
}
reverse_proxy localhost:3000
```

Repeat the commands in different terminals:

```console
$ node index.mjs
$ npx @leafac/caddy run
$ curl -sv "https://localhost/" > /dev/null
```

```
< HTTP/2 103
< alt-svc: h3=":443"; ma=2592000
< hello-from-caddy: true
< link: </script.js>; rel=preload; as=script
< HTTP/2 200
< alt-svc: h3=":443"; ma=2592000
< content-type: text/html; charset=utf-8
< date: Fri, 23 Sep 2022 17:41:52 GMT
< etag: W/"b-Ck1VqNd45QIvq3AZd8XYQLvEhtA"
< hello-from-caddy: true
< hello-from-express: true
< link: </script.js>; rel=preload; as=script
< x-powered-by: Express
< content-length: 11
```

**At this point, I note two weird things:**

1. Caddy no longer removed the `X-Powered-By` header from the underlying application, even though it added the `Hello-From-Caddy` header, which indicates that it is processing headers.
2. Caddy adds the `Link` header even to the `200` response. It’s weird that the `Link` headers are **leaking** from the `route` in the Caddy configuration.

---

_Adding `103 Early Hints` with the Underlying Application_

Revert the Caddyfile to not add `103 Early Hints`:

```caddyfile
# Caddyfile
localhost
header Hello-From-Caddy true
header -Server
header -X-Powered-By
reverse_proxy localhost:3000
```

Change the underlying application to:

```javascript
import express from "express";
const app = express();
app.get("/", (req, res) => {
  res.socket.write(`HTTP/1.1 103 Early Hints
Link: </style.css>; rel=preload; as=style

`);
  res.header("Hello-From-Express", "true");
  res.send("Hello World");
});
app.listen(3000);
```

Repeat the commands in different terminals:

```console
$ node index.mjs
$ npx @leafac/caddy run
$ curl -sv "https://localhost/" > /dev/null
```

```
< HTTP/2 103
< alt-svc: h3=":443"; ma=2592000
< hello-from-caddy: true
< link: </style.css>; rel=preload; as=style
< HTTP/2 200
< content-type: text/html; charset=utf-8
< date: Fri, 23 Sep 2022 17:50:19 GMT
< etag: W/"b-Ck1VqNd45QIvq3AZd8XYQLvEhtA"
< hello-from-express: true
< x-powered-by: Express
< content-length: 11
```

**At this point, I note another weird thing:** Caddy is only processing the headers for the `103 Early Hints`, and the main response goes through with no header processing.

---

Let me know if you want more detail on something. Also, let me know if I’m misunderstanding something about the intended behavior of `103 Early Hints`. And let me know if there’s something I can do to help.

And thanks for Caddy! It’s awesome!


Discussion / hints:
> Caddy no longer removed the X-Powered-By header from the underlying application, even though it added the Hello-From-Caddy header, which indicates that it is processing headers.

Interesting. I'm just guessing but that's probably because header removal is a _deferred_ action by default (done right before the header is written), not an immediate change to the headers.

I'd need to look at the code but maybe 103 doesn't count as "the header is written" because it's like a pre-response.

> Caddy adds the Link header even to the 200 response. It’s weird that the Link headers are leaking from the route in the Caddy configuration.

I'm not sure what you mean by "leaking" -- do you mean that you expect the Link headers should _only_ be present in the 103 response?

RFC 8297 says:

> Typically, a server will include the header fields sent in a 103 (Early Hints) response in the final response as well.

So I think that is fine in this case.

> At this point, I note another weird thing: Caddy is only processing the headers for the 103 Early Hints, and the main response goes through with no header processing.

That one I'm not quite as sure about... would need to investigate.
> I'd need to look at the code but maybe 103 doesn't count as "the header is written" because it's like a pre-response.

"Deferred" means when the handler goes back up the chain, but that doesn't happen with a 103 until after the proxy writes its response because it's still on the way down the chain.
I forgot that the `headers` handler has its own responseWriterWrapper, which doesn't really account for 1xx responses:

https://github.com/caddyserver/caddy/blob/eead00f54a4b5d55292b2dcf5f1ec462c79207ad/modules/caddyhttp/headers/headers.go#L331-L335

Compare this to:

https://github.com/caddyserver/caddy/blob/eead00f54a4b5d55292b2dcf5f1ec462c79207ad/modules/caddyhttp/responsewriter.go#L177-L192

which does account for 1xx.
@leafac Thanks for the detailed and simplified reproducers. I was able to easily see the behavior you're talking about and understand it.

As you can see above, on the first call to WriteHeader() we'd always set `wroteHeader = true`, which would prevent any repeated header manipulations.

This restriction can be relaxed for 1xx responses. I even seem to remember thinking this when implementing 1xx but then something came up and I got distracted.

Anyway, that's why:

> Caddy no longer removed the X-Powered-By header from the underlying application, even though it added the Hello-From-Caddy header, which indicates that it is processing headers.

because the 103 response did the (deferred) header manipulations for that, then didn't for the final response; and

> At this point, I note another weird thing: Caddy is only processing the headers for the 103 Early Hints, and the main response goes through with no header processing.

because the (deferred) header manipulations only happened once.

Now we only set `wroteHeader = true` if the status is not in the 1xx range. See 7041970.

Please feel free to test out that commit and let us know!
> Thanks for the detailed and simplified reproducers. I was able to easily see the behavior you're talking about and understand it.

Thank **you** and @francislavoie for the amazing work on Caddy and for answering so quickly!

> I'm not sure what you mean by "leaking" -- do you mean that you expect the Link headers should _only_ be present in the 103 response?
> 
> RFC 8297 says:
> 
> > Typically, a server will include the header fields sent in a 103 (Early Hints) response in the final response as well.
>
> So I think that is fine in this case.

Oh, I didn’t know that 😅 Yeah, it’s fine. Thanks for teaching me 😁

> Please feel free to test out that commit and let us know!

I tested and confirmed that it fixed one of the remaining weird behaviors, but not the other:

It fixed the case in which `X-Powered-By: Express` was passing through.

It didn’t fix the case in which Caddy seems to not add headers to the 200 response.

<details>
<summary>For You Convenience Here’s the Test Scenario Again</summary>

```javascript
// index.mjs
import express from "express";
const app = express();
app.get("/", (req, res) => {
  res.socket.write(`HTTP/1.1 103 Early Hints
Link: </style.css>; rel=preload; as=style

`);
  res.header("Hello-From-Express", "true");
  res.send("Hello World");
});
app.listen(3000);
```

```caddyfile
# Caddyfile
localhost
header Hello-From-Caddy true
header -Server
header -X-Powered-By
reverse_proxy localhost:3000
```


```console
$ node index.mjs
$ npx @leafac/caddy run
$ curl -sv "https://localhost/" > /dev/null
```

</details>

Here’s the response I’m getting:

```
< HTTP/2 103 
< alt-svc: h3=":443"; ma=2592000
< hello-from-caddy: true
< link: </style.css>; rel=preload; as=style
< HTTP/2 200 
< content-type: text/html; charset=utf-8
< date: Sat, 24 Sep 2022 10:09:40 GMT
< etag: W/"b-Ck1VqNd45QIvq3AZd8XYQLvEhtA"
< hello-from-express: true
< content-length: 11
```

Note that the 103 response includes `hello-from-caddy: true`, but the 200 response does not.

If I understand correctly, this contradicts the RFC recommendation that you mentioned above.

In fact, I suppose that to follow that recommendation Caddy could pick up the `Link` (and whatever other headers) generated by the application in the 103 response and replicate them in the subsequent 200 response. One could argue that this is the application’s responsibility, but it would a nice quality-of-life thing for Caddy to do.
I deleted a previous comment because I read your latest comment wrong and didn't replicate the setup faithfully.

I've been able to reproduce the latest behavior you're reporting, where the Hello-From-Caddy header disappears. I've confirmed that these lines:

https://github.com/caddyserver/caddy/blob/61822f129b9edbbc1ed2fd068d8e5ba919b5881d/modules/caddyhttp/reverseproxy/reverseproxy.go#L785-L788

are the culprit (by commenting them out). However when I remove those lines, way too many headers are printed.

Will try to come up with a solution that doesn't eat headers set by Caddy.

Fix patch:
diff --git a/modules/caddyhttp/headers/caddyfile.go b/modules/caddyhttp/headers/caddyfile.go
index eec11490c26..a6bec9573d4 100644
--- a/modules/caddyhttp/headers/caddyfile.go
+++ b/modules/caddyhttp/headers/caddyfile.go
@@ -32,12 +32,12 @@ func init() {
 // parseCaddyfile sets up the handler for response headers from
 // Caddyfile tokens. Syntax:
 //
-//     header [<matcher>] [[+|-|?]<field> [<value|regexp>] [<replacement>]] {
-//         [+]<field> [<value|regexp> [<replacement>]]
-//         ?<field> <default_value>
-//         -<field>
-//         [defer]
-//     }
+//	header [<matcher>] [[+|-|?]<field> [<value|regexp>] [<replacement>]] {
+//	    [+]<field> [<value|regexp> [<replacement>]]
+//	    ?<field> <default_value>
+//	    -<field>
+//	    [defer]
+//	}
 //
 // Either a block can be opened or a single header field can be configured
 // in the first line, but not both in the same directive. Header operations
@@ -148,8 +148,7 @@ func parseCaddyfile(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error)
 // parseReqHdrCaddyfile sets up the handler for request headers
 // from Caddyfile tokens. Syntax:
 //
-//     request_header [<matcher>] [[+|-]<field> [<value|regexp>] [<replacement>]]
-//
+//	request_header [<matcher>] [[+|-]<field> [<value|regexp>] [<replacement>]]
 func parseReqHdrCaddyfile(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) {
 	if !h.Next() {
 		return nil, h.ArgErr()
diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go
index 4c8b29b13e1..55d3aa8cea4 100644
--- a/modules/caddyhttp/reverseproxy/reverseproxy.go
+++ b/modules/caddyhttp/reverseproxy/reverseproxy.go
@@ -782,8 +782,9 @@ func (h *Handler) reverseProxy(rw http.ResponseWriter, req *http.Request, origRe
 				copyHeader(h, http.Header(header))
 				rw.WriteHeader(code)
 
-				// Clear headers, it's not automatically done by ResponseWriter.WriteHeader() for 1xx responses
-				for k := range h {
+				// Clear headers coming from the backend
+				// (it's not automatically done by ResponseWriter.WriteHeader() for 1xx responses)
+				for k := range header {
 					delete(h, k)
 				}
 


Test patch: