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.

Sign up for GitHub

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

Unwrap JsonMappingException if cause is LengthLimitingJsonProcessingException #1082

Merged
merged 2 commits into from
Jun 16, 2023
Merged

Unwrap JsonMappingException if cause is LengthLimitingJsonProcessingException #1082

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
54f197b
Unwrap JsonMappingException if cause is LengthLimitingJsonProcessingE…
jasmith-hs Jun 15, 2023
259bdc0
Don't try eager interpret when OutputTooBigException is thrown
jasmith-hs Jun 16, 2023
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
15 changes: 11 additions & 4 deletions src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,14 +132,21 @@ public JinjavaInterpreter(JinjavaInterpreter orig) {
}

public static void checkOutputSize(String string) {
if (isOutputTooLarge(string)) {
throw new OutputTooBigException(
getCurrent().getConfig().getMaxOutputSize(),
string.length()
);
}
}

public static boolean isOutputTooLarge(String string) {
Optional<Long> maxStringLength = getCurrentMaybe()
.map(interpreter -> interpreter.getConfig().getMaxOutputSize())
.filter(max -> max > 0);
if (
return (
maxStringLength.map(max -> string != null && string.length() > max).orElse(false)
) {
throw new OutputTooBigException(maxStringLength.get(), string.length());
}
);
}

/**
Expand Down
47 changes: 29 additions & 18 deletions src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerTagDecorator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,29 +38,40 @@ public T getTag() {
@Override
public final String interpret(TagNode tagNode, JinjavaInterpreter interpreter) {
try {
String output = innerInterpret(tagNode, interpreter);
JinjavaInterpreter.checkOutputSize(output);
return output;
} catch (DeferredValueException | TemplateSyntaxException | OutputTooBigException e) {
String output;
try {
return EagerReconstructionUtils.wrapInAutoEscapeIfNeeded(
eagerInterpret(
tagNode,
interpreter,
e instanceof InterpretException
? (InterpretException) e
: new InterpretException("Exception with default render", e)
),
interpreter
);
} catch (OutputTooBigException e1) {
throw new DeferredValueException(
String.format("Output too big for eager execution: %s", e1.getMessage())
);
output = innerInterpret(tagNode, interpreter);
} catch (DeferredValueException | TemplateSyntaxException e) {
return wrapEagerInterpret(tagNode, interpreter, e);
}
if (JinjavaInterpreter.isOutputTooLarge(output)) {
return wrapEagerInterpret(tagNode, interpreter, null);
}
Comment on lines +47 to +49
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We only want to do the deferred execution eager interpret if we fully succeeded, but the output is too large. If we encountered an uncaught OutputTooBigException while running innerInterpret, we haven't handled that, and the context won't be in a proper state. (It likely would come from attempting to reconstruct a value, but its string representation is too large.)

return output;
} catch (OutputTooBigException e) {
throw new DeferredValueException(
String.format("Output too big for eager execution: %s", e.getMessage())
);
}
}

private String wrapEagerInterpret(
TagNode tagNode,
JinjavaInterpreter interpreter,
RuntimeException e
) {
return EagerReconstructionUtils.wrapInAutoEscapeIfNeeded(
eagerInterpret(
tagNode,
interpreter,
e instanceof InterpretException
? (InterpretException) e
: new InterpretException("Exception with default render", e)
),
interpreter
);
}

protected String innerInterpret(TagNode tagNode, JinjavaInterpreter interpreter) {
return tag.interpret(tagNode, interpreter);
}
Expand Down
13 changes: 10 additions & 3 deletions src/main/java/com/hubspot/jinjava/objects/serialization/PyishObjectMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.fasterxml.jackson.core.JsonFactoryBuilder;
import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.databind.JsonMappingException;
import com.fasterxml.jackson.databind.JsonSerializer;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.ObjectWriter;
Expand Down Expand Up @@ -66,11 +67,17 @@ private static String getAsPyishString(Object val, boolean forOutput) {
try {
return getAsPyishStringOrThrow(val, forOutput);
} catch (IOException e) {
if (e instanceof LengthLimitingJsonProcessingException) {
Throwable unwrapped = e;
if (e instanceof JsonMappingException) {
unwrapped = unwrapped.getCause();
}
if (unwrapped instanceof LengthLimitingJsonProcessingException) {
throw new OutputTooBigException(
((LengthLimitingJsonProcessingException) e).getMaxSize(),
((LengthLimitingJsonProcessingException) e).getAttemptedSize()
((LengthLimitingJsonProcessingException) unwrapped).getMaxSize(),
((LengthLimitingJsonProcessingException) unwrapped).getAttemptedSize()
);
} else if (unwrapped instanceof OutputTooBigException) {
throw (OutputTooBigException) unwrapped;
}
return Objects.toString(val, "");
}
Expand Down
39 changes: 36 additions & 3 deletions src/test/java/com/hubspot/jinjava/lib/tag/eager/EagerTagDecoratorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.mockito.Mockito.*;

import com.hubspot.jinjava.BaseInterpretingTest;
import com.hubspot.jinjava.JinjavaConfig;
Expand All @@ -15,8 +13,11 @@
import com.hubspot.jinjava.lib.fn.ELFunctionDefinition;
import com.hubspot.jinjava.lib.tag.Tag;
import com.hubspot.jinjava.mode.EagerExecutionMode;
import com.hubspot.jinjava.objects.collections.PyList;
import com.hubspot.jinjava.objects.serialization.PyishSerializable;
import com.hubspot.jinjava.tree.TagNode;
import com.hubspot.jinjava.tree.parse.TagToken;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import org.junit.After;
Expand Down Expand Up @@ -157,4 +158,36 @@ public static void addToContext(String key, Object value) {
public static void modifyContext(String key, Object value) {
((List<Object>) JinjavaInterpreter.getCurrent().getContext().get(key)).add(value);
}

static class TooBig extends PyList implements PyishSerializable {

public TooBig(List<Object> list) {
super(list);
}

@Override
public <T extends Appendable & CharSequence> T appendPyishString(T appendable)
throws IOException {
throw new OutputTooBigException(1, 1);
}
}

@Test
public void itDefersNodeWhenOutputTooBigIsThrownWithinInnerInterpret() {
TooBig tooBig = new TooBig(new ArrayList<>());
interpreter =
new JinjavaInterpreter(
jinjava,
context,
JinjavaConfig
.newBuilder()
.withExecutionMode(EagerExecutionMode.instance())
.build()
);
interpreter.getContext().put("too_big", tooBig);
interpreter.render(
"{% for i in range(2) %}{% do too_big.append(deferred) %}{% endfor %}"
);
assertThat(interpreter.getContext().getDeferredNodes()).isNotEmpty();
}
}
3 changes: 3 additions & 0 deletions src/test/java/com/hubspot/jinjava/objects/serialization/PyishObjectMapperTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,10 @@ public void itLimitsDepth() {
assertThatThrownBy(() -> PyishObjectMapper.getAsPyishStringOrThrow(original))
.as("The string to be serialized is larger than the max output size")
.isInstanceOf(JsonMappingException.class)
.hasCauseInstanceOf(LengthLimitingJsonProcessingException.class)
.hasMessageContaining("Max length of 10000 chars reached");
assertThatThrownBy(() -> PyishObjectMapper.getAsPyishString(original))
.isInstanceOf(OutputTooBigException.class);
} finally {
JinjavaInterpreter.popCurrent();
}
Expand Down