Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ensure jackson overrides are available to static initializers #16719

Merged
merged 1 commit into from
Dec 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions logstash-core/lib/logstash/runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -321,8 +321,8 @@ def execute
# Add local modules to the registry before everything else
LogStash::Modules::Util.register_local_modules(LogStash::Environment::LOGSTASH_HOME)

# Set up the Jackson defaults
LogStash::Util::Jackson.set_jackson_defaults(logger)
# Verify the Jackson defaults
LogStash::Util::Jackson.verify_jackson_overrides

@dispatcher = LogStash::EventDispatcher.new(self)
LogStash::PLUGIN_REGISTRY.hooks.register_emitter(self.class, @dispatcher)
Expand Down
71 changes: 4 additions & 67 deletions logstash-core/lib/logstash/util/jackson.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,76 +18,13 @@
module LogStash
module Util
module Jackson
def self.set_jackson_defaults(logger)
JacksonStreamReadConstraintsDefaults.new(logger).configure
end

class JacksonStreamReadConstraintsDefaults

java_import com.fasterxml.jackson.core.StreamReadConstraints

PROPERTY_MAX_STRING_LENGTH = 'logstash.jackson.stream-read-constraints.max-string-length'.freeze
PROPERTY_MAX_NUMBER_LENGTH = 'logstash.jackson.stream-read-constraints.max-number-length'.freeze
PROPERTY_MAX_NESTING_DEPTH = 'logstash.jackson.stream-read-constraints.max-nesting-depth'.freeze

def initialize(logger)
@logger = logger
end

public

def configure
max_string_len = get_default_value_override!(PROPERTY_MAX_STRING_LENGTH)
max_num_len = get_default_value_override!(PROPERTY_MAX_NUMBER_LENGTH)
max_nesting_depth = get_default_value_override!(PROPERTY_MAX_NESTING_DEPTH)

if max_string_len || max_num_len || max_nesting_depth
begin
override_default_stream_read_constraints(max_string_len, max_num_len, max_nesting_depth)
rescue java.lang.IllegalArgumentException => e
raise LogStash::ConfigurationError, "Invalid `logstash.jackson.*` system properties configuration: #{e.message}"
end
end
end

private

def get_default_value_override!(property)
value = get_property_value(property)
return if value.nil?
def self.verify_jackson_overrides
java_import org.logstash.ObjectMappers

begin
int_value = java.lang.Integer.parseInt(value)

if int_value < 1
raise LogStash::ConfigurationError, "System property '#{property}' must be bigger than zero. Received: #{int_value}"
end

@logger.info("Jackson default value override `#{property}` configured to `#{int_value}`")

int_value
rescue java.lang.NumberFormatException => _e
raise LogStash::ConfigurationError, "System property '#{property}' must be a positive integer value. Received: #{value}"
end
end

def get_property_value(name)
java.lang.System.getProperty(name)
end

def override_default_stream_read_constraints(max_string_len, max_num_len, max_nesting_depth)
builder = new_stream_read_constraints_builder
builder.maxStringLength(max_string_len) if max_string_len
builder.maxNumberLength(max_num_len) if max_num_len
builder.maxNestingDepth(max_nesting_depth) if max_nesting_depth

StreamReadConstraints.overrideDefaultStreamReadConstraints(builder.build)
end

def new_stream_read_constraints_builder
StreamReadConstraints::builder
end
ObjectMappers::getConfiguredStreamReadConstraints().validateIsGlobalDefault()
end

end
end
end
4 changes: 2 additions & 2 deletions logstash-core/spec/logstash/runner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -462,8 +462,8 @@
subject { LogStash::Runner.new("") }
let(:args) { ["-e", "input {} output {}"] }

it 'should be set' do
expect(LogStash::Util::Jackson).to receive(:set_jackson_defaults)
it 'should be verified' do
expect(LogStash::Util::Jackson).to receive(:verify_jackson_overrides)
subject.run(args)
end
end
Expand Down
113 changes: 0 additions & 113 deletions logstash-core/spec/logstash/util/jackson_spec.rb

This file was deleted.

16 changes: 16 additions & 0 deletions logstash-core/src/main/java/org/logstash/ObjectMappers.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import java.math.BigDecimal;
import java.math.BigInteger;
import java.util.HashMap;

import org.apache.logging.log4j.core.jackson.Log4jJsonObjectMapper;
import org.jruby.RubyBignum;
import org.jruby.RubyBoolean;
Expand All @@ -52,12 +53,27 @@
import org.jruby.RubySymbol;
import org.jruby.ext.bigdecimal.RubyBigDecimal;
import org.logstash.ext.JrubyTimestampExtLibrary;
import org.logstash.jackson.StreamReadConstraintsUtil;
import org.logstash.log.RubyBasicObjectSerializer;

