fix: support encoded control zones safely (full percent-decode for zone/group params)

[yaoge123]

Problem

The /status/control API parses group and zone query parameters using hard-coded, character-by-character %XX replacements. Only three sequences are handled: %2A → *, %3A/%3a → :, and %40 → @. As a result:

• Characters such as space (%20), backtick (%60), pipe (%7C), + (encoded as %2B), and any multi-byte UTF-8 sequence all fail to decode — the encoded form never matches a stored zone name.
• The group parameter duplicates every branch: one for the literal string and one for its %XX form, making the code fragile and hard to extend.

Fix

Add ngx_http_vhost_traffic_status_url_decode() in string.c/string.h — a standard percent-decode routine that:

• Decodes all valid %XX sequences (case-insensitive hex digits).
• Leaves bare + as a literal + (not a space — this is a path/query parameter, not form data).
• Leaves malformed % sequences (e.g. %2G) untouched.

In display.c, both group and zone are decoded with this helper before comparison/lookup. All ad-hoc replace_strc() calls and the duplicated %XX branches are removed.

Changes

• src/ngx_http_vhost_traffic_status_string.c — add ngx_http_vhost_traffic_status_url_decode()
• src/ngx_http_vhost_traffic_status_string.h — add declaration
• src/ngx_http_vhost_traffic_status_display.c — decode group and zone before use; remove all hard-coded %XX replacements

Tests

New test cases added to t/008, t/011, and t/014:

Scenario	Example
Percent-encoded :: and @ in zone (existing tests 6-8, now actually exercising the full decode path)	zone=storage%3A%3Alocalhost%40vol0
Percent-encoded : in upstream@alone zone	group=upstream%40alone&zone=127.0.0.1%3A1981
Bare + stays as literal +	zone=storage::localhost@test+value
%2B decodes to literal +	zone=storage::localhost@test%2Bvalue
Space via %20	zone=storage::localhost@test%20value
Backtick via %60	zone=storage::localhost@test%60value
Pipe via %7C	zone=storage::localhost@test%7Cvalue
Multi-byte UTF-8 Chinese	zone=storage::localhost@%E5%95%86%E6%A0%87
Malformed %2G is left untouched	zone=storage::localhost@test%2Gvalue

[Copilot]

Pull request overview

This PR improves /status/control query parsing by introducing a general percent-decoder for group/zone parameters and removing the prior hard-coded %XX replacement logic, enabling control operations on zones containing a wider range of encoded characters.

Changes:

• Added ngx_http_vhost_traffic_status_url_decode() helper to percent-decode %XX sequences in-place.
• Updated control handler to decode group and zone before comparisons/lookup and removed the ad-hoc %2A/%3A/%40 handling.
• Extended Test::Nginx coverage for spaces, punctuation, UTF-8, + vs %2B, and malformed encodings.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/ngx_http_vhost_traffic_status_string.c	Adds ngx_http_vhost_traffic_status_url_decode() implementation.
src/ngx_http_vhost_traffic_status_string.h	Exposes the new decode helper declaration.
src/ngx_http_vhost_traffic_status_display.c	Decodes group/zone before evaluating control command behavior.
t/008.control_status_zone.t	Adds coverage for literal + and %2B decoding in status command.
t/011.control_reset_zone.t	Adds coverage for literal + and %2B decoding in reset command.
t/014.control_delete_zone.t	Adds broader coverage for percent-decoding in delete command (space, punctuation, UTF-8, malformed sequences).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

After ngx_http_vhost_traffic_status_copy_str() failure, the code logs but continues and immediately calls url_decode() / reads decoded_group.*. If copy_str() returns NGX_ERROR (e.g., OOM), decoded_group.data may be NULL/uninitialized, leading to a crash or undefined behavior. Consider returning NGX_HTTP_INTERNAL_SERVER_ERROR (or at least setting control->command to NONE and skipping further processing) when copy_str() fails, and only calling url_decode() when rc == NGX_OK.

Suggested change

	"display_handler_control::copy_str() failed");
	}
	rc = ngx_http_vhost_traffic_status_url_decode(&decoded_group);
	if (rc != NGX_OK) {
	ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
	"display_handler_control::url_decode() failed");
	"display_handler_control::copy_str() failed");
	return NGX_HTTP_INTERNAL_SERVER_ERROR;
	}
	rc = ngx_http_vhost_traffic_status_url_decode(&decoded_group);
	if (rc != NGX_OK) {
	ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
	"display_handler_control::url_decode() failed");
	return NGX_HTTP_INTERNAL_SERVER_ERROR;

[Copilot]

Same pattern as group: if ngx_http_vhost_traffic_status_copy_str() fails for zone, the function logs but still calls url_decode() and subsequent mutation helpers on control->zone. If copy_str() failed, control->zone->data can be NULL, which can crash. It’s safer to abort the request (500) or skip control processing when copy_str() fails.

[Copilot]

ngx_http_vhost_traffic_status_url_decode() always writes a terminating '\0' at buf->data + buf->len without verifying there is space / that the input is already NUL-terminated. Other in-place helpers here (e.g., replace_strc) explicitly check for a trailing NUL before using it. To avoid potential out-of-bounds writes when callers pass an ngx_str_t that isn’t allocated with an extra byte, add a guard (e.g., buf->data != NULL and *(buf->data + original_len) == '\0') and return NGX_ERROR if the precondition isn’t met.