Skip to content

Commit

Permalink
ensure jackson overrides are available to static initializers (#16719) (
Browse files Browse the repository at this point in the history
#16757)

Moves the application of jackson defaults overrides into pure java, and
applies them statically _before_ the `org.logstash.ObjectMappers` has a chance
to start initializing object mappers that rely on the defaults.

We replace the runner's invocation (which was too late to be fully applied) with
a _verification_ that the configured defaults have been applied.

(cherry picked from commit 202d07c)

Co-authored-by: Ry Biesemeyer <[email protected]>
  • Loading branch information
github-actions[bot] and yaauie authored Dec 5, 2024
1 parent 5ab462e commit 6f8fd5a
Show file tree
Hide file tree
Showing 10 changed files with 388 additions and 185 deletions.
4 changes: 2 additions & 2 deletions logstash-core/lib/logstash/runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -353,8 +353,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 @@ -571,8 +571,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);
} 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

0 comments on commit 6f8fd5a

Please sign in to comment.