darkweak / go-esi

Pure implementation of the non-standard ESI (Edge-Side-Include) specification in Go
MIT License
22 stars 6 forks source link

ESI include should pass some original req's headers into the new request #5

Closed dkarlovi closed 1 year ago

dkarlovi commented 1 year ago

Specification says:

  1. Protocol Considerations When an ESI template is processed, a separate request will need to be made for each include encountered. Implementations may use the original request's headers (e.g., Cookie, User-Agent, etc.) when doing so. Additionally, response headers from fragments (e.g., Set-Cookie, Server, Cache-Control, Last-Modified) may be ignored, and should not influence the assembled page.

(emphasis mine)

When creating the child request, some of the headers need to be included for the child response to be constructed properly, for example Cookie / Authorization headers are required to know the identity of the user for which we're constructing the child response.

Security considerations

Some of these headers must NOT be forwarded in cross-origin scenarios. For example, Cookie and Authorization musn't be passed unless the child request's host, port and scheme all match. IMO the best approach is to have an allow list of headers in general and a separate list of same-origin headers.

dkarlovi commented 1 year ago

Since this package is used officially with Caddy's cache handler, @dunglas might be interested in this or weigh in.

darkweak commented 1 year ago

The specification says that the processor may use the initial request headers, not must. We can create a PR to support that.

dkarlovi commented 1 year ago

@darkweak correct, but the usability of the ESI implementation is severely limited by some of these headers lacking, it's basically a non starter for many intended use cases.

darkweak commented 1 year ago

Okay, so let's copy the base requests headers 👍

dkarlovi commented 1 year ago

We must only pay attention to not leak headers cross origin. 👍

darkweak commented 1 year ago

Do you want to create the PR for that @dkarlovi?

dkarlovi commented 1 year ago

@darkweak done, see #9.