Skip to content

Commit e0d75e4

Browse files
nkissebeclaude
andcommitted
The retired trigger families give up their table slots
Five slots and three handlers go: <BotShieldTrigger>, BotShieldTrigger, <BotShieldFlagTrigger>, BotShieldFlagTrigger and <BotShieldFeedbackTrigger>. bs_cmds[] goes from 96 directives to 91. They existed to name the replacement, because Apache's own answer to an unregistered section is "Invalid command, perhaps misspelled or defined by a module not included", which sends a reader looking for a build or LoadModule problem rather than a rename. Dropping them is a real trade and not a tidy-up: the error gets worse. What pays for it is that the audience is empty. This is the only deployment, no config here has named those spellings since 2026-09-06, and a table slot that exists for a reader who cannot exist is surface with no consumer -- which is the same argument the families themselves were retired on. bs_flat_trigger_retired stays. It is generic over cmd->cmd->name and still backs the one-line key=value form of BotShieldRule, a live directive whose flat form is retired. A family that is gone entirely and a live directive's old calling convention are different cases, and only the first one loses its slot. Both tests that covered these spellings assert configtest failure rather than the message text, so they pass unchanged. Their docstrings did argue for keeping the stubs, so those now say what the failure looks like instead -- the refusal is the contract, the message was a courtesy. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 4c68ec6 commit e0d75e4

4 files changed

Lines changed: 28 additions & 85 deletions

File tree

DESIGN.md

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2226,14 +2226,24 @@ the linker on Apache symbols.
22262226

22272227
### Directive table
22282228

2229-
`bs_cmds[]` registers 96 directives. Retired spellings fall into two
2230-
groups, and which group a name is in decides what an old config sees.
2231-
`BotShieldTrigger`, `BotShieldFlagTrigger` and
2232-
`BotShieldFeedbackTrigger` still occupy table slots pointing at
2233-
`bs_*_retired` handlers, so naming one fails with a message saying
2234-
what to write instead. `BotShieldPathTrigger` has no such stub, so it
2235-
fails with Apache's generic "Invalid command" and the operator gets no
2236-
migration note.
2229+
`bs_cmds[]` registers 91 directives. No retired *family* keeps a
2230+
table slot any more: `BotShieldPathTrigger`, `BotShieldTrigger`,
2231+
`BotShieldFlagTrigger` and `BotShieldFeedbackTrigger` all fail with
2232+
Apache's generic "Invalid command, perhaps misspelled or defined by a
2233+
module not included".
2234+
2235+
The three trigger families held migration stubs until 2026-09-07, and
2236+
dropping them was a deliberate trade rather than a tidy-up: the stub
2237+
message named the replacement, and the generic error instead sends a
2238+
reader looking for a build or LoadModule problem. What paid for it is
2239+
that no config on this deployment -- the only one -- has named those
2240+
spellings since 2026-09-06, so the audience for the message was empty
2241+
while the slots were not.
2242+
2243+
One retired spelling does keep a handler. `bs_flat_trigger_retired` is
2244+
generic over `cmd->cmd->name` and still backs the one-line key=value
2245+
form of `BotShieldRule`, which is a live directive whose flat form is
2246+
retired -- a different case from a family that is gone entirely.
22372247
The family groupings below summarize the surface — the canonical
22382248
per-directive spec (handler, arg count, scope flags, help text) is
22392249
the `bs_cmds[]` table at `src/botshield.c:213`.

src/botshield.c

Lines changed: 0 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -143,60 +143,12 @@ int bs_bot_name_valid(const char *s)
143143
* carries the work; what differs between families is only which setter
144144
* receives the tokens it builds. */
145145

146-
/* Registered rather than dropped so the error names the shape that
147-
* replaced it, instead of Apache's "Invalid command, perhaps
148-
* misspelled or defined by a module not included". */
149-
static const char *bs_open_trigger_retired(cmd_parms *cmd, void *dconf,
150-
const char *arg)
151-
{
152-
(void)dconf; (void)arg;
153-
return apr_psprintf(cmd->pool,
154-
"<BotShieldTrigger> is gone; write <BotShieldRule name> in the "
155-
"same container. A rule declared inside <Location>, "
156-
"<Directory> or <Files> needs no match key -- Apache has "
157-
"already matched, and the container is the condition, which "
158-
"is all this directive ever was.");
159-
}
160-
161-
/* Registered rather than dropped so the error names the shape that
162-
* replaced it. */
163-
static const char *bs_open_flagtrigger_retired(cmd_parms *cmd,
164-
void *dconf, const char *arg)
165-
{
166-
(void)dconf; (void)arg;
167-
return apr_psprintf(cmd->pool,
168-
"<BotShieldFlagTrigger> is gone; write <BotShieldRule name> "
169-
"with BotShieldFlagged <flag>. Its three actions are rule "
170-
"actions: action=score is BotShieldScore, action=tier_floor "
171-
"is BotShieldChallenge, action=block is BotShieldRespond. "
172-
"`reset` has no equivalent -- rules settle by declaration "
173-
"order.");
174-
}
175-
176146
static const char *bs_open_feedback(cmd_parms *cmd, void *dconf,
177147
const char *arg)
178148
{
179149
return bs_section_trigger(cmd, dconf, arg, "BotShieldFeedback", bs_set_feedback);
180150
}
181151

