From f5077f2700f47c45b618c4c61c721c59b44f0948 Mon Sep 17 00:00:00 2001 From: Florian Petersen <188503754+fpetersen-gl@users.noreply.github.com> Date: Thu, 30 Jul 2026 11:50:51 +0200 Subject: [PATCH 1/2] Skip CEF fields that carry an empty numeric value instead of logging a stack trace for each message. --- changelog/unreleased/issue-26833.toml | 5 + .../plugins/cef/parser/CEFMapping.java | 18 +- .../plugins/cef/parser/MappedMessage.java | 13 +- .../plugins/cef/parser/MappedMessageTest.java | 196 ++++++++++++++++++ 4 files changed, 223 insertions(+), 9 deletions(-) create mode 100644 changelog/unreleased/issue-26833.toml create mode 100644 graylog2-server/src/test/java/org/graylog/plugins/cef/parser/MappedMessageTest.java diff --git a/changelog/unreleased/issue-26833.toml b/changelog/unreleased/issue-26833.toml new file mode 100644 index 000000000000..9687dd3d0a3c --- /dev/null +++ b/changelog/unreleased/issue-26833.toml @@ -0,0 +1,5 @@ +type = "f" +message = "Skip CEF fields that carry an empty numeric value instead of logging a stack trace for each message." + +issues = ["26833"] +pulls = [""] diff --git a/graylog2-server/src/main/java/org/graylog/plugins/cef/parser/CEFMapping.java b/graylog2-server/src/main/java/org/graylog/plugins/cef/parser/CEFMapping.java index 6eccfe97bcc6..45efed92807c 100644 --- a/graylog2-server/src/main/java/org/graylog/plugins/cef/parser/CEFMapping.java +++ b/graylog2-server/src/main/java/org/graylog/plugins/cef/parser/CEFMapping.java @@ -17,9 +17,9 @@ package org.graylog.plugins.cef.parser; import com.google.common.collect.ImmutableMap; +import jakarta.annotation.Nullable; import org.joda.time.DateTime; -import javax.annotation.Nullable; import java.math.BigInteger; import java.util.function.Function; @@ -228,24 +228,29 @@ private static String convertString(String s) { return s; } + @Nullable private static Float convertFloat(String s) { - return Float.parseFloat(s); + return s.isBlank() ? null : Float.parseFloat(s); } + @Nullable private static Double convertDouble(String s) { - return Double.parseDouble(s); + return s.isBlank() ? null : Double.parseDouble(s); } + @Nullable private static Long convertLong(String s) { - return Long.parseLong(s); + return s.isBlank() ? null : Long.parseLong(s); } + @Nullable private static BigInteger convertBigInteger(String s) { - return new BigInteger(s); + return s.isBlank() ? null : new BigInteger(s); } + @Nullable private static Integer convertInteger(String s) { - return Integer.parseInt(s); + return s.isBlank() ? null : Integer.parseInt(s); } private static Object convertTimestamp(String s) { @@ -309,6 +314,7 @@ public String getFullName() { return converter; } + @Nullable public Object convert(String s) { return converter.apply(s); } diff --git a/graylog2-server/src/main/java/org/graylog/plugins/cef/parser/MappedMessage.java b/graylog2-server/src/main/java/org/graylog/plugins/cef/parser/MappedMessage.java index 40681ed0118d..f65208db9bb9 100644 --- a/graylog2-server/src/main/java/org/graylog/plugins/cef/parser/MappedMessage.java +++ b/graylog2-server/src/main/java/org/graylog/plugins/cef/parser/MappedMessage.java @@ -47,14 +47,21 @@ private Map mapExtensions(Map extensions) { } final CEFMapping fieldMapping = CEFMapping.forKeyName(keyName); + final String value = extension.getValue(); if (fieldMapping != null) { try { - mappedExtensions.put(getLabel(keyName, fieldMapping.getFullName(), extensions), fieldMapping.convert(extension.getValue())); + final Object converted = fieldMapping.convert(value); + if (converted == null) { + LOG.trace("CEF field [{}] has an empty value. Skipping.", keyName); + } else { + mappedExtensions.put(getLabel(keyName, fieldMapping.getFullName(), extensions), converted); + } } catch (Exception e) { - LOG.warn("Could not transform CEF field [{}] according to standard. Skipping.", keyName, e); + LOG.warn("Could not transform CEF field [{}] with value [{}] according to standard. Skipping.", + keyName, value); } } else { - mappedExtensions.put(getLabel(keyName, keyName, extensions), extension.getValue()); + mappedExtensions.put(getLabel(keyName, keyName, extensions), value); } } diff --git a/graylog2-server/src/test/java/org/graylog/plugins/cef/parser/MappedMessageTest.java b/graylog2-server/src/test/java/org/graylog/plugins/cef/parser/MappedMessageTest.java new file mode 100644 index 000000000000..1c0b61cd248d --- /dev/null +++ b/graylog2-server/src/test/java/org/graylog/plugins/cef/parser/MappedMessageTest.java @@ -0,0 +1,196 @@ +/* + * Copyright (C) 2020 Graylog, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * . + */ +package org.graylog.plugins.cef.parser; + +import com.github.jcustenborder.cef.CEFParser; +import com.github.jcustenborder.cef.CEFParserFactory; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.core.LogEvent; +import org.apache.logging.log4j.core.Logger; +import org.apache.logging.log4j.core.appender.AbstractAppender; +import org.apache.logging.log4j.core.config.Configurator; +import org.apache.logging.log4j.core.config.Property; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.math.BigInteger; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class MappedMessageTest { + private static final String HEADER = "CEF:0|SecureAuth|SecureAuth IdP|9.3|90030|Application - End request|4|"; + + private final CEFParser parser = CEFParserFactory.create(); + + private Logger logger; + private Level originalLevel; + private CapturingAppender appender; + + @BeforeEach + void captureLogOutput() { + appender = new CapturingAppender(); + appender.start(); + logger = (Logger) LogManager.getLogger(MappedMessage.class); + originalLevel = logger.getLevel(); + logger.addAppender(appender); + Configurator.setLevel(MappedMessage.class.getName(), Level.TRACE); + } + + @AfterEach + void releaseLogOutput() { + logger.removeAppender(appender); + appender.stop(); + Configurator.setLevel(MappedMessage.class.getName(), originalLevel); + } + + private MappedMessage parse(String extensions, boolean useFullNames) { + return new MappedMessage(parser.parse(HEADER + extensions), useFullNames); + } + + private Map mapExtensions(String extensions) { + return parse(extensions, false).mappedExtensions(); + } + + private List warnings() { + return appender.events.stream() + .filter(event -> event.getLevel().isMoreSpecificThan(Level.WARN)) + .toList(); + } + + // An empty numeric value is a quirk of the sending device, not an operator problem, so it must + // not produce a warning. Devices that send one on every message used to flood server.log. + @Test + void emptyNumericValueIsNotLoggedAsAWarning() { + mapExtensions("cfp1= cs1=AUDIT src=10.0.0.1"); + + assertTrue(warnings().isEmpty(), "Expected no warning for an empty numeric value, got: " + warnings()); + } + + // A value we genuinely cannot parse is worth reporting, but the stack trace is identical every + // time and tells the operator nothing. The offending value does. + @Test + void malformedNumericValueIsLoggedWithoutAStackTrace() { + mapExtensions("cfp1=abc cs1=AUDIT"); + + final List warnings = warnings(); + assertEquals(1, warnings.size(), "Expected exactly one warning, got: " + warnings); + + final LogEvent event = warnings.getFirst(); + assertNull(event.getThrown(), "Warning must not carry a throwable"); + + final String message = event.getMessage().getFormattedMessage(); + assertTrue(message.contains("cfp1"), "Warning should name the field, got: " + message); + assertTrue(message.contains("abc"), "Warning should include the offending value, got: " + message); + } + + @Test + void emptyNumericValueIsSkipped() { + final MappedMessage message = parse("cfp1= cs1=AUDIT src=10.0.0.1", false); + + // The empty value does reach us from the CEF parser, it is dropped while mapping. + assertEquals("", message.extensions().get("cfp1")); + + final Map fields = message.mappedExtensions(); + assertFalse(fields.containsKey("cfp1")); + assertEquals("AUDIT", fields.get("cs1")); + assertEquals("10.0.0.1", fields.get("src")); + } + + @Test + void emptyValueIsSkippedForEveryNumericType() { + final Map fields = mapExtensions("cfp1= dlat= flexNumber1= cn1= type= cs1=AUDIT"); + + assertFalse(fields.containsKey("cfp1"), "Float"); + assertFalse(fields.containsKey("dlat"), "Double"); + assertFalse(fields.containsKey("flexNumber1"), "Long"); + assertFalse(fields.containsKey("cn1"), "BigInteger"); + assertFalse(fields.containsKey("type"), "Integer"); + assertEquals("AUDIT", fields.get("cs1")); + assertTrue(warnings().isEmpty(), "Expected no warnings, got: " + warnings()); + } + + @Test + void validNumericValueIsConverted() { + final Map fields = mapExtensions("cfp1=2.5 dlat=51.5 flexNumber1=7 cn1=42 type=1"); + + assertEquals(2.5f, fields.get("cfp1")); + assertEquals(51.5d, fields.get("dlat")); + assertEquals(7L, fields.get("flexNumber1")); + assertEquals(new BigInteger("42"), fields.get("cn1")); + assertEquals(1, fields.get("type")); + } + + // Empty values are dropped for numeric fields only. String and unmapped fields keep the empty + // value so that existing has_field()/_exists_ searches keep behaving the same way. + @Test + void emptyStringValueIsRetained() { + final Map fields = mapExtensions("cs1= cs2=AUDIT"); + + assertTrue(fields.containsKey("cs1")); + assertEquals("", fields.get("cs1")); + } + + @Test + void emptyUnmappedValueIsRetained() { + final Map fields = mapExtensions("someVendorField= cs1=AUDIT"); + + assertTrue(fields.containsKey("someVendorField")); + assertEquals("", fields.get("someVendorField")); + } + + // A label renames its field, so dropping an empty numeric value must not leave the label behind + // as a field of its own. + @Test + void emptyNumericValueWithLabelIsSkipped() { + final Map fields = mapExtensions("cfp1Label=RiskScore cfp1= cs1=AUDIT"); + + assertFalse(fields.containsKey("cfp1")); + assertFalse(fields.containsKey("cfp1Label")); + assertFalse(fields.containsKey("RiskScore")); + assertEquals("AUDIT", fields.get("cs1")); + } + + @Test + void emptyNumericValueIsSkippedWhenUsingFullNames() { + final Map fields = parse("cfp1= cs1=AUDIT", true).mappedExtensions(); + + assertFalse(fields.containsKey("deviceCustomFloatingPoint1")); + assertFalse(fields.containsKey("cfp1")); + assertEquals("AUDIT", fields.get("deviceCustomString1")); + } + + private static final class CapturingAppender extends AbstractAppender { + private final List events = new ArrayList<>(); + + private CapturingAppender() { + super("capturing", null, null, true, Property.EMPTY_ARRAY); + } + + @Override + public void append(LogEvent event) { + events.add(event.toImmutable()); + } + } +} From 4a12885a71dee3c24c7beafbd5f6426f15380b17 Mon Sep 17 00:00:00 2001 From: Florian Petersen <188503754+fpetersen-gl@users.noreply.github.com> Date: Thu, 30 Jul 2026 12:08:56 +0200 Subject: [PATCH 2/2] Add PR to CL --- changelog/unreleased/issue-26833.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/unreleased/issue-26833.toml b/changelog/unreleased/issue-26833.toml index 9687dd3d0a3c..24cacb11b552 100644 --- a/changelog/unreleased/issue-26833.toml +++ b/changelog/unreleased/issue-26833.toml @@ -2,4 +2,4 @@ type = "f" message = "Skip CEF fields that carry an empty numeric value instead of logging a stack trace for each message." issues = ["26833"] -pulls = [""] +pulls = ["26835"]