moonchen opened a new pull request, #13352:
URL: https://github.com/apache/trafficserver/pull/13352
## Problem
`Pattern::replace()` in the prefetch plugin rejects any replacement
reference `$N` whose index is `>=` the value returned by the match. But that
value is *one past the highest capture group that **participated*** in the
match, not the number of groups the pattern defines. A trailing optional group
such as `(\?.*)?` that does not participate — e.g. a request with no query
string — lowers that count and makes a perfectly valid `$N` look out of range.
Concretely, with a remap like:
```
@plugin=prefetch.so
@pparam=--fetch-path-pattern=/(.*-)(\d+)(\?.*)?$/$1{$2+1}$3/
```
every matching request **without a query string** logs:
```
ERROR: (prefetch) invalid reference in replacement string: $3
```
and the prefetch is silently dropped (the client is still served).
## Fix
- **`replace()`**: at match time, substitute an empty string for a group
that did not participate instead of failing the whole replacement — the
documented PCRE2 semantics for an unmatched group.
- **`compile()`**: validate `$N` references once, at config-load time,
against the pattern's actual capture-group count (`Regex::get_capture_count()`)
— the correct place to catch a genuinely out-of-range reference such as `$5`
against a 3-group pattern, and a pattern defining more groups than can be
captured (`$0..$9`).
## Additional hardening
- Treat an unusable `--fetch-path-pattern`, and invalid `--fetch-count` /
`--fetch-max` / `--fetch-overflow` values, as **configuration errors**: the
remap instance fails to load instead of silently running with prefetch
disabled. On a running server, `traffic_ctl config reload` safely rejects the
bad config and keeps the current one.
- Skip an empty expanded path instead of scheduling a zero-length fetch
(which `BgFetch` turns into a self-prefetch of the original request path).
- Remove the now-dead `Pattern::process()` / `Pattern::capture()` helpers
(no callers).
> **Note for reviewers:** the "refuse to load on invalid config" behavior
change is intentional and aligns with how `--fetch-policy` already behaves, but
it *is* a behavior change — happy to split it into a separate PR if preferred.
## Tests
Adds three autests under `tests/gold_tests/pluginTest/prefetch/`:
- `prefetch_optional_group` — the non-participating optional-group case
(fails on the pre-fix code).
- `prefetch_bad_pattern_refused` — an over-limit (10-group) pattern makes
ATS refuse to load the remap.
- `prefetch_empty_replacement` — an empty-collapsing replacement is logged
and skipped, not self-prefetched.
Built locally (macOS, PCRE2); all prefetch autests pass. Draft pending CI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
