diff --git a/changelog/unreleased/issue-23541.toml b/changelog/unreleased/issue-23541.toml new file mode 100644 index 0000000000..1cec4035bf --- /dev/null +++ b/changelog/unreleased/issue-23541.toml @@ -0,0 +1,6 @@ +type = "fixed" +message = "Fix parsing of field values >999 in Rule Builder conditional." + +issues = ["23541"] +pulls = ["25218"] + diff --git a/graylog2-server/src/main/java/org/graylog/plugins/pipelineprocessor/rulebuilder/RuleBuilderRegistry.java b/graylog2-server/src/main/java/org/graylog/plugins/pipelineprocessor/rulebuilder/RuleBuilderRegistry.java index 01ef3f4c63..31d1e2b105 100644 --- a/graylog2-server/src/main/java/org/graylog/plugins/pipelineprocessor/rulebuilder/RuleBuilderRegistry.java +++ b/graylog2-server/src/main/java/org/graylog/plugins/pipelineprocessor/rulebuilder/RuleBuilderRegistry.java @@ -131,6 +131,7 @@ public class RuleBuilderRegistry { Map fragments = conditionsWithInternal(); fragments.putAll(actionsWithInternal()); final Configuration config = secureFreemarkerConfigProvider.get(); + config.setNumberFormat("computer"); StringTemplateLoader stringTemplateLoader = new StringTemplateLoader(); fragments.entrySet().stream().filter(c -> c.getValue().isFragment()).forEach(c -> stringTemplateLoader.putTemplate(c.getKey(), c.getValue().fragment())); config.setTemplateLoader(stringTemplateLoader); diff --git a/graylog2-server/src/main/java/org/graylog/plugins/pipelineprocessor/rulebuilder/parser/RuleBuilderService.java b/graylog2-server/src/main/java/org/graylog/plugins/pipelineprocessor/rulebuilder/parser/RuleBuilderService.java index d4a42108db..6d48781ac4 100644 --- a/graylog2-server/src/main/java/org/graylog/plugins/pipelineprocessor/rulebuilder/parser/RuleBuilderService.java +++ b/graylog2-server/src/main/java/org/graylog/plugins/pipelineprocessor/rulebuilder/parser/RuleBuilderService.java @@ -60,6 +60,7 @@ public class RuleBuilderService { this.conditionParser = conditionParser; this.actionParser = actionParser; this.freemarkerConfiguration = secureFreemarkerConfigProvider.get(); + this.freemarkerConfiguration.setNumberFormat("computer"); StringTemplateLoader templateLoader = new StringTemplateLoader(); conditionParser.getConditions().forEach((key, value) -> templateLoader.putTemplate(key, value.descriptor().ruleBuilderTitle())); actionParser.getActions().forEach((key, value) -> templateLoader.putTemplate(key, value.descriptor().ruleBuilderTitle())); diff --git a/graylog2-server/src/test/java/org/graylog/plugins/pipelineprocessor/rulebuilder/parser/ParserUtilTest.java b/graylog2-server/src/test/java/org/graylog/plugins/pipelineprocessor/rulebuilder/parser/ParserUtilTest.java index 373fab29fb..4fbe285e75 100644 --- a/graylog2-server/src/test/java/org/graylog/plugins/pipelineprocessor/rulebuilder/parser/ParserUtilTest.java +++ b/graylog2-server/src/test/java/org/graylog/plugins/pipelineprocessor/rulebuilder/parser/ParserUtilTest.java @@ -47,10 +47,12 @@ public class ParserUtilTest { public void initializeFreemarkerConfig() { SecureFreemarkerConfigProvider secureFreemarkerConfigProvider = new SecureFreemarkerConfigProvider(); this.configuration = secureFreemarkerConfigProvider.get(); + configuration.setNumberFormat("computer"); configuration.setLogTemplateExceptions(false); StringTemplateLoader templateLoader = new StringTemplateLoader(); templateLoader.putTemplate("test_fragment1", "let gl2_fragmentvar_v1 = $message.${field};"); templateLoader.putTemplate("test_fragment2", "let gl2_fragmentvar_v1 = $message.${field!\"defaultField\"};"); + templateLoader.putTemplate("test_numeric_fragment", "( has_field(${field}) && to_long($message.${field}) <= ${fieldValue} )"); configuration.setTemplateLoader(templateLoader); } @@ -212,5 +214,14 @@ public class ParserUtilTest { .isEqualTo("let gl2_fragmentvar_v1 = $message.\"my_field\";"); } + @Test + public void generateForFragmentFormatsLargeNumbersWithoutThousandSeparators() { + final RuleBuilderStep step = mock(RuleBuilderStep.class); + when(step.function()).thenReturn("test_numeric_fragment"); + final Map params = Map.of("field", "bytes", "fieldValue", 10000); + when(step.parameters()).thenReturn(params); + assertThat(ParserUtil.generateForFragment(step, configuration)) + .isEqualTo("( has_field(\"bytes\") && to_long($message.\"bytes\") <= 10000 )"); + } } diff --git a/graylog2-server/src/test/java/org/graylog/plugins/pipelineprocessor/rulebuilder/parser/RuleBuilderServiceTest.java b/graylog2-server/src/test/java/org/graylog/plugins/pipelineprocessor/rulebuilder/parser/RuleBuilderServiceTest.java index ac1e346d4d..946a3ef187 100644 --- a/graylog2-server/src/test/java/org/graylog/plugins/pipelineprocessor/rulebuilder/parser/RuleBuilderServiceTest.java +++ b/graylog2-server/src/test/java/org/graylog/plugins/pipelineprocessor/rulebuilder/parser/RuleBuilderServiceTest.java @@ -16,7 +16,11 @@ */ package org.graylog.plugins.pipelineprocessor.rulebuilder.parser; +import com.google.common.collect.ImmutableList; +import org.graylog.plugins.pipelineprocessor.ast.functions.FunctionDescriptor; import org.graylog.plugins.pipelineprocessor.rulebuilder.RuleBuilder; +import org.graylog.plugins.pipelineprocessor.rulebuilder.RuleBuilderStep; +import org.graylog.plugins.pipelineprocessor.rulebuilder.db.RuleFragment; import org.graylog2.bindings.providers.SecureFreemarkerConfigProvider; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -27,8 +31,12 @@ import org.mockito.junit.jupiter.MockitoSettings; import org.mockito.quality.Strictness; import java.util.HashMap; +import java.util.List; +import java.util.Map; import static org.assertj.core.api.Assertions.assertThat; +import static org.graylog.plugins.pipelineprocessor.ast.functions.ParameterDescriptor.integer; +import static org.graylog.plugins.pipelineprocessor.ast.functions.ParameterDescriptor.string; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyInt; @@ -65,5 +73,39 @@ public class RuleBuilderServiceTest { end"""); } + @Test + public void generateTitlesFormatsLargeNumbersWithoutThousandSeparators() { + final String conditionName = "has_field_greater_or_equal"; + final RuleFragment fragment = RuleFragment.builder() + .fragment("( has_field(${field}) && to_long($message.${field}) >= ${fieldValue} )") + .descriptor(FunctionDescriptor.builder() + .name(conditionName) + .params(ImmutableList.of( + string("field").build(), + integer("fieldValue").build() + )) + .returnType(Boolean.class) + .ruleBuilderEnabled() + .ruleBuilderTitle("Field '${field}' greater than or equal '${fieldValue}'") + .build()) + .isCondition() + .build(); + when(conditionParser.getConditions()).thenReturn(Map.of(conditionName, fragment)); + when(actionParser.getActions()).thenReturn(new HashMap<>()); + final RuleBuilderService service = new RuleBuilderService(conditionParser, actionParser, new SecureFreemarkerConfigProvider()); + + final RuleBuilderStep step = RuleBuilderStep.builder() + .function(conditionName) + .parameters(Map.of("field", "bytes", "fieldValue", 10000)) + .build(); + final RuleBuilder ruleBuilder = RuleBuilder.builder() + .conditions(List.of(step)) + .build(); + + final RuleBuilder result = service.generateTitles(ruleBuilder); + + assertThat(result.conditions().get(0).title()) + .isEqualTo("Field 'bytes' greater than or equal '10000'"); + } }