Reject status-only response matchers in handle_response

[suryaanshah]

Caddy PR #6529: Status Matcher Fix

Assistance Disclosure

I used GitHub Copilot to understand the issue and repository structure. It generated the code changes, which I verified for correctness.

Issue Summary

This pull request addresses Caddy issue #6529, where the configuration parser incorrectly enforced a two-argument check for handle_status instead of replace_status when replacing HTTP status codes. This failed for abstracted status replacements or multi-argument usage.

Proposed Solution

The fix introduces a helper function that returns true if len(m.StatusCode) > 0 and len(m.Headers) == 0, accurately detecting matchers dedicated solely to status codes.

Key Logic

• Status codes present: len(m.StatusCode) > 0 confirms at least one status rule (e.g., @bad status 500 sets StatusCode = [500]).
• No header rules: len(m.Headers) == 0 ensures no additional header matching (e.g., @bad status 500 header Foo bar sets Headers = {"Foo": ["bar"]}, returning false).

This catches pure status matchers like @bad status 500 used in handle_response @bad { ... }.

Accuracy Rationale

The check precisely answers: "Is this matcher only about status?" by verifying status rules exist without header interference. It aligns with Caddy's matcher syntax for directives like reverse_proxy and handle_response.

Limitations

This simple heuristic covers status codes and headers only. Future ResponseMatcher expansions may require updates to the helper.

[CLAassistant]

All committers have signed the CLA.

[francislavoie]

Must use tabs

Suggested change

	return d.Errf("use 'replace_status' instead of 'handle_response' for status changes")
	return d.Errf("use 'replace_status' instead of 'handle_response' for status changes")

[francislavoie]

I think this is wrong? It's not about it being a status matcher, it's about not having routes inside it.

I think we should just change the condition to reject more than a single argument, and stop referencing replace_status in the error message, with the message saying that only a single matcher name token is allowed.

[suryaanshah]

@francislavoie if we do reject >1 arguments (which was a suggestion copilot gave me but I rejected it)

@bad status 500
handle_response @bad {

gives no errors

Hence I thought a different approach was needed.

[suryaanshah]

How can we verify if it doesn't have routes inside it according to you? This was one approach not ideal though I agree

[francislavoie]

But... that's valid. Matching on 500 status is the whole point of it, so you can render a different error page or something.

I think you misunderstood the issue. We used to have only handle_response with inline status code argument, and it had special behaviour to just change the status code before, but that was split off into replace_status and the old syntax was made an error with that message. That old syntax has been disallowed for 2 years now, so there's no need to have an explanation for it in the error message anymore.

See the examples in the docs https://caddyserver.com/docs/caddyfile/directives/reverse_proxy#examples:

example.com {
	reverse_proxy localhost:8080 {
		@error status 500 503
		handle_response @error {
			root /path/to/error/pages
			rewrite /{rp.status_code}.html
			file_server
		}
	}
}

[suryaanshah]

So you're suggesting to

1. Change it to >1 instead of ==2

2. Changing the error message

Is that accurate?

PS. Thanks for quick responses, my first time contributing to this repo so pardon for misunderstanding