public final class ObjectMappers {

static final String RUBY_SERIALIZERS_MODULE_ID = "RubySerializers";

static final StreamReadConstraintsUtil CONFIGURED_STREAM_READ_CONSTRAINTS;

static {
// The StreamReadConstraintsUtil needs to load the configured constraints from system
// properties and apply them _statically_, before any object mappers are initialized.
CONFIGURED_STREAM_READ_CONSTRAINTS = StreamReadConstraintsUtil.fromSystemProperties();
CONFIGURED_STREAM_READ_CONSTRAINTS.applyAsGlobalDefault();
}

public static StreamReadConstraintsUtil getConfiguredStreamReadConstraints() {
return CONFIGURED_STREAM_READ_CONSTRAINTS;
}


private static final SimpleModule RUBY_SERIALIZERS =
new SimpleModule(RUBY_SERIALIZERS_MODULE_ID)
.addSerializer(RubyString.class, new RubyStringSerializer())
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
package org.logstash.jackson;

import com.fasterxml.jackson.core.StreamReadConstraints;
import com.google.common.collect.Sets;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

import java.util.*;
import java.util.function.BiConsumer;
import java.util.function.BiFunction;
import java.util.function.Function;
import java.util.stream.Collectors;

public class StreamReadConstraintsUtil {

private final Map<String,String> propertyOverrides;
private final Logger logger;

private StreamReadConstraints configuredStreamReadConstraints;

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),
;

static final String PROP_PREFIX = "logstash.jackson.stream-read-constraints.";

final String propertyName;
private final IntValueApplicator applicator;
private final IntValueObserver observer;

Override(final IntValueApplicator applicator,
final IntValueObserver observer) {
this.propertyName = PROP_PREFIX + this.name().toLowerCase().replace('_', '-');
this.applicator = applicator;
this.observer = observer;
}

@FunctionalInterface
interface IntValueObserver extends Function<StreamReadConstraints, Integer> {}

@FunctionalInterface
interface IntValueApplicator extends BiFunction<StreamReadConstraints.Builder, Integer, StreamReadConstraints.Builder> {}
}

/**
* @return an instance configured by {@code System.getProperties()}
*/
public static StreamReadConstraintsUtil fromSystemProperties() {
return new StreamReadConstraintsUtil(System.getProperties());
}

StreamReadConstraintsUtil(final Properties properties) {
this(properties, null);
}

StreamReadConstraintsUtil(final Properties properties,
final Logger logger) {
this(extractProperties(properties), logger);
}

static private Map<String,String> extractProperties(final Properties properties) {
return properties.stringPropertyNames().stream()
.filter(propName -> propName.startsWith(Override.PROP_PREFIX))
.map(propName -> Map.entry(propName, properties.getProperty(propName)))
.filter(entry -> entry.getValue() != null)
.collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, Map.Entry::getValue));
}

private StreamReadConstraintsUtil(final Map<String,String> propertyOverrides,
final Logger logger) {
this.propertyOverrides = Map.copyOf(propertyOverrides);
this.logger = Objects.requireNonNullElseGet(logger, () -> LogManager.getLogger(StreamReadConstraintsUtil.class));
}

StreamReadConstraints get() {
if (configuredStreamReadConstraints == null) {
final StreamReadConstraints.Builder builder = StreamReadConstraints.defaults().rebuild();

eachOverride((override, value) -> override.applicator.apply(builder, value));

this.configuredStreamReadConstraints = builder.build();
}
return configuredStreamReadConstraints;
}

public void applyAsGlobalDefault() {
StreamReadConstraints.overrideDefaultStreamReadConstraints(get());
}

public void validateIsGlobalDefault() {
validate(StreamReadConstraints.defaults());
}

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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, this will log at info for the default Logstash Jackson config set in config/jvm.options:

[2024-12-04T12:53:30,180][INFO ][org.logstash.jackson.StreamReadConstraintsUtil] Jackson default value override `logstash.jackson.stream-read-constraints.max-string-length` configured to `200000000`
[2024-12-04T12:53:30,180][INFO ][org.logstash.jackson.StreamReadConstraintsUtil] Jackson default value override `logstash.jackson.stream-read-constraints.max-number-length` configured to `10000`

What do you think about logging at debug, or verifying against the Logstash defaults?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It replaces identical info-level logging from before:

https://github.com/elastic/logstash/pull/16719/files#diff-8779c222532118bcfe1d6ccd7f5152238150615ec0b0d394d44e4fbcf334f903L66

I hesitate to add functionality to compare it against logstash defaults, when those defaults are currently held in and provided by a user-overridable config file.

} else {
fatalIssues.add(String.format("`%s` (expected: `%s`, actual: `%s`)", override.propertyName, specifiedValue, effectiveValue));
}
});
for (String unsupportedProperty : getUnsupportedProperties()) {
logger.warn("Jackson default value override `{}` is unknown and has been ignored", unsupportedProperty);
}
if (!fatalIssues.isEmpty()) {
throw new IllegalStateException(String.format("Jackson default values not applied: %s", String.join(",", fatalIssues)));
}
}

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);
}
}
}
}

Set<String> getUnsupportedProperties() {
Set<String> supportedProps = Arrays.stream(Override.values()).map(p -> p.propertyName).collect(Collectors.toSet());
Set<String> providedProps = this.propertyOverrides.keySet();

return Sets.difference(providedProps, supportedProps);
}
}
Loading
Loading