182-
/* The old tag. Registered rather than simply dropped so the error
183-
* names the replacement -- an operator who hits this has a working
184-
* config in front of them, and Apache's own "Invalid command,
185-
* perhaps misspelled or defined by a module not included" sends them
186-
* looking for a build problem instead. */
187-
static const char *bs_open_feedbacktrigger_retired(cmd_parms *cmd,
188-
void *dconf, const char *arg)
189-
{
190-
(void)dconf; (void)arg;
191-
return apr_psprintf(cmd->pool,
192-
"<BotShieldFeedbackTrigger> is gone; write <BotShieldFeedback> "
193-
"and move the event into the block: the tag name is a label "
194-
"now, like a rule's, and BotShieldEvent <name> is the "
195-
"condition. It was never a trigger -- it runs after the "
196-
"response is built and matches a signed header, not a "
197-
"request.");
198-
}
199-
200152

201153
static const char *bs_open_match(cmd_parms *cmd, void *dconf,
202154
const char *arg)
@@ -561,14 +513,6 @@ static const command_rec bs_cmds[] = {
561513
"graceful-shutdown save runs). Range when non-zero: "
562514
"30..86400. Requires mod_watchdog to be loaded; otherwise "
563515
"degrades to shutdown-only with a NOTICE."),
564-
AP_INIT_RAW_ARGS("<BotShieldTrigger", bs_open_trigger_retired,
565-
NULL, RSRC_CONF | ACCESS_CONF,
566-
"Removed. Write <BotShieldRule> in the same "
567-
"container."),
568-
AP_INIT_RAW_ARGS("<BotShieldFlagTrigger",
569-
bs_open_flagtrigger_retired, NULL, RSRC_CONF,
570-
"Removed. Write <BotShieldRule> with "
571-
"BotShieldFlagged."),
572516
AP_INIT_RAW_ARGS("<BotShieldFeedback", bs_open_feedback, NULL, RSRC_CONF,
573517
"Open a BotShieldFeedback block: what a signed "
574518
"application event means. Takes a label; "
@@ -577,10 +521,6 @@ static const command_rec bs_cmds[] = {
577521
"remember about the client. Runs on the response "
578522
"path, so it cannot direct the response -- flags are "
579523
"what the next request reads."),
580-
AP_INIT_RAW_ARGS("<BotShieldFeedbackTrigger",
581-
bs_open_feedbacktrigger_retired, NULL, RSRC_CONF,
582-
"Removed. Write <BotShieldFeedback> with "
583-
"BotShieldEvent inside it."),
584524
AP_INIT_RAW_ARGS("<BotShieldRule", bs_open_rule, NULL,
585525
RSRC_CONF | ACCESS_CONF,
586526
"Open a BotShieldRule block. Takes the rule name; every "
@@ -594,13 +534,6 @@ static const command_rec bs_cmds[] = {
594534
"</BotShieldMatch>. Conditions only -- actions belong "
595535
"on the rules that name the set. Define a set above "
596536
"the rules that use it."),
597-
AP_INIT_TAKE_ARGV("BotShieldTrigger", bs_flat_trigger_retired, NULL,
598-
RSRC_CONF | ACCESS_CONF,
599-
"Removed. Write <BotShieldRule name> in the same "
600-
"Apache container: a rule declared inside "
601-
"<Location>, <Directory>, <Files> or <If> needs no "
602-
"match key, because Apache has already matched and "
603-
"the container is the condition."),
604537
/* E1 — Allow family */
605538
AP_INIT_TAKE_ARGV("BotShieldClassify", bs_set_classify, NULL,
606539
RSRC_CONF,
@@ -847,12 +780,6 @@ static const command_rec bs_cmds[] = {
847780
"(e.g., dev+prod for one logical app, or api+www "
848781
"subdomains). Strings up to 128 chars; hashed to a "
849782
"32-bit ns_id and stored in each SHM slot."),
850-
AP_INIT_TAKE_ARGV("BotShieldFlagTrigger",
851-
bs_flat_trigger_retired, NULL, RSRC_CONF,
852-
"Removed. A rule matching BotShieldFlagged does what "
853-
"this did: BotShieldScore for action=score, "
854-
"BotShieldChallenge for action=tier_floor, "
855-
"BotShieldRespond for action=block."),
856783
/* E4 — cookie triggers */
857784
AP_INIT_TAKE1("BotShieldSessionCookieName",
858785
bs_set_session_cookie_name, NULL, RSRC_CONF,

tests/pytests/test_app_feedback.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -548,11 +548,12 @@ def test_app_feedback_can_mark_the_address(config_override, log_slice):
548548
def test_the_retired_tag_is_refused(config_override):
549549
"""<BotShieldFeedbackTrigger> is gone.
550550
551-
Registered rather than simply dropped, so the error names the
552-
replacement. Apache's own answer to an unregistered section is
551+
It held a migration stub naming the replacement until 2026-09-07,
552+
when the slot was dropped; the failure is now Apache's generic
553553
"Invalid command, perhaps misspelled or defined by a module not
554-
included", which sends an operator looking for a build problem
555-
instead of a rename.
554+
included". What this asserts either way is that the old spelling
555+
does not quietly work -- the message is a courtesy, the refusal is
556+
the contract.
556557
"""
557558
with pytest.raises(Exception) as exc_info:
558559
with config_override(

tests/pytests/test_triggers.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -541,6 +541,11 @@ def test_removed_flagtrigger_family_is_refused(config_override):
541541
the family added over that was its own vocabulary for the same
542542
actions and a `reset` keyword for a merge order rules settle by
543543
declaration.
544+
545+
The migration stub that named the replacement was dropped
546+
2026-09-07, so this now fails with Apache's generic "Invalid
547+
command" rather than a rename note. The assertion is unchanged:
548+
what matters is that the spelling is refused, not how politely.
544549
"""
545550
with pytest.raises(Exception) as exc_info:
546551
with config_override(

0 commit comments

Comments
 (0)