Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 105 additions & 0 deletions docs/adr/0061-pm-zero-copy-haystack.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
# ADR-0061: Zero-copy haystack for the `@pm` Aho-Corasick matcher

- **Status:** proposed
- **Date:** 2026-10-14 (expected; update before merge)
- **Version:** unreleased (post-v3.8.1-cl5)
- **PR:** [#26](https://github.com/cloudlinux/coraza/pull/26) (expected number; update before merge)
- **Issue(s):** No linked issue (CloudLinux DEF-57621, large JSON request bodies)
- **Deciders:** Gabriel Grigoras (CloudLinux)
- **Category:** Perf

## Context and Problem

`@pm`, `@pmFromFile` and `@pmFromDataset` share `pmEvaluate`, which calls the
`aho-corasick` library's `matcher.Iter(value)`. That method is
`IterByte([]byte(haystack))`: it copies the whole value into a new `[]byte` on
every call. ADR-0053 already names the copy, and answers it only for values
shorter than the shortest pattern.

For a value that is large and not short, nothing avoids it. Each `@pm` rule
tests each of its targets, so a large request body is copied once per rule and
per argument. A CPU and allocation profile of the Imunify360 "full" coraza
ruleset (7152 `SecRule` lines) handling one POST with a JSON object of 8 members
and 4.7 MB in total (the `perf-cloudways-badday` lab shape of DEF-57621) shows
`AhoCorasick.Iter` as 1209 MB of the 2025 MB allocated over 6 transactions, 60%
of all allocations and about 200 MB, some 40 times the body, per transaction. The
copy is also a share of the `memmove` time that remains once `SecRxPreFilter`
has removed most regular-expression work. The wafd process handles these bodies
on its request path, and allocation volume sets how often its GC runs, which
every request next to them pays for.

## Decision Drivers

- No allocation proportional to the value for a pattern match that only needs to
read it.
- No change to what matches: same matcher, same options, same results, including
the capture offsets.
- Stay inside what the project allows for `unsafe` (AGENTS.md: reuse the
helpers in `internal/strings`; a new use needs a benchmark, an ADR and
reviewer agreement).
- TinyGo builds keep working.

## Considered Options

- Keep `Iter(value)`. The copy stays.
- Pool and reuse the `[]byte` buffers. Removes the allocation but not the copy
(`memmove` of the whole value), and a pool that has held a multi-megabyte buffer
keeps that memory alive.
- Change the library so it takes a string. Not in our hands; the library is a
dependency and its `IterByte` already takes a slice.
- Move `@pm` to the Wu-Manber `indexedMatcher` of ADR-0052. A much larger change
with its own limits (needle count, leftmost-longest capture semantics that
`@pm` has to keep).
- View the string's bytes in place with `unsafe.Slice(unsafe.StringData(s), len(s))`
and call `IterByte`. One new helper in `internal/strings`, where the project
keeps its other zero-copy conversion (`WrapUnsafe`).

## Decision Outcome

Chosen: **the in-place view.** `internal/strings.UnsafeBytes(s)` returns the
string's bytes without copying, with `len == cap` so that an `append` would
reallocate instead of writing into the string. `pmEvaluate` passes it to
`IterByte`. The helper is documented as read-only.

This is safe because the matcher only reads the haystack. At the library version
pinned in `go.mod` (`v0.0.0-20250424160509-463d218d4745`) the iterator stores the
slice, and the search code (`automaton.go`) and the prefilters (`prefilter.go`)
index it and never assign to it. Both automata Coraza builds go through that same
code: the DFA (`dfa.go`, `DFA: true` in `@pm` and `@pmFromDataset`) and the NFA
(`nfa.go`, `DFA: false` in `@pmFromFile`) only forward the slice to it. A library
update that wrote to the haystack would break the assumption, so a dependency
bump has to keep that in view; `TestPmDoesNotCopyLargeValue` and
`TestPmFindsPatternInLargeValue` pin the behaviour.

The capture path is unchanged: matches are still reported as offsets, and the
captured text is sliced from the original string, not from the view.

## Technical Discussion

No substantive technical discussion recorded; the change was drafted from the
DEF-57621 profile and has not been reviewed yet.

## Participants

- Gabriel Grigoras (CloudLinux): author

## Consequences

- **Positive:** `@pm` no longer allocates per evaluation in proportion to the
value. `TestPmDoesNotCopyLargeValue` asserts at most 1/16 of a 1 MiB value
allocated per evaluation, where the copy allocated all of it.
`BenchmarkPmLargeValue` measures it per value size; the before and after
figures, and the effect on the 4.7 MB lab body, are in the pull request.
- **Negative / follow-up:** One more use of `unsafe`, in the one place the
project already accepts it, with a documented read-only contract. The fork now
carries this patch relative to upstream; it is small and can be proposed
upstream. Replacing the Aho-Corasick matcher of `@pm` with a faster one stays
open: it is the largest CPU cost left after the prefilter in the DEF-57621
profile.

## References

- Related ADRs: ADR-0019 (`unsafe.StringData` refactor), ADR-0049 and ADR-0050
(`@rx` prefilter, `SecRxPreFilter`), ADR-0052 (indexed-bitmap matcher), ADR-0053
(`@pm` `minLen` prefilter)
- Library: https://github.com/petar-dambovaliev/aho-corasick
1 change: 1 addition & 0 deletions docs/adr/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -129,5 +129,6 @@ Superseding does not delete the old record. Set its status and let the history s
| [0058](0058-json-flattened-key-collision.md) | GHSA-5gj4-9gm7-2fx2 | 2026-09-29 | unreleased | P | Preserve both values on a JSON-flattened key collision |
| [0059](0059-secresponsebodyjsondepthlimit.md) | GHSA-3c6w-j9xm-8h2h | 2026-09-26 | unreleased | F | `SecResponseBodyJsonDepthLimit` directive |
| [0060](0060-json-bounded-recursion-prescan.md) | GHSA-6gcq-wc29-5xf2 | 2026-09-30 | unreleased | P | Iterative depth pre-scan before `gjson.Valid` in the JSON body processor |
| [0061](0061-pm-zero-copy-haystack.md) | [#26](https://github.com/cloudlinux/coraza/pull/26) | 2026-10-14 (expected) | unreleased | ⚡ | Zero-copy haystack for the `@pm` Aho-Corasick matcher |

Categories: **F**eature · **P**arity · **⚡** Perf · **R**efactor
7 changes: 6 additions & 1 deletion internal/operators/pm.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
ahocorasick "github.com/petar-dambovaliev/aho-corasick"

"github.com/corazawaf/coraza/v3/experimental/plugins/plugintypes"
utils "github.com/corazawaf/coraza/v3/internal/strings"
)

// Description:
Expand Down Expand Up @@ -83,7 +84,11 @@ func minPatternLen(patterns []string) int {
}

func pmEvaluate(matcher ahocorasick.AhoCorasick, tx plugintypes.TransactionState, value string) bool {
iter := matcher.Iter(value)
// Iter(value) would copy the value into a []byte on every call: with a rule set
// of a few hundred @pm rules scanning every argument, a large body is copied
// once per rule and per argument (ADR-0061). The matcher only reads the
// haystack, so hand it a view of the string instead.
iter := matcher.IterByte(utils.UnsafeBytes(value))

if !tx.Capturing() {
// Not capturing so just one match is enough.
Expand Down
81 changes: 81 additions & 0 deletions internal/operators/pm_alloc_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// Copyright 2026 CloudLinux
// SPDX-License-Identifier: Apache-2.0

//go:build !tinygo

package operators

import (
"runtime"
"strings"
"testing"

"github.com/corazawaf/coraza/v3/experimental/plugins/plugintypes"
)

// TestPmDoesNotCopyLargeValue guards ADR-0061: testing a value against @pm must
// not allocate in proportion to the value. The matcher's Iter(string) copies the
// whole value into a []byte, which is what this used to cost on every @pm rule
// and every argument of a large body.
func TestPmDoesNotCopyLargeValue(t *testing.T) {
op, err := newPM(plugintypes.OperatorOptions{Arguments: "attack injection exploit"})
if err != nil {
t.Fatal(err)
}
p := op.(*pm)
tx := newTestTx()
defer tx.Close()

const size = 1 << 20
value := strings.Repeat("a", size)

const runs = 20
var before, after runtime.MemStats
runtime.ReadMemStats(&before)
for i := 0; i < runs; i++ {
if p.Evaluate(tx, value) {
t.Fatal("no pattern is in the value, yet it matched")
}
}
runtime.ReadMemStats(&after)

perRun := (after.TotalAlloc - before.TotalAlloc) / runs
// A copy of the value is size bytes per run. What is left is the matcher's
// own bookkeeping, a few hundred bytes.
if limit := uint64(size / 16); perRun > limit {
t.Errorf("one @pm evaluation of a %d byte value allocated %d bytes, want at most %d", size, perRun, limit)
}
}

// TestPmFindsPatternInLargeValue checks the match itself on a value the matcher
// reads in place: a pattern at the start, in the middle, at the very end, and
// none at all.
func TestPmFindsPatternInLargeValue(t *testing.T) {
op, err := newPM(plugintypes.OperatorOptions{Arguments: "attack injection exploit"})
if err != nil {
t.Fatal(err)
}
p := op.(*pm)
tx := newTestTx()
defer tx.Close()

filler := strings.Repeat("a", 1<<20)
tests := []struct {
name string
value string
want bool
}{
{"at the start", "attack" + filler, true},
{"in the middle", filler + "Injection" + filler, true},
{"at the very end", filler + "EXPLOIT", true},
{"cut off one byte short", filler + "exploi", false},
{"absent", filler, false},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
if got := p.Evaluate(tx, tc.value); got != tc.want {
t.Errorf("Evaluate = %v, want %v", got, tc.want)
}
})
}
}
28 changes: 28 additions & 0 deletions internal/operators/pm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
package operators

import (
"strconv"
"strings"
"testing"

"github.com/corazawaf/coraza/v3/experimental/plugins/plugintypes"
Expand Down Expand Up @@ -97,6 +99,32 @@ func TestPmMinLenWithSingleCharPattern(t *testing.T) {
}
}

// BenchmarkPmLargeValue is the case ADR-0061 is about: one large value tested
// against a keyword set. Every call used to copy the value into a []byte.
func BenchmarkPmLargeValue(b *testing.B) {
op, err := newPM(plugintypes.OperatorOptions{
Arguments: "select insert update delete from where union injection script alert",
})
if err != nil {
b.Fatal(err)
}
p := op.(*pm)

tx := newTestTx()
b.Cleanup(func() { tx.Close() })

for _, size := range []int{1 << 10, 64 << 10, 1 << 20} {
value := strings.Repeat("a", size)
b.Run(strconv.Itoa(size>>10)+"KiB", func(b *testing.B) {
b.ReportAllocs()
b.SetBytes(int64(size))
for b.Loop() {
p.Evaluate(tx, value)
}
})
}
}

func BenchmarkPmShortInput(b *testing.B) {
op, err := newPM(plugintypes.OperatorOptions{
Arguments: "select insert update delete from where union injection script alert",
Expand Down
10 changes: 10 additions & 0 deletions internal/strings/strings.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,16 @@ func WrapUnsafe(buf []byte) string {
return *(*string)(unsafe.Pointer(&buf))
}

// UnsafeBytes returns s as a byte slice without copying it. The slice aliases
// the string's memory, so it is for reading only: a string is immutable and may
// sit in read-only memory, and writing through the slice is undefined behaviour.
// It is meant for handing a value to a third-party matcher that takes []byte but
// never writes to it, where the conversion []byte(s) would copy the whole value
// on every call (ADR-0061).
func UnsafeBytes(s string) []byte {
return unsafe.Slice(unsafe.StringData(s), len(s))
}

// HasRegex checks if s is enclosed in unescaped forward slashes (e.g. "/pattern/"),
// consistent with the ModSecurity regex delimiter convention. It returns (true, pattern)
// where pattern is the content between the slashes. Escaped closing slashes (e.g. "/foo\/")
Expand Down
40 changes: 40 additions & 0 deletions internal/strings/unsafebytes_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Copyright 2026 CloudLinux
// SPDX-License-Identifier: Apache-2.0

//go:build !tinygo

package strings

import (
"bytes"
"testing"
"unsafe"
)

func TestUnsafeBytes(t *testing.T) {
tests := []struct {
name string
in string
}{
{"empty", ""},
{"one byte", "a"},
{"ascii", "select * from users"},
{"multibyte", "héllo wörld ☃"},
{"nul and high bytes", "a\x00b\xffc"},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
got := UnsafeBytes(tc.in)
if !bytes.Equal(got, []byte(tc.in)) {
t.Fatalf("UnsafeBytes(%q) = %q", tc.in, got)
}
if len(got) != len(tc.in) || cap(got) != len(tc.in) {
t.Errorf("len/cap = %d/%d, want %d/%d: an append must reallocate, not write into the string",
len(got), cap(got), len(tc.in), len(tc.in))
}
if len(tc.in) > 0 && unsafe.SliceData(got) != unsafe.StringData(tc.in) {
t.Error("the slice does not alias the string: the value was copied")
}
})
}
}
Loading
Loading