Skip to content

Commit 71867ba

Browse files
stayallivestayallive
committed
Ensure rules ordered 0 are processed first
1 parent b775058 commit 71867ba

9 files changed

Lines changed: 192 additions & 80 deletions

File tree

‎cmd/dev.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ func writeDevPreviewConfig(path string, difficulty int) error {
162162
Rules: map[string]*config.Rule{
163163
"challenge-page": {
164164
Action: "challenge-pow",
165-
SortOrder: 10,
165+
SortOrder: devPreviewIntPtr(10),
166166
Conditions: &config.RuleConditions{
167167
Matches: []config.MatchCondition{
168168
{Type: "path", Match: "starts-with", Value: "/challenge"},
@@ -171,7 +171,7 @@ func writeDevPreviewConfig(path string, difficulty int) error {
171171
},
172172
"block-page": {
173173
Action: "block-403",
174-
SortOrder: 20,
174+
SortOrder: devPreviewIntPtr(20),
175175
Conditions: &config.RuleConditions{
176176
Matches: []config.MatchCondition{
177177
{Type: "path", Match: "starts-with", Value: "/blocked"},
@@ -180,7 +180,7 @@ func writeDevPreviewConfig(path string, difficulty int) error {
180180
},
181181
"rate-limit-page": {
182182
Action: "rate-limit-preview",
183-
SortOrder: 30,
183+
SortOrder: devPreviewIntPtr(30),
184184
Conditions: &config.RuleConditions{
185185
Matches: []config.MatchCondition{
186186
{Type: "path", Match: "starts-with", Value: "/rate-limit"},

‎config.schema.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@
156156
},
157157
"sort_order": {
158158
"type": "integer",
159-
"description": "Optional explicit ordering for rule processing. Lower values are processed first. Rules without sort_order are processed after ordered rules, sorted by rule ID."
159+
"description": "Optional explicit ordering for rule processing. Lower values are processed first, including 0. Rules without sort_order are processed after ordered rules, sorted by rule ID."
160160
},
161161
"conditions": {
162162
"$ref": "#/definitions/conditions"

‎config/load.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -125,16 +125,16 @@ func (m *Manager) computeSortedRules(rules map[string]*Rule) []*Rule {
125125

126126
// Sort by sort_order (primary), then by ID (secondary)
127127
sort.Slice(ruleList, func(i, j int) bool {
128-
iHasOrder := ruleList[i].SortOrder != 0
129-
jHasOrder := ruleList[j].SortOrder != 0
128+
iHasOrder := ruleList[i].SortOrder != nil
129+
jHasOrder := ruleList[j].SortOrder != nil
130130

131131
switch {
132132
case iHasOrder && !jHasOrder:
133133
return true
134134
case !iHasOrder && jHasOrder:
135135
return false
136-
case iHasOrder && jHasOrder && ruleList[i].SortOrder != ruleList[j].SortOrder:
137-
return ruleList[i].SortOrder < ruleList[j].SortOrder
136+
case iHasOrder && jHasOrder && *ruleList[i].SortOrder != *ruleList[j].SortOrder:
137+
return *ruleList[i].SortOrder < *ruleList[j].SortOrder
138138
}
139139

140140
// Fall back to ID comparison for deterministic ordering

‎config/load_test.go‎

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,19 @@ func TestComputeSortedRules(t *testing.T) {
66
manager := &Manager{}
77

88
sorted := manager.computeSortedRules(map[string]*Rule{
9-
"z-last": {ID: "z-last"},
10-
"a-first": {ID: "a-first", SortOrder: 1},
11-
"b-next": {ID: "b-next", SortOrder: 1},
12-
"c-mid": {ID: "c-mid", SortOrder: 2},
9+
"z-last": {ID: "z-last"},
10+
"zero-first": {ID: "zero-first", SortOrder: sortOrderPtr(0)},
11+
"a-next": {ID: "a-next", SortOrder: sortOrderPtr(1)},
12+
"b-next": {ID: "b-next", SortOrder: sortOrderPtr(1)},
13+
"c-mid": {ID: "c-mid", SortOrder: sortOrderPtr(2)},
1314
})
1415

15-
if len(sorted) != 4 {
16-
t.Fatalf("expected 4 sorted rules, got %d", len(sorted))
16+
if len(sorted) != 5 {
17+
t.Fatalf("expected 5 sorted rules, got %d", len(sorted))
1718
}
1819

19-
got := []string{sorted[0].ID, sorted[1].ID, sorted[2].ID, sorted[3].ID}
20-
want := []string{"a-first", "b-next", "c-mid", "z-last"}
20+
got := []string{sorted[0].ID, sorted[1].ID, sorted[2].ID, sorted[3].ID, sorted[4].ID}
21+
want := []string{"zero-first", "a-next", "b-next", "c-mid", "z-last"}
2122
for i := range want {
2223
if got[i] != want[i] {
2324
t.Fatalf("expected order %v, got %v", want, got)
@@ -32,7 +33,20 @@ func TestComputeSortedRules(t *testing.T) {
3233
func TestLoadHydratesIDsAndSortsRules(t *testing.T) {
3334
configPath := writeTestConfig(t, `{
3435
"id": "cfg-123",
35-
"rules": {
36+
"rules": {
37+
"zero-rule": {
38+
"action": "action-zero",
39+
"sort_order": 0,
40+
"conditions": {
41+
"matches": [
42+
{
43+
"type": "path",
44+
"match": "starts-with",
45+
"value": "/zero"
46+
}
47+
]
48+
}
49+
},
3650
"z-rule": {
3751
"action": "action-z",
3852
"conditions": {
@@ -60,6 +74,10 @@ func TestLoadHydratesIDsAndSortsRules(t *testing.T) {
6074
}
6175
},
6276
"actions": {
77+
"action-zero": {
78+
"action": "block",
79+
"status": 401
80+
},
6381
"action-z": {
6482
"action": "block",
6583
"status": 403
@@ -88,14 +106,18 @@ func TestLoadHydratesIDsAndSortsRules(t *testing.T) {
88106
t.Fatal("expected action IDs to be hydrated from map keys")
89107
}
90108

91-
if len(manager.sortedRules) != 2 {
92-
t.Fatalf("expected 2 sorted rules, got %d", len(manager.sortedRules))
109+
if len(manager.sortedRules) != 3 {
110+
t.Fatalf("expected 3 sorted rules, got %d", len(manager.sortedRules))
93111
}
94-
if manager.sortedRules[0].ID != "a-rule" || manager.sortedRules[1].ID != "z-rule" {
95-
t.Fatalf("unexpected sorted rule order: %s, %s", manager.sortedRules[0].ID, manager.sortedRules[1].ID)
112+
if manager.sortedRules[0].ID != "zero-rule" || manager.sortedRules[1].ID != "a-rule" || manager.sortedRules[2].ID != "z-rule" {
113+
t.Fatalf("unexpected sorted rule order: %s, %s, %s", manager.sortedRules[0].ID, manager.sortedRules[1].ID, manager.sortedRules[2].ID)
96114
}
97115
}
98116

117+
func sortOrderPtr(v int) *int {
118+
return &v
119+
}
120+
99121
func TestLoadRejectsAllProtocolsDisabled(t *testing.T) {
100122
configPath := writeTestConfig(t, `{
101123
"server": {

‎config/types.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ type Rule struct {
120120
ID string // Rule ID from the map key
121121
Name string `json:"name"`
122122
Action string `json:"action"`
123-
SortOrder int `json:"sort_order,omitempty"` // Optional: explicit ordering (lower = processed first)
123+
SortOrder *int `json:"sort_order,omitempty"` // Optional: explicit ordering (lower = processed first)
124124
Conditions *RuleConditions `json:"conditions"`
125125
}
126126

‎middleware/challenge_test.go‎

Lines changed: 7 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"net/http"
55
"net/http/httptest"
66
"net/url"
7-
"sort"
87
"strconv"
98
"strings"
109
"testing"
@@ -28,29 +27,7 @@ func (m *challengeMockConfigProvider) GetRules() map[string]*config.Rule {
2827
}
2928

3029
func (m *challengeMockConfigProvider) GetSortedRules() []*config.Rule {
31-
if len(m.rules) == 0 {
32-
return nil
33-
}
34-
35-
rules := make([]*config.Rule, 0, len(m.rules))
36-
for _, rule := range m.rules {
37-
rules = append(rules, rule)
38-
}
39-
sort.Slice(rules, func(i, j int) bool {
40-
iHasOrder := rules[i].SortOrder != 0
41-
jHasOrder := rules[j].SortOrder != 0
42-
switch {
43-
case iHasOrder && !jHasOrder:
44-
return true
45-
case !iHasOrder && jHasOrder:
46-
return false
47-
case iHasOrder && jHasOrder && rules[i].SortOrder != rules[j].SortOrder:
48-
return rules[i].SortOrder < rules[j].SortOrder
49-
default:
50-
return rules[i].ID < rules[j].ID
51-
}
52-
})
53-
return rules
30+
return sortedTestRules(m.rules)
5431
}
5532

5633
func (m *challengeMockConfigProvider) GetActions() map[string]*config.RuleAction {
@@ -557,15 +534,15 @@ func TestChallengePassedCanStillBeBlockedByLaterRule(t *testing.T) {
557534
"challenge-admin": {
558535
ID: "challenge-admin",
559536
Action: "challenge-action",
560-
SortOrder: 1,
537+
SortOrder: testIntPtr(1),
561538
Conditions: &config.RuleConditions{
562539
Matches: []config.MatchCondition{{Type: "path", Match: "starts-with", Value: "/admin"}},
563540
},
564541
},
565542
"block-admin": {
566543
ID: "block-admin",
567544
Action: "block-action",
568-
SortOrder: 2,
545+
SortOrder: testIntPtr(2),
569546
Conditions: &config.RuleConditions{
570547
Matches: []config.MatchCondition{{Type: "path", Match: "starts-with", Value: "/admin"}},
571548
},
@@ -619,15 +596,15 @@ func TestChallengePassedCanStillBeRateLimitedByLaterRule(t *testing.T) {
619596
"challenge-admin": {
620597
ID: "challenge-admin",
621598
Action: "challenge-action",
622-
SortOrder: 1,
599+
SortOrder: testIntPtr(1),
623600
Conditions: &config.RuleConditions{
624601
Matches: []config.MatchCondition{{Type: "path", Match: "starts-with", Value: "/admin"}},
625602
},
626603
},
627604
"rate-limit-admin": {
628605
ID: "rate-limit-admin",
629606
Action: "rate-limit-action",
630-
SortOrder: 2,
607+
SortOrder: testIntPtr(2),
631608
Conditions: &config.RuleConditions{
632609
Matches: []config.MatchCondition{{Type: "path", Match: "starts-with", Value: "/admin"}},
633610
},
@@ -691,15 +668,15 @@ func TestFirstMatchingChallengeRuleWins(t *testing.T) {
691668
"challenge-first": {
692669
ID: "challenge-first",
693670
Action: "challenge-first-action",
694-
SortOrder: 1,
671+
SortOrder: testIntPtr(1),
695672
Conditions: &config.RuleConditions{
696673
Matches: []config.MatchCondition{{Type: "path", Match: "starts-with", Value: "/admin"}},
697674
},
698675
},
699676
"challenge-second": {
700677
ID: "challenge-second",
701678
Action: "challenge-second-action",
702-
SortOrder: 2,
679+
SortOrder: testIntPtr(2),
703680
Conditions: &config.RuleConditions{
704681
Matches: []config.MatchCondition{{Type: "path", Match: "starts-with", Value: "/admin"}},
705682
},
@@ -899,7 +876,3 @@ func assertChallengeInfoNames(t *testing.T, r *http.Request, ruleName string, ac
899876
t.Fatalf("expected challenge action name %q, got %#v", actionName, info.Action)
900877
}
901878
}
902-
903-
func testIntPtr(v int) *int {
904-
return &v
905-
}

0 commit comments

Comments
 (0)