diff --git a/regex/operators/assembler.go b/regex/operators/assembler.go index 8a906c8..2540d64 100644 --- a/regex/operators/assembler.go +++ b/regex/operators/assembler.go @@ -129,12 +129,11 @@ func (a *Operator) complete(assembleParser *parser.Parser) (string, error) { return "", err } - if len(assembleParser.Prefixes) > 0 && len(assembleParser.Suffixes) > 0 && len(result) > 0 { - result = "(?:" + result + ")" - } - prefixes := strings.Join(assembleParser.Prefixes, "") - suffixes := strings.Join(assembleParser.Suffixes, "") - result = prefixes + result + suffixes + // Note: Prefix/suffix application is now handled by individual Assemble processors (block-scoped). + // Prefix/suffix directives are passed through as raw lines, so they are not applied here. + // Parser.Prefixes and Parser.Suffixes record which directives a file contained; the parser + // uses that to decide whether an included file needs its own assemble block, and the operator + // path does not read them. if len(result) > 0 { logger.Trace().Msgf("Applying last cleanups to %s\n", result) diff --git a/regex/operators/assembler_test.go b/regex/operators/assembler_test.go index 396a435..b376687 100644 --- a/regex/operators/assembler_test.go +++ b/regex/operators/assembler_test.go @@ -4,7 +4,9 @@ package operators import ( + "io/fs" "os" + "path" "testing" "github.com/stretchr/testify/suite" @@ -832,6 +834,7 @@ d } func (s *assemblerTestSuite) TestAssemble_ConcatenationWithPrefixAndSuffix() { + // Test that top-level prefix/suffix apply to the implicit top-level block contents := `##!^ prefix ##!$ suffix ##!> assemble @@ -1121,3 +1124,217 @@ func (s *assemblerTestSuite) TestAssemble_ValidateCharacterClass() { s.ErrorContains(err, "unicode hex escape codepoint too big: 1114111 > 255") } + +// Test block-scoped prefix/suffix behavior +func (s *assemblerTestSuite) TestAssemble_BlockScopedPrefixSuffix_InsideBlock() { + // Test that prefix/suffix inside an assemble block are block-scoped + contents := `##!> assemble +##!^ prefix +##!$ suffix +a +b +##!<` + assembler := NewAssembler(s.ctx) + output, err := assembler.Run(contents) + s.Require().NoError(err) + // wrapCompletedAssembly already wraps in (?:...) + s.Equal(`prefix[ab]suffix`, output) +} + +func (s *assemblerTestSuite) TestAssemble_BlockScopedPrefixSuffix_TopLevel() { + // Verify top-level directives still work (implicit top-level block) + contents := `##!^ prefix +##!$ suffix +a +b` + assembler := NewAssembler(s.ctx) + output, err := assembler.Run(contents) + s.Require().NoError(err) + s.Equal(`prefix[ab]suffix`, output) +} + +func (s *assemblerTestSuite) TestAssemble_BlockScopedPrefixSuffix_MultipleBlocks() { + // Test multiple assemble blocks with different prefix/suffix in each + contents := `##!> assemble +##!^ prefix1 +a +b +##!< +##!=> +##!> assemble +##!$ suffix2 +c +d +##!<` + assembler := NewAssembler(s.ctx) + output, err := assembler.Run(contents) + s.Require().NoError(err) + // Each block applies its own prefix/suffix independently + s.Equal(`prefix1[ab][cd]suffix2`, output) +} + +func (s *assemblerTestSuite) TestAssemble_BlockScopedPrefixSuffix_NestedPattern() { + // Test the pattern from 934160.ra - prefix/suffix at start of assemble block + contents := `##!> assemble +##!^ while\s*\([\s(]* +##!$ .*\) +true +false +##!<` + assembler := NewAssembler(s.ctx) + output, err := assembler.Run(contents) + s.Require().NoError(err) + // Note: assembler applies various transformations like \s -> [\s\x0b] + // We just check that prefix/suffix are applied + s.Contains(output, `while`) + s.Contains(output, `.*\)`) + s.Contains(output, `tru`) + s.Contains(output, `fals`) +} + +// writeIncludeFile writes an include file that `##!> include name` will resolve. +func (s *assemblerTestSuite) writeIncludeFile(name string, contents string) { + s.T().Helper() + includeDir := path.Join(s.tempDir, "regex-assembly", "include") + s.Require().NoError(os.MkdirAll(includeDir, fs.ModePerm)) + s.Require().NoError(os.WriteFile(path.Join(includeDir, name+".ra"), []byte(contents), fs.ModePerm)) +} + +func (s *assemblerTestSuite) TestAssemble_IncludedPrefixIsScopedToIncludedContent() { + s.writeIncludeFile("scoped_prefix", "##!^ PRE_\nalpha\n") + + // `beta` belongs to the including file and must not pick up the include's prefix, + // no matter which side of the include it sits on. + for _, tt := range []struct { + name string + contents string + expected string + }{ + {"caller line after include", "##!> include scoped_prefix\nbeta", `(?:PRE_alph|bet)a`}, + {"caller line before include", "beta\n##!> include scoped_prefix", `(?:bet|PRE_alph)a`}, + {"caller lines on both sides", "gamma\n##!> include scoped_prefix\nbeta", `(?:gamm|PRE_alph|bet)a`}, + } { + s.Run(tt.name, func() { + output, err := NewAssembler(s.ctx).Run(tt.contents) + s.Require().NoError(err) + s.Equal(tt.expected, output) + }) + } +} + +func (s *assemblerTestSuite) TestAssemble_IncludedSuffixIsScopedToIncludedContent() { + s.writeIncludeFile("scoped_suffix", "##!$ _SUF\nalpha\n") + + output, err := NewAssembler(s.ctx).Run("##!> include scoped_suffix\nbeta") + s.Require().NoError(err) + s.Equal(`alpha_SUF|beta`, output) +} + +func (s *assemblerTestSuite) TestAssemble_PrefixSuffixWithStashing() { + // Test that prefixes/suffixes are applied when storing expressions with ##!=< + contents := `##!> assemble +##!^ prefix +##!$ suffix +a +b +##!=< stored +##!< +##!=> stored` + assembler := NewAssembler(s.ctx) + output, err := assembler.Run(contents) + s.Require().NoError(err) + // The stored expression should include the prefix/suffix from its block + s.Equal(`prefix[ab]suffix`, output) +} + +func (s *assemblerTestSuite) TestAssemble_PrefixSuffixWithStashing_MultipleBlocks() { + // Test that each block's prefix/suffix are independently applied when stashing + contents := `##!> assemble +##!^ prefix1 +##!$ suffix1 +a +b +##!=< block1 +##!< +##!> assemble +##!^ prefix2 +##!$ suffix2 +c +d +##!=< block2 +##!< +##!=> block1 +##!=> block2` + assembler := NewAssembler(s.ctx) + output, err := assembler.Run(contents) + s.Require().NoError(err) + // Each stored expression should have its own block's prefix/suffix + s.Equal(`prefix1[ab]suffix1prefix2[cd]suffix2`, output) +} + +func (s *assemblerTestSuite) TestAssemble_PrefixSuffixWithStashing_NoPrefix() { + // Test stashing without prefix/suffix works as before + contents := `##!> assemble +a +b +##!=< stored +##!< +##!=> stored` + assembler := NewAssembler(s.ctx) + output, err := assembler.Run(contents) + s.Require().NoError(err) + // No prefix/suffix should be applied + s.Equal(`[ab]`, output) +} + +func (s *assemblerTestSuite) TestAssemble_PrefixSuffixWithStashing_OnlyPrefix() { + // Test stashing with only prefix + contents := `##!> assemble +##!^ prefix +a +b +##!=< stored +##!< +##!=> stored` + assembler := NewAssembler(s.ctx) + output, err := assembler.Run(contents) + s.Require().NoError(err) + // Only prefix should be applied + s.Equal(`prefix[ab]`, output) +} + +func (s *assemblerTestSuite) TestAssemble_BlockScopedPrefixSuffix_MultipleNestingLevels() { + // Test that prefix/suffix is correctly scoped when blocks are nested multiple levels deep. + // The outer block's prefix/suffix wraps the inner block's output, while the inner block's + // prefix/suffix only applies to the inner block's content. + contents := `##!> assemble +##!^ outer_prefix +##!$ outer_suffix +##!> assemble +##!^ inner_prefix +##!$ inner_suffix +a +b +##!< +##!<` + assembler := NewAssembler(s.ctx) + output, err := assembler.Run(contents) + s.Require().NoError(err) + s.Equal(`outer_prefixinner_prefix[ab]inner_suffixouter_suffix`, output) +} + +func (s *assemblerTestSuite) TestAssemble_PrefixSuffixWithStashing_OnlySuffix() { + // Test stashing with only suffix + contents := `##!> assemble +##!$ suffix +a +b +##!=< stored +##!< +##!=> stored` + assembler := NewAssembler(s.ctx) + output, err := assembler.Run(contents) + s.Require().NoError(err) + // Only suffix should be applied + s.Equal(`[ab]suffix`, output) +} diff --git a/regex/parser/include_test.go b/regex/parser/include_test.go index f6c39ec..0b46024 100644 --- a/regex/parser/include_test.go +++ b/regex/parser/include_test.go @@ -72,11 +72,11 @@ func (s *parserIncludeTestSuite) TestParserInclude_Prefixes() { included regex`, "data regex") parser := NewParser(s.ctx, s.reader) actual := parser.Parse(false) + // Directives are passed through as-is, wrapped in an assemble block that scopes + // them to the included content instead of the rest of the including file. expected := bytes.NewBufferString(`##!> assemble -prefix1 -##!=> -prefix2 -##!=> +##!^ prefix1 +##!^ prefix2 included regex ##!< data regex @@ -90,13 +90,12 @@ func (s *parserIncludeTestSuite) TestParserInclude_Suffixes() { included regex`, "data regex") parser := NewParser(s.ctx, s.reader) actual := parser.Parse(false) + // Directives are passed through as-is, wrapped in an assemble block that scopes + // them to the included content instead of the rest of the including file. expected := bytes.NewBufferString(`##!> assemble +##!$ suffix1 +##!$ suffix2 included regex -##!=> -suffix1 -##!=> -suffix2 -##!=> ##!< data regex `) @@ -112,17 +111,14 @@ func (s *parserIncludeTestSuite) TestParserInclude_FlagsPrefixesSuffixes() { included regex`, "data regex") parser := NewParser(s.ctx, s.reader) actual := parser.Parse(false) + // Directives are passed through as-is, wrapped in an assemble block that scopes + // them to the included content instead of the rest of the including file. expected := bytes.NewBufferString(`##!> assemble -prefix1 -##!=> -prefix2 -##!=> +##!$ suffix1 +##!$ suffix2 +##!^ prefix1 +##!^ prefix2 included regex -##!=> -suffix1 -##!=> -suffix2 -##!=> ##!< data regex `) diff --git a/regex/parser/parser.go b/regex/parser/parser.go index b6816a3..fb5fcbd 100644 --- a/regex/parser/parser.go +++ b/regex/parser/parser.go @@ -150,9 +150,15 @@ func (p *Parser) Parse(formatOnly bool) *bytes.Buffer { } } case prefix: + // Pass the directive through for the Assemble processor to apply (block-scoped). + // Prefixes is still recorded so that parseFile knows whether an included file + // needs its own assemble block to scope these directives to its content. p.Prefixes = append(p.Prefixes, parsedLine.prefix) + text = line + "\n" case suffix: + // See the prefix case above. p.Suffixes = append(p.Suffixes, parsedLine.suffix) + text = line + "\n" } if formatOnly { text = line + "\n" @@ -275,7 +281,7 @@ func parseFile(rootParser *Parser, filename string, definitions map[string]strin newP.variables = definitions } out := newP.Parse(false) - newOut, err := mergePrefixesSuffixes(newP, out) + newOut, err := scopeIncludedFileDirectives(newP, out) if err != nil { logger.Fatal().Msgf("error parsing file: %v", err.Error()) } @@ -283,16 +289,18 @@ func parseFile(rootParser *Parser, filename string, definitions map[string]strin return newOut, newP.variables } -// Merge prefixes, and suffixes from include files into another parser. -// All of these need to be treated as local to the source parser. +// Scope prefixes and suffixes from an included file to the content of that file. +// The directives themselves are passed through as raw lines for the Assemble processor +// to apply, so they need an assemble block of their own. Without it they stay active +// for the rest of the including file and wrap the caller's content as well. // We removed flag merging because of https://github.com/coreruleset/crs-toolchain/issues/72 -func mergePrefixesSuffixes(source *Parser, out *bytes.Buffer) (*bytes.Buffer, error) { - logger.Trace().Msg("merging prefixes, suffixes from included file") +func scopeIncludedFileDirectives(source *Parser, out *bytes.Buffer) (*bytes.Buffer, error) { + logger.Trace().Msg("scoping prefixes, suffixes from included file") // If the included file has flags, this is an error if len(source.Flags) > 0 { return new(bytes.Buffer), errors.New("include files must not contain flags. See https://github.com/coreruleset/crs-toolchain/v2/issues/71") } - // IMPORTANT: don't write the assemble block at all if there are no flags, prefixes, or + // IMPORTANT: don't write the assemble block at all if there are no prefixes or // suffixes. Enclosing the output in an assemble block can change the semantics, for example, // when the included content is processed by the cmdline processor in the including file. if len(source.Prefixes) == 0 && len(source.Suffixes) == 0 { @@ -301,32 +309,12 @@ func mergePrefixesSuffixes(source *Parser, out *bytes.Buffer) (*bytes.Buffer, er newOut := new(bytes.Buffer) newOut.WriteString("##!> assemble\n") - - for _, prefix := range source.Prefixes { - newOut.WriteString(prefix) - newOut.WriteString("\n##!=>\n") - } if _, err := out.WriteTo(newOut); err != nil { logger.Fatal().Err(err).Msg("failed to copy output to new buffer") } - - sawNewLine := false - if err := out.UnreadByte(); err == nil { - lastByte, err := out.ReadByte() - if err == nil { - sawNewLine = lastByte == 13 - } - } - if sawNewLine { + if !bytes.HasSuffix(newOut.Bytes(), []byte("\n")) { newOut.WriteString("\n") } - if len(source.Suffixes) > 0 { - newOut.WriteString("##!=>\n") - } - for _, suffix := range source.Suffixes { - newOut.WriteString(suffix) - newOut.WriteString("\n##!=>\n") - } newOut.WriteString("##!<\n") return newOut, nil } diff --git a/regex/processors/assemble.go b/regex/processors/assemble.go index c3272c4..6c2c93e 100644 --- a/regex/processors/assemble.go +++ b/regex/processors/assemble.go @@ -19,8 +19,10 @@ const ( ) type Assemble struct { - proc *Processor - output strings.Builder + proc *Processor + output strings.Builder + prefixes []string + suffixes []string } // NewAssemble creates a new assemble processor @@ -32,7 +34,23 @@ func NewAssemble(ctx *Context) *Assemble { // ProcessLine applies the processors logic to a single line func (a *Assemble) ProcessLine(line string) error { - match := regex.AssembleInputRegex.FindStringSubmatch(line) + // Check for prefix directive + match := regex.PrefixRegex.FindStringSubmatch(line) + if len(match) > 0 { + a.prefixes = append(a.prefixes, match[1]) + logger.Trace().Msgf("Added block-scoped prefix: %s", match[1]) + return nil + } + + // Check for suffix directive + match = regex.SuffixRegex.FindStringSubmatch(line) + if len(match) > 0 { + a.suffixes = append(a.suffixes, match[1]) + logger.Trace().Msgf("Added block-scoped suffix: %s", match[1]) + return nil + } + + match = regex.AssembleInputRegex.FindStringSubmatch(line) if len(match) > 0 { if err := a.store(match[1]); err != nil { logger.Error().Err(err).Msgf("Failed to store input: %s", line) @@ -69,6 +87,13 @@ func (a *Assemble) Complete() ([]string, error) { } result := a.wrapCompletedAssembly(regex) + + // Apply block-scoped prefixes and suffixes + if len(a.prefixes) > 0 || len(a.suffixes) > 0 { + result = strings.Join(a.prefixes, "") + result + strings.Join(a.suffixes, "") + logger.Trace().Msgf("Applied block-scoped prefixes/suffixes: %s", result) + } + logger.Trace().Msgf("Completed assembly: %s", result) if result == "" { @@ -102,6 +127,18 @@ func (a *Assemble) store(identifier string) error { // the value we just stored a.output.Reset() + // Apply block-scoped prefixes and suffixes to stored expressions + if len(a.prefixes) > 0 || len(a.suffixes) > 0 { + prefixes := strings.Join(a.prefixes, "") + suffixes := strings.Join(a.suffixes, "") + outputString = prefixes + outputString + suffixes + logger.Trace().Msgf("Applied block-scoped prefixes/suffixes to stored expression: %s", outputString) + // Clear prefixes/suffixes after applying to stored expression + // so they won't be applied again in Complete() + a.prefixes = nil + a.suffixes = nil + } + logger.Debug().Msgf("Storing expression at %s: %s", identifier, outputString) a.proc.ctx.stash[identifier] = outputString return nil