Skip to content

Commit

Permalink
Introduce nesting depth limits for encoders and decoders
Browse files Browse the repository at this point in the history
Create a new base class LimitingStream for all Decoder and Encoder implementations that checks the nesting depth. The nesting depth is configured using a new configuration property.

The purpose of this is to harden against issues like FasterXML/jackson-databind#3972 . Another benefit is that accidental recursive data structures made by developers won't lead to a stack overflow but will be caught earlier with a nicer error message.

This patch changes many classes, mostly because the new limit has to be propagated from the serde configuration to the encoders/decoders. To simplify addition of other limits in the future, I've wrapped the depth limit into an opaque data structure. Other possible limits would be e.g. on total output size or on array sizes.

I've tried to keep this patch as compatible as possible even though it's only going into a major version, to make the release process easier. Methods that need the new limit parameter and were not internal I've deprecated.
  • Loading branch information
yawkat committed Jun 19, 2023
1 parent 8df9192 commit 843a50a
Show file tree
Hide file tree
Showing 30 changed files with 552 additions and 167 deletions.
128 changes: 128 additions & 0 deletions serde-api/src/main/java/io/micronaut/serde/LimitingStream.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
/*
* Copyright 2017-2023 original authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.micronaut.serde;

import io.micronaut.core.annotation.Internal;
import io.micronaut.core.annotation.NonNull;
import io.micronaut.core.type.Argument;
import io.micronaut.serde.config.SerdeConfiguration;
import io.micronaut.serde.exceptions.SerdeException;

/**
* Utility class to check data limits in encoders and decoders. Currently, the only limit that is
* checked is the nesting depth.
* <p>
* This class can be used in both implementation "patterns" for {@link Encoder} (and
* {@link Decoder}): For the same-object pattern, where {@link Encoder#encodeObject(Argument)}
* returns the same instance, the encoder should call {@link #increaseDepth()}, with a
* corresponding {@link #decreaseDepth()} call in {@link Encoder#finishStructure()}. Conversely, if
* the encoder is implemented using the child-object pattern, where
* {@link Encoder#encodeObject(Argument)} returns a child encoder responsible for the subtree, that
* child encoder should be instantiated using the new limits returned by {@link #childLimits()}.
* <p>
* If there is a limit violation, {@link #childLimits()} or {@link #increaseDepth()} will throw an
* exception.
*
* @author Jonas Konrad
* @since 2.0.0
*/
@Internal
public abstract class LimitingStream {
/**
* Default nesting depth limit.
*/
public static final int DEFAULT_MAXIMUM_DEPTH = 1024;
/**
* Default limits, only use this when no {@link SerdeConfiguration} is available.
*/
public static final RemainingLimits DEFAULT_LIMITS = new RemainingLimits(DEFAULT_MAXIMUM_DEPTH);

private int remainingDepth;

public LimitingStream(@NonNull RemainingLimits remainingLimits) {
this.remainingDepth = remainingLimits.remainingDepth;
}

/**
* Get a snapshot of our current limits to be passed to the constructor. This can be used for
* buffering decoders, when a new decoder takes over but no change in depth takes place.
*
* @return The current limits
*/
@NonNull
protected final RemainingLimits ourLimits() {
return new RemainingLimits(remainingDepth);
}

/**
* Get the limits of a new child encoder.
*
* @return The new limits
* @throws SerdeException If there is a nesting depth limit violation
*/
@NonNull
protected final RemainingLimits childLimits() throws SerdeException {
if (remainingDepth == 0) {
reportMaxDepthExceeded();
}
return new RemainingLimits(remainingDepth - 1);
}

/**
* Increase the current depth.
*
* @throws SerdeException If there is a nesting depth limit violation
*/
protected final void increaseDepth() throws SerdeException {
if (remainingDepth == 0) {
reportMaxDepthExceeded();
}
remainingDepth--;
}

/**
* Decrease the current depth, always needs a corresponding {@link #increaseDepth()} call.
*/
protected final void decreaseDepth() {
remainingDepth++;
}

private void reportMaxDepthExceeded() throws SerdeException {
boolean encoder = this instanceof Encoder;
throw new SerdeException("Maximum depth exceeded while " + (encoder ? "serializing" : "deserializing") + ". The maximum nesting depth can be increased, if necessary, using the " + SerdeConfiguration.PREFIX + ".maximum-nesting-depth config property.");
}

/**
* Get the configured limits.
*
* @param configuration The serde configuration
* @return The configured limits
*/
public static RemainingLimits limitsFromConfiguration(SerdeConfiguration configuration) {
return new RemainingLimits(configuration.getMaximumNestingDepth());
}

/**
* This data structure contains the limits for this stream.
*/
public static class RemainingLimits {
final int remainingDepth;

private RemainingLimits(int remainingDepth) {
this.remainingDepth = remainingDepth;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import io.micronaut.context.annotation.BootstrapContextCompatible;
import io.micronaut.context.annotation.ConfigurationProperties;
import io.micronaut.core.bind.annotation.Bindable;
import io.micronaut.serde.LimitingStream;

import java.util.List;
import java.util.Locale;
Expand Down Expand Up @@ -77,6 +78,15 @@ public interface SerdeConfiguration {
@Bindable(defaultValue = "io.micronaut")
List<String> getIncludedIntrospectionPackages();

/**
* The maximum nesting depth for serialization and deserialization.
*
* @return The maximum nesting depth for serialization and deserialization
* @since 2.0.0
*/
@Bindable(defaultValue = LimitingStream.DEFAULT_MAXIMUM_DEPTH + "")
int getMaximumNestingDepth();

/**
* Shape to use for time serialization.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,17 @@

import io.micronaut.core.annotation.Internal;
import io.micronaut.core.annotation.NonNull;
import io.micronaut.core.annotation.Nullable;
import io.micronaut.core.type.Argument;
import io.micronaut.json.JsonStreamConfig;
import io.micronaut.json.tree.JsonNode;
import io.micronaut.serde.*;
import io.micronaut.serde.Deserializer;
import io.micronaut.serde.Encoder;
import io.micronaut.serde.LimitingStream;
import io.micronaut.serde.ObjectMapper;
import io.micronaut.serde.SerdeRegistry;
import io.micronaut.serde.Serializer;
import io.micronaut.serde.config.SerdeConfiguration;
import io.micronaut.serde.support.util.BufferingJsonNodeProcessor;
import io.micronaut.serde.support.util.JsonNodeDecoder;
import io.micronaut.serde.support.util.JsonNodeEncoder;
Expand All @@ -43,16 +50,19 @@
@Internal
public abstract class AbstractBsonMapper implements ObjectMapper {
protected final SerdeRegistry registry;
@Nullable
protected final SerdeConfiguration serdeConfiguration;
protected final Class<?> view;
protected Serializer.EncoderContext encoderContext;
protected Deserializer.DecoderContext decoderContext;

public AbstractBsonMapper(SerdeRegistry registry) {
this(registry, null);
public AbstractBsonMapper(SerdeRegistry registry, SerdeConfiguration serdeConfiguration) {
this(registry, serdeConfiguration, null);
}

protected AbstractBsonMapper(SerdeRegistry registry, Class<?> view) {
protected AbstractBsonMapper(SerdeRegistry registry, SerdeConfiguration serdeConfiguration, Class<?> view) {
this.registry = registry;
this.serdeConfiguration = serdeConfiguration;
this.view = view;
this.encoderContext = registry.newEncoderContext(view);
this.decoderContext = registry.newDecoderContext(view);
Expand All @@ -62,9 +72,14 @@ protected AbstractBsonMapper(SerdeRegistry registry, Class<?> view) {

protected abstract AbstractBsonWriter createBsonWriter(OutputStream bsonOutput) throws IOException;

@NonNull
private LimitingStream.RemainingLimits limits() {
return serdeConfiguration == null ? LimitingStream.DEFAULT_LIMITS : LimitingStream.limitsFromConfiguration(serdeConfiguration);
}

@Override
public <T> JsonNode writeValueToTree(Argument<T> type, T value) throws IOException {
JsonNodeEncoder encoder = JsonNodeEncoder.create();
JsonNodeEncoder encoder = JsonNodeEncoder.create(limits());
serialize(encoder, value);
return encoder.getCompletedValue();
}
Expand All @@ -75,7 +90,7 @@ public <T> void writeValue(OutputStream outputStream, Argument<T> type, T object
if (object == null) {
bsonWriter.writeNull();
} else {
BsonWriterEncoder encoder = new BsonWriterEncoder(bsonWriter);
BsonWriterEncoder encoder = new BsonWriterEncoder(bsonWriter, limits());
serialize(encoder, object, type);
}
bsonWriter.flush();
Expand All @@ -92,7 +107,7 @@ public <T> byte[] writeValueAsBytes(Argument<T> type, T object) throws IOExcepti
@Override
public <T> T readValueFromTree(JsonNode tree, Argument<T> type) throws IOException {
final Deserializer<? extends T> deserializer = this.decoderContext.findDeserializer(type).createSpecific(decoderContext, type);
return deserializer.deserialize(JsonNodeDecoder.create(tree), decoderContext, type);
return deserializer.deserialize(JsonNodeDecoder.create(tree, limits()), decoderContext, type);
}

@Override
Expand All @@ -114,7 +129,7 @@ private <T> T readValue(ByteBuffer byteBuffer, Argument<T> type) throws IOExcept
private <T> T readValue(BsonReader bsonReader, Argument<T> type) throws IOException {
return decoderContext.findDeserializer(type)
.createSpecific(decoderContext, type)
.deserialize(new BsonReaderDecoder(bsonReader), decoderContext, type);
.deserialize(new BsonReaderDecoder(bsonReader, limits()), decoderContext, type);
}

@Override
Expand All @@ -125,7 +140,7 @@ public Processor<byte[], JsonNode> createReactiveParser(Consumer<Processor<byte[
@Override
protected JsonNode parseOne(@NonNull InputStream is) throws IOException {
try (BsonReader bsonReader = createBsonReader(toByteBuffer(is))) {
final BsonReaderDecoder decoder = new BsonReaderDecoder(bsonReader);
final BsonReaderDecoder decoder = new BsonReaderDecoder(bsonReader, limits());
final Object o = decoder.decodeArbitrary();
return writeValueToTree(o);
}
Expand All @@ -134,7 +149,7 @@ protected JsonNode parseOne(@NonNull InputStream is) throws IOException {
@Override
protected JsonNode parseOne(byte[] remaining) throws IOException {
try (BsonReader bsonReader = createBsonReader(ByteBuffer.wrap(remaining))) {
final BsonReaderDecoder decoder = new BsonReaderDecoder(bsonReader);
final BsonReaderDecoder decoder = new BsonReaderDecoder(bsonReader, limits());
final Object o = decoder.decodeArbitrary();
return writeValueToTree(o);
}
Expand All @@ -144,7 +159,7 @@ protected JsonNode parseOne(byte[] remaining) throws IOException {

@Override
public JsonNode writeValueToTree(Object value) throws IOException {
JsonNodeEncoder encoder = JsonNodeEncoder.create();
JsonNodeEncoder encoder = JsonNodeEncoder.create(limits());
serialize(encoder, value);
return encoder.getCompletedValue();
}
Expand All @@ -155,7 +170,7 @@ public void writeValue(OutputStream outputStream, Object object) throws IOExcept
if (object == null) {
bsonWriter.writeNull();
} else {
BsonWriterEncoder encoder = new BsonWriterEncoder(bsonWriter);
BsonWriterEncoder encoder = new BsonWriterEncoder(bsonWriter, limits());
serialize(encoder, object);
}
bsonWriter.flush();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import io.micronaut.core.annotation.Order;
import io.micronaut.json.JsonMapper;
import io.micronaut.serde.SerdeRegistry;
import io.micronaut.serde.config.SerdeConfiguration;
import jakarta.inject.Inject;
import jakarta.inject.Singleton;
import org.bson.AbstractBsonWriter;
Expand All @@ -41,17 +42,17 @@
public final class BsonBinaryMapper extends AbstractBsonMapper {

@Inject
public BsonBinaryMapper(SerdeRegistry registry) {
super(registry);
public BsonBinaryMapper(SerdeRegistry registry, SerdeConfiguration serdeConfiguration) {
super(registry, serdeConfiguration);
}

public BsonBinaryMapper(SerdeRegistry registry, Class<?> view) {
super(registry, view);
public BsonBinaryMapper(SerdeRegistry registry, SerdeConfiguration serdeConfiguration, Class<?> view) {
super(registry, serdeConfiguration, view);
}

@Override
public JsonMapper cloneWithViewClass(Class<?> viewClass) {
return new BsonBinaryMapper(registry, viewClass);
return new BsonBinaryMapper(registry, serdeConfiguration, viewClass);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import io.micronaut.core.annotation.Order;
import io.micronaut.json.JsonMapper;
import io.micronaut.serde.SerdeRegistry;
import io.micronaut.serde.config.SerdeConfiguration;
import jakarta.inject.Inject;
import jakarta.inject.Singleton;
import org.bson.AbstractBsonWriter;
Expand All @@ -43,18 +44,28 @@
@Order(100) // lower precedence than Jackson
public final class BsonJsonMapper extends AbstractBsonMapper {

@Inject
@Deprecated
public BsonJsonMapper(SerdeRegistry registry) {
super(registry);
super(registry, null);
}

@Deprecated
public BsonJsonMapper(SerdeRegistry registry, Class<?> view) {
super(registry, view);
super(registry, null, view);
}

@Inject
public BsonJsonMapper(SerdeRegistry registry, SerdeConfiguration serdeConfiguration) {
super(registry, serdeConfiguration);
}

private BsonJsonMapper(SerdeRegistry registry, SerdeConfiguration serdeConfiguration, Class<?> view) {
super(registry, serdeConfiguration, view);
}

@Override
public JsonMapper cloneWithViewClass(Class<?> viewClass) {
return new BsonJsonMapper(registry, viewClass);
return new BsonJsonMapper(registry, serdeConfiguration, viewClass);
}

@Override
Expand Down
Loading

0 comments on commit 843a50a

Please sign in to comment.