Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
dkarlovi opened this issue Nov 11, 2022 · 7 comments · Fixed by #10
Closed

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

dkarlovi opened this issue Nov 11, 2022 · 7 comments · Fixed by #10

Comments

@dkarlovi
Copy link
Contributor

dkarlovi commented Nov 11, 2022

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
Copy link
Contributor Author

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

@darkweak
Copy link
Owner

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

@dkarlovi
Copy link
Contributor Author

@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
Copy link
Owner

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

@dkarlovi
Copy link
Contributor Author

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

@darkweak
Copy link
Owner

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

@dkarlovi
Copy link
Contributor Author

@darkweak done, see #9.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants