mirror of
https://github.com/elastic/logstash.git
synced 2025-04-17 19:35:03 -04:00
jackson stream read constraints: code-based defaults (#16854)
* Revert "Apply Jackson stream read constraints defaults at runtime (#16832)"
This reverts commit cc608eb88b
.
* jackson stream read constraints: code-based defaults
refactors stream read constraints to couple default values with their
associated overrides, which allows us to have more descriptive logging
that includes provenance of the value that has been applied.
This commit is contained in:
parent
dae7fd93db
commit
16392908e2
4 changed files with 109 additions and 72 deletions
|
@ -71,11 +71,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.
|
||||
|
|
|
@ -5,10 +5,16 @@ import com.google.common.collect.Sets;
|
|||
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<Override, Integer> 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;
|
||||
}
|
||||
|
||||
@FunctionalInterface
|
||||
interface IntValueObserver extends Function<StreamReadConstraints, Integer> {}
|
||||
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 IntValueApplicator extends BiFunction<StreamReadConstraints.Builder, Integer, StreamReadConstraints.Builder> {}
|
||||
interface IntValueObserver {
|
||||
int observeIntValue(final StreamReadConstraints constraints);
|
||||
}
|
||||
|
||||
@FunctionalInterface
|
||||
interface IntValueApplicator {
|
||||
@SuppressWarnings("UnusedReturnValue")
|
||||
StreamReadConstraints.Builder applyIntValue(final StreamReadConstraints.Builder constraintsBuilder, final int value);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -86,9 +105,7 @@ public class StreamReadConstraintsUtil {
|
|||
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 class StreamReadConstraintsUtil {
|
|||
private void validate(final StreamReadConstraints streamReadConstraints) {
|
||||
final List<String> 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 @@ public class StreamReadConstraintsUtil {
|
|||
void eachOverride(BiConsumer<Override,Integer> 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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -49,4 +49,9 @@ public class ObjectMappersTest {
|
|||
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();
|
||||
}
|
||||
}
|
|
@ -7,13 +7,17 @@ import org.apache.logging.log4j.junit.LoggerContextRule;
|
|||
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 class StreamReadConstraintsUtilTest {
|
|||
|
||||
@Test
|
||||
public void configuresDefaultsByDefault() {
|
||||
Objects.requireNonNull(ObjectMappers.CBOR_MAPPER); // force static init
|
||||
StreamReadConstraintsUtil.fromSystemProperties().validateIsGlobalDefault();
|
||||
}
|
||||
|
||||
|
@ -55,8 +59,8 @@ public class StreamReadConstraintsUtilTest {
|
|||
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 class StreamReadConstraintsUtilTest {
|
|||
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 class StreamReadConstraintsUtilTest {
|
|||
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 class StreamReadConstraintsUtilTest {
|
|||
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 class StreamReadConstraintsUtilTest {
|
|||
}
|
||||
|
||||
@Test
|
||||
public void usesJacksonDefaultsWhenNoConfig() {
|
||||
StreamReadConstraintsUtil util = new StreamReadConstraintsUtil(new Properties(), this.observedLogger);
|
||||
StreamReadConstraints constraints = util.get();
|
||||
public void validatesApplicationWithDefaults() {
|
||||
final Properties properties = new Properties(); // empty -- no overrides
|
||||
|
||||
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));
|
||||
}
|
||||
fromProperties(properties, (configuredUtil, defaults) -> {
|
||||
configuredUtil.applyAsGlobalDefault();
|
||||
configuredUtil.validateIsGlobalDefault();
|
||||
|
||||
@Test
|
||||
public void configOverridesDefault() {
|
||||
Properties props = new Properties();
|
||||
props.setProperty("logstash.jackson.stream-read-constraints.max-string-length", "100");
|
||||
|
||||
StreamReadConstraintsUtil util = new StreamReadConstraintsUtil(props, this.observedLogger);
|
||||
StreamReadConstraints constraints = util.get();
|
||||
|
||||
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 @@ public class StreamReadConstraintsUtilTest {
|
|||
.anySatisfy(logEvent -> assertThat(logEvent.getMessage().getFormattedMessage()).contains(messageFragments));
|
||||
}
|
||||
|
||||
private synchronized void fromProperties(final Properties properties, BiConsumer<StreamReadConstraintsUtil, StreamReadConstraints> defaultsConsumer) {
|
||||
private synchronized void fromProperties(final Properties properties,
|
||||
final BiConsumer<StreamReadConstraintsUtil, StreamReadConstraints> 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);
|
||||
}
|
||||
}
|
||||
}
|
Loading…
Add table
Reference in a new issue