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

caddyhttp: Merge query matchers in Caddyfile #3839

Merged
merged 1 commit into from
Nov 2, 2020

Conversation

francislavoie
Copy link
Member

I noticed this while revising the matcher docs, query wasn't adapting as advertised. It was only letting you have one value per key, and if you specified more tokens, it would fail because d.Args(&query) would end up hitting every 2nd token because it's in a d.Next() loop. Switches to Add from Set which will append the value (even if it's empty) instead of overwriting every time.

Also, turns out that Add on headers will work even if there's nothing there yet (because it's initialized already as an empty array), so we can remove the condition I introduced in #3832 (effectively we just switched from Set to Add)

Also, turns out that `Add` on headers will work even if there's nothing there yet, so we can remove the condition I introduced in caddyserver#3832
Copy link
Member

@mholt mholt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice, thanks for the fix and tests!

@mholt mholt merged commit b4f49e2 into caddyserver:master Nov 2, 2020
@francislavoie francislavoie deleted the query-matcher-merging branch November 2, 2020 23:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐞 Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants