cloudflare / cloudflare/workers-sdk

[workers-shared] _redirects/_headers rules with prefix-colliding placeholders are silently dropped

Open Beginner friendly
#14,887 1 comment 0 reactions 0 assignees View on GitHub
feature:workers-assets package:workers-shared product:pages
Dominant language
TypeScript
Stars
4.5k
Forks
1.5k
Avg merge
3d 8h
Merged PRs (30d)
186

Description

### Which Cloudflare product(s) does this pertain to?

Workers Assets, Pages

### What version(s) of the tool(s) are you using?

`@cloudflare/workers-shared` asset-worker (verified against `main` at 960f199af)

### What version of Node is your environment running?

v20+

### Describe the Bug

When two placeholders in a `_redirects` or `_headers` rule share a prefix — e.g. `:id` and `:id_2` — the rule is **silently dropped at runtime with no diagnostic**. The user sees their redirect simply not fire.

There are two separate prefix-collision bugs in `rules-engine.ts`, both caused by substituting on a raw `:name` string rather than on a placeholder-aware match.

**1. `generateRuleRegExp` builds an invalid regex (this is the one that drops the rule)**

https://github.com/cloudflare/workers-sdk/blob/main/packages/workers-shared/asset-worker/src/utils/rules-engine.ts#L57-L60

```ts
const path_matches = rule.matchAll(PLACEHOLDER_REGEX);
for (const path_match of path_matches) {
rule = rule.split(path_match[0]).join(`(?<${path_match[1]}>[^/]+)`);
}
```

Splitting on the literal `":id"` also splits inside `":id_2"`, so both placeholders are replaced by the *same* named group:

```
generateRuleRegExp("/p/:id/:id_2")
-> SyntaxError: Invalid regular expression:
/^\/p\/(?[^/]+)\/(?[^/]+)_2$/: Duplicate capture group name
```

`generateRulesMatcher` compiles rules inside a bare `catch {}` (L83-86), so the `SyntaxError` is swallowed and the rule is dropped from `compiledRules` entirely. The user gets no warning, no log — the rule just never matches.

**2. `replacer` substitutes the shorter placeholder first, corrupting the longer one**

https://github.com/cloudflare/workers-sdk/blob/main/packages/workers-shared/asset-worker/src/utils/rules-engine.ts#L24-L29

```ts
for (const [replacement, value] of Object.entries(replacements)) {
str = str.replaceAll(`:${replacement}`, value);
}
```

`:a` is substituted before `:ab`, eating the `:ab` token:

```
replacer("/new/:a/:ab", { a: "X", ab: "Y" }) => "/new/X/Xb" (expected "/new/X/Y")
replacer("/dest/:id/:id_2", { id: "1", id_2: "2" }) => "/dest/1/1_2" (expected "/dest/1/2")
```

This one also means a replacement *value* that happens to contain `:something` can be re-substituted by a later iteration of the same loop.

### Steps to reproduce

Both reproduce directly against the real source. Running the two functions verbatim:

```
replacer('/new/:a/:ab', {a:X, ab:Y}) => "/new/X/Xb"
replacer('/dest/:id/:id_2', {id:1, id_2:2}) => "/dest/1/1_2"
generateRuleRegExp('/p/:id/:id_2') THREW SyntaxError: ... Duplicate capture group name

--- control, no prefix collision, both correct ---
replacer('/new/:a/:b', {a:X, b:Y}) => "/new/X/Y"
generateRuleRegExp('/p/:id/:other') => /^\/p\/(?[^/]+)\/(?[^/]+)$/
```

End to end, a `_redirects` file containing:

```
/p/:id/:id_2 /dest/:id/:id_2 301
```

produces no redirect at all — `generateRulesMatcher(...)({ request })` returns `[]`.

### Expected behaviour

Placeholder names are already fully specified by `PLACEHOLDER_REGEX` (`/:([A-Za-z]\w*)/g`), so `:id_2` is one token and is not a `:id` followed by `_2`. Both substitutions should be done in a single regex-driven pass over the string so that each placeholder is matched whole:

- `generateRuleRegExp` should produce `/^\/p\/(?[^/]+)\/(?[^/]+)$/`
- `replacer("/dest/:id/:id_2", { id: "1", id_2: "2" })` should return `/dest/1/2`

Separately, it may be worth reconsidering the bare `catch {}` in `generateRulesMatcher` — an invalid rule silently vanishing is what makes this class of bug so hard to diagnose. Happy to leave that out of scope.

### Please provide any relevant error logs

```
SyntaxError: Invalid regular expression: /^\/p\/(?[^/]+)\/(?[^/]+)_2$/: Duplicate capture group name
```
(swallowed by `catch {}` — never surfaced to the user)

---

I have a fix ready with regression tests and a changeset — a single `String.replace` pass for both functions. Happy to open a PR.

Contributor guide

Open the contributing guide

Research direction

Start in packages/workers-shared/asset-worker/src/utils/rules-engine.ts, focusing on replacer, generateRuleRegExp, and generateRulesMatcher, then run the direct reproductions from the issue. Done means prefix-colliding placeholders remain whole tokens, the shown regular expression is generated without duplicate groups, and both replacement examples produce their expected values with regression coverage.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
backend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
85/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.