fix: FromApp inherits the app-level api {} config into site handlers - #142
Open
jtelo88 wants to merge 1 commit into
Open
jtelo88 wants to merge 1 commit into
jtelo88 wants to merge 1 commit into
Conversation
Copy link
Copy Markdown
Sorry, something went wrong.
The api {} block is global-only in the Caddyfile adapter, but FromApp never
copied app.API into the handler configuration, so GenerateHandlerMap built a
nil handler map in every http.handlers.cache instance: the souin API was
unreachable in-band on any listener — a PURGE/GET to /souin-api/* fell
through the cache handler as an ordinary request. Inherit the app-level API
(only when the handler config carries none of its own, which raw-JSON configs
still can) exactly like the other app-level defaults merged here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Caddyfile api {} block is global-only (parseCaddyfileGlobalOption rejects it inside site blocks), but FromApp never copies app.API into the handler configuration. GenerateHandlerMap therefore sees a zero API config (nothing enabled) in every http.handlers.cache instance and returns nil — so the souin API is unreachable in-band on any listener: a PURGE/GET to /souin-api/* falls through the cache handler as an ordinary request (with no terminal handler after it, Caddy answers its default empty 200, which callers can easily mistake for success — we shipped a Surrogate-Key purge that silently no-oped for two months this way).
This copies the app-level API into the handler config exactly like the other app-level defaults merged in FromApp, guarded so a raw-JSON per-handler API config (still possible outside the Caddyfile adapter) keeps winning.
Verified end-to-end with a 3-listener Caddyfile (origin + public site + internal API listener): PURGE with Surrogate-Key now reaches the API (204) and evicts; before the patch the same request fell through with a 200.
Note for reviewers: since the cache directive orders before rewrite, once the API is genuinely served in-band, operators who relied on a later respond /souin-api* 404 to shield the API on public listeners must pin the order (e.g. a route block) — happy to add a README note if you want it in this PR.