diff --git a/config/jvm.options b/config/jvm.options index b767025f662..a00db32e9a3 100644 --- a/config/jvm.options +++ b/config/jvm.options @@ -79,11 +79,11 @@ # text values with sizes less than or equal to this limit will be treated as invalid. # This value should be higher than `logstash.jackson.stream-read-constraints.max-number-length`. # The jackson library defaults to 20000000 or 20MB, whereas Logstash defaults to 200MB or 200000000 characters. --Dlogstash.jackson.stream-read-constraints.max-string-length=200000000 +#-Dlogstash.jackson.stream-read-constraints.max-string-length=200000000 # # Sets the maximum number length (in chars or bytes, depending on input context). # The jackson library defaults to 1000, whereas Logstash defaults to 10000. --Dlogstash.jackson.stream-read-constraints.max-number-length=10000 +#-Dlogstash.jackson.stream-read-constraints.max-number-length=10000 # # Sets the maximum nesting depth. The depth is a count of objects and arrays that have not # been closed, `{` and `[` respectively. diff --git a/logstash-core/src/main/java/org/logstash/jackson/StreamReadConstraintsUtil.java b/logstash-core/src/main/java/org/logstash/jackson/StreamReadConstraintsUtil.java index 606cc313ba1..b34139a271c 100644 --- a/logstash-core/src/main/java/org/logstash/jackson/StreamReadConstraintsUtil.java +++ b/logstash-core/src/main/java/org/logstash/jackson/StreamReadConstraintsUtil.java @@ -5,10 +5,16 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import java.util.*; +import javax.annotation.Nullable; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.Properties; +import java.util.Set; import java.util.function.BiConsumer; -import java.util.function.BiFunction; -import java.util.function.Function; import java.util.stream.Collectors; public class StreamReadConstraintsUtil { @@ -18,38 +24,51 @@ public class StreamReadConstraintsUtil { private StreamReadConstraints configuredStreamReadConstraints; - // Provide default values for Jackson constraints in the case they are - // not specified in configuration file. - private static final Map JACKSON_DEFAULTS = Map.of( - Override.MAX_STRING_LENGTH, 200_000_000, - Override.MAX_NUMBER_LENGTH, 10_000, - Override.MAX_NESTING_DEPTH, 1_000 - ); - enum Override { - MAX_STRING_LENGTH(StreamReadConstraints.Builder::maxStringLength, StreamReadConstraints::getMaxStringLength), - MAX_NUMBER_LENGTH(StreamReadConstraints.Builder::maxNumberLength, StreamReadConstraints::getMaxNumberLength), - MAX_NESTING_DEPTH(StreamReadConstraints.Builder::maxNestingDepth, StreamReadConstraints::getMaxNestingDepth), + MAX_STRING_LENGTH(200_000_000, StreamReadConstraints.Builder::maxStringLength, StreamReadConstraints::getMaxStringLength), + MAX_NUMBER_LENGTH(10_000, StreamReadConstraints.Builder::maxNumberLength, StreamReadConstraints::getMaxNumberLength), + MAX_NESTING_DEPTH(1_000, StreamReadConstraints.Builder::maxNestingDepth, StreamReadConstraints::getMaxNestingDepth), ; static final String PROP_PREFIX = "logstash.jackson.stream-read-constraints."; final String propertyName; + final int defaultValue; private final IntValueApplicator applicator; private final IntValueObserver observer; - Override(final IntValueApplicator applicator, + Override(final int defaultValue, + final IntValueApplicator applicator, final IntValueObserver observer) { this.propertyName = PROP_PREFIX + this.name().toLowerCase().replace('_', '-'); + this.defaultValue = defaultValue; this.applicator = applicator; this.observer = observer; } + int resolve(final Integer specifiedValue) { + return Objects.requireNonNullElse(specifiedValue, defaultValue); + } + + void apply(final StreamReadConstraints.Builder constraintsBuilder, + @Nullable final Integer specifiedValue) { + this.applicator.applyIntValue(constraintsBuilder, resolve(specifiedValue)); + } + + int observe(final StreamReadConstraints constraints) { + return this.observer.observeIntValue(constraints); + } + @FunctionalInterface - interface IntValueObserver extends Function {} + interface IntValueObserver { + int observeIntValue(final StreamReadConstraints constraints); + } @FunctionalInterface - interface IntValueApplicator extends BiFunction {} + interface IntValueApplicator { + @SuppressWarnings("UnusedReturnValue") + StreamReadConstraints.Builder applyIntValue(final StreamReadConstraints.Builder constraintsBuilder, final int value); + } } /** @@ -86,9 +105,7 @@ StreamReadConstraints get() { if (configuredStreamReadConstraints == null) { final StreamReadConstraints.Builder builder = StreamReadConstraints.defaults().rebuild(); - // Apply the Jackson defaults first, then the overrides from config - JACKSON_DEFAULTS.forEach((override, value) -> override.applicator.apply(builder, value)); - eachOverride((override, value) -> override.applicator.apply(builder, value)); + eachOverride((override, specifiedValue) -> override.apply(builder, specifiedValue)); this.configuredStreamReadConstraints = builder.build(); } @@ -106,11 +123,14 @@ public void validateIsGlobalDefault() { private void validate(final StreamReadConstraints streamReadConstraints) { final List fatalIssues = new ArrayList<>(); eachOverride((override, specifiedValue) -> { - final Integer effectiveValue = override.observer.apply(streamReadConstraints); - if (Objects.equals(specifiedValue, effectiveValue)) { - logger.info("Jackson default value override `{}` configured to `{}`", override.propertyName, specifiedValue); + final int effectiveValue = override.observe(streamReadConstraints); + final int expectedValue = override.resolve(specifiedValue); + + if (!Objects.equals(effectiveValue, expectedValue)) { + fatalIssues.add(String.format("`%s` (expected: `%s`, actual: `%s`)", override.propertyName, expectedValue, effectiveValue)); } else { - fatalIssues.add(String.format("`%s` (expected: `%s`, actual: `%s`)", override.propertyName, specifiedValue, effectiveValue)); + final String reason = Objects.nonNull(specifiedValue) ? "system properties" : "logstash default"; + logger.info("Jackson default value override `{}` configured to `{}` ({})", override.propertyName, effectiveValue, reason); } }); for (String unsupportedProperty : getUnsupportedProperties()) { @@ -124,13 +144,14 @@ private void validate(final StreamReadConstraints streamReadConstraints) { void eachOverride(BiConsumer overrideIntegerBiConsumer) { for (Override override : Override.values()) { final String propValue = this.propertyOverrides.get(override.propertyName); - if (propValue != null) { - try { - int intValue = Integer.parseInt(propValue); - overrideIntegerBiConsumer.accept(override, intValue); - } catch (IllegalArgumentException e) { - throw new IllegalArgumentException(String.format("System property `%s` must be positive integer value. Received: `%s`", override.propertyName, propValue), e); - } + + try { + final Integer explicitValue = Optional.ofNullable(propValue) + .map(Integer::parseInt) + .orElse(null); + overrideIntegerBiConsumer.accept(override, explicitValue); + } catch (IllegalArgumentException e) { + throw new IllegalArgumentException(String.format("System property `%s` must be positive integer value. Received: `%s`", override.propertyName, propValue), e); } } } diff --git a/logstash-core/src/test/java/org/logstash/ObjectMappersTest.java b/logstash-core/src/test/java/org/logstash/ObjectMappersTest.java index f89b394cdfb..4d26f023ceb 100644 --- a/logstash-core/src/test/java/org/logstash/ObjectMappersTest.java +++ b/logstash-core/src/test/java/org/logstash/ObjectMappersTest.java @@ -49,4 +49,9 @@ public void testLog4jOMRegisterRubyBasicObjectSerializersFirst() { assertNotNull(found); assertTrue("RubyBasicObjectSerializer must be registered before others non-default serializers", found instanceof RubyBasicObjectSerializer); } + + @Test + public void testStreamReadConstraintsApplication() { + ObjectMappers.CONFIGURED_STREAM_READ_CONSTRAINTS.validateIsGlobalDefault(); + } } \ No newline at end of file diff --git a/logstash-core/src/test/java/org/logstash/jackson/StreamReadConstraintsUtilTest.java b/logstash-core/src/test/java/org/logstash/jackson/StreamReadConstraintsUtilTest.java index e984a58d943..c08d6543290 100644 --- a/logstash-core/src/test/java/org/logstash/jackson/StreamReadConstraintsUtilTest.java +++ b/logstash-core/src/test/java/org/logstash/jackson/StreamReadConstraintsUtilTest.java @@ -7,13 +7,17 @@ import org.apache.logging.log4j.test.appender.ListAppender; import org.junit.Before; import org.junit.ClassRule; +import org.junit.Rule; import org.junit.Test; import java.util.Arrays; import java.util.List; +import java.util.Objects; import java.util.Properties; import java.util.function.BiConsumer; + import org.apache.logging.log4j.Level; +import org.logstash.ObjectMappers; import static org.assertj.core.api.Assertions.*; import static org.logstash.jackson.StreamReadConstraintsUtil.Override.*; @@ -23,13 +27,12 @@ public class StreamReadConstraintsUtilTest { @ClassRule public static final LoggerContextRule LOGGER_CONTEXT_RULE = new LoggerContextRule("log4j2-log-stream-read-constraints.xml"); + @Rule + public final DefaultsRestorer defaultsRestorer = new DefaultsRestorer(); + private ListAppender listAppender; private Logger observedLogger; - private static final int DEFAULT_MAX_STRING_LENGTH = 200_000_000; - private static final int DEFAULT_MAX_NUMBER_LENGTH = 10_000; - private static final int DEFAULT_MAX_NESTING_DEPTH = 1_000; - @Before public void setUpLoggingListAppender() { int i = 1+16; @@ -39,6 +42,7 @@ public void setUpLoggingListAppender() { @Test public void configuresDefaultsByDefault() { + Objects.requireNonNull(ObjectMappers.CBOR_MAPPER); // force static init StreamReadConstraintsUtil.fromSystemProperties().validateIsGlobalDefault(); } @@ -55,8 +59,8 @@ public void configuresMaxStringLength() { assertThat(configuredConstraints).as("inherited defaults") .returns(defaults.getMaxDocumentLength(), from(StreamReadConstraints::getMaxDocumentLength)) .returns(defaults.getMaxNameLength(), from(StreamReadConstraints::getMaxNameLength)) - .returns(DEFAULT_MAX_NESTING_DEPTH, from(StreamReadConstraints::getMaxNestingDepth)) - .returns(DEFAULT_MAX_NUMBER_LENGTH, from(StreamReadConstraints::getMaxNumberLength)); + .returns(MAX_NESTING_DEPTH.defaultValue, from(StreamReadConstraints::getMaxNestingDepth)) + .returns(MAX_NUMBER_LENGTH.defaultValue, from(StreamReadConstraints::getMaxNumberLength)); assertThatThrownBy(configuredUtil::validateIsGlobalDefault).isInstanceOf(IllegalStateException.class).hasMessageContaining(MAX_STRING_LENGTH.propertyName); @@ -98,8 +102,8 @@ public void configuresMaxNumberLength() { assertThat(configuredConstraints).as("inherited defaults") .returns(defaults.getMaxDocumentLength(), from(StreamReadConstraints::getMaxDocumentLength)) .returns(defaults.getMaxNameLength(), from(StreamReadConstraints::getMaxNameLength)) - .returns(DEFAULT_MAX_NESTING_DEPTH, from(StreamReadConstraints::getMaxNestingDepth)) - .returns(DEFAULT_MAX_STRING_LENGTH, from(StreamReadConstraints::getMaxStringLength)); + .returns(MAX_NESTING_DEPTH.defaultValue, from(StreamReadConstraints::getMaxNestingDepth)) + .returns(MAX_STRING_LENGTH.defaultValue, from(StreamReadConstraints::getMaxStringLength)); assertThatThrownBy(configuredUtil::validateIsGlobalDefault).isInstanceOf(IllegalStateException.class).hasMessageContaining(MAX_NUMBER_LENGTH.propertyName); @@ -141,8 +145,8 @@ public void configuresMaxNestingDepth() { assertThat(configuredConstraints).as("inherited defaults") .returns(defaults.getMaxDocumentLength(), from(StreamReadConstraints::getMaxDocumentLength)) .returns(defaults.getMaxNameLength(), from(StreamReadConstraints::getMaxNameLength)) - .returns(DEFAULT_MAX_STRING_LENGTH, from(StreamReadConstraints::getMaxStringLength)) - .returns(DEFAULT_MAX_NUMBER_LENGTH, from(StreamReadConstraints::getMaxNumberLength)); + .returns(MAX_STRING_LENGTH.defaultValue, from(StreamReadConstraints::getMaxStringLength)) + .returns(MAX_NUMBER_LENGTH.defaultValue, from(StreamReadConstraints::getMaxNumberLength)); assertThatThrownBy(configuredUtil::validateIsGlobalDefault).isInstanceOf(IllegalStateException.class).hasMessageContaining(MAX_NESTING_DEPTH.propertyName); @@ -187,8 +191,6 @@ public void validatesApplication() { configuredUtil.validateIsGlobalDefault(); }); - System.out.format("OK%n"); - assertLogObserved(Level.INFO, "override `" + MAX_NESTING_DEPTH.propertyName + "` configured to `1010`"); assertLogObserved(Level.INFO, "override `" + MAX_STRING_LENGTH.propertyName + "` configured to `1011`"); assertLogObserved(Level.INFO, "override `" + MAX_NUMBER_LENGTH.propertyName + "` configured to `1110`"); @@ -198,28 +200,17 @@ public void validatesApplication() { } @Test - public void usesJacksonDefaultsWhenNoConfig() { - StreamReadConstraintsUtil util = new StreamReadConstraintsUtil(new Properties(), this.observedLogger); - StreamReadConstraints constraints = util.get(); - - assertThat(constraints) - .returns(DEFAULT_MAX_STRING_LENGTH, from(StreamReadConstraints::getMaxStringLength)) - .returns(DEFAULT_MAX_NUMBER_LENGTH, from(StreamReadConstraints::getMaxNumberLength)) - .returns(DEFAULT_MAX_NESTING_DEPTH, from(StreamReadConstraints::getMaxNestingDepth)); - } - - @Test - public void configOverridesDefault() { - Properties props = new Properties(); - props.setProperty("logstash.jackson.stream-read-constraints.max-string-length", "100"); + public void validatesApplicationWithDefaults() { + final Properties properties = new Properties(); // empty -- no overrides - StreamReadConstraintsUtil util = new StreamReadConstraintsUtil(props, this.observedLogger); - StreamReadConstraints constraints = util.get(); + fromProperties(properties, (configuredUtil, defaults) -> { + configuredUtil.applyAsGlobalDefault(); + configuredUtil.validateIsGlobalDefault(); - assertThat(constraints) - .returns(100, from(StreamReadConstraints::getMaxStringLength)) - .returns(DEFAULT_MAX_NUMBER_LENGTH, from(StreamReadConstraints::getMaxNumberLength)) - .returns(DEFAULT_MAX_NESTING_DEPTH, from(StreamReadConstraints::getMaxNestingDepth)); + assertLogObserved(Level.INFO, "override `" + MAX_NESTING_DEPTH.propertyName + "` configured to `" + MAX_NESTING_DEPTH.defaultValue + "` (logstash default)"); + assertLogObserved(Level.INFO, "override `" + MAX_STRING_LENGTH.propertyName + "` configured to `" + MAX_STRING_LENGTH.defaultValue + "` (logstash default)"); + assertLogObserved(Level.INFO, "override `" + MAX_NUMBER_LENGTH.propertyName + "` configured to `" + MAX_NUMBER_LENGTH.defaultValue + "` (logstash default)"); + }); } private void assertLogObserved(final Level level, final String... messageFragments) { @@ -230,13 +221,33 @@ private void assertLogObserved(final Level level, final String... messageFragmen .anySatisfy(logEvent -> assertThat(logEvent.getMessage().getFormattedMessage()).contains(messageFragments)); } - private synchronized void fromProperties(final Properties properties, BiConsumer defaultsConsumer) { + private synchronized void fromProperties(final Properties properties, + final BiConsumer defaultsConsumer) { final StreamReadConstraints defaults = StreamReadConstraints.defaults(); - try { - final StreamReadConstraintsUtil util = new StreamReadConstraintsUtil(properties, this.observedLogger); - defaultsConsumer.accept(util, defaults); - } finally { - StreamReadConstraints.overrideDefaultStreamReadConstraints(defaults); + final StreamReadConstraintsUtil util = new StreamReadConstraintsUtil(properties, this.observedLogger); + + defaultsConsumer.accept(util, defaults); + } + + /** + * A TestRule to snapshot the current defaults from jackson prior to test execution, and to + * ensure that snapshot is restored after test execution has completed. + */ + public static class DefaultsRestorer extends org.junit.rules.ExternalResource { + private StreamReadConstraints snapshot; + + public StreamReadConstraints getSnapshot() { + return this.snapshot; + } + + @Override + protected void before() throws Throwable { + this.snapshot = StreamReadConstraints.defaults(); + } + + @Override + protected void after() { + StreamReadConstraints.overrideDefaultStreamReadConstraints(this.snapshot); } } } \ No newline at end of file