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

Introduces a limited length render filter and HTML tag close filter #1128

Merged
merged 16 commits into from
Nov 27, 2023
Merged
Show file tree
Hide file tree
Changes from 15 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
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,11 @@ public String renderFlat(String template) {
* @return rendered result
*/
public String render(String template) {
return render(parse(template), true);
return render(template, config.getMaxOutputSize());
}

public String render(String template, long renderLimit) {
return render(parse(template), true, renderLimit);
}

/**
Expand All @@ -270,7 +274,19 @@ public String render(String template) {
* @return rendered result
*/
public String render(Node root) {
return render(root, true);
return render(root, true, config.getMaxOutputSize());
}

/**
* Render the given root node with an option to process extend parents.
* Equivalent to render(root, processExtendRoots).
* @param root
* node to render
* @param processExtendRoots
* @return
*/
public String render(Node root, boolean processExtendRoots) {
return render(root, processExtendRoots, config.getMaxOutputSize());
}

/**
Expand All @@ -280,11 +296,12 @@ public String render(Node root) {
* node to render
* @param processExtendRoots
* if true, also render all extend parents
* @param renderLimit
* the number of characters the result may contain
* @return rendered result
*/
public String render(Node root, boolean processExtendRoots) {
OutputList output = new OutputList(config.getMaxOutputSize());

private String render(Node root, boolean processExtendRoots, long renderLimit) {
OutputList output = new OutputList(clampOutputSizeSafely(renderLimit));
for (Node node : root.getChildren()) {
lineNumber = node.getLineNumber();
position = node.getStartPosition();
Expand Down Expand Up @@ -340,8 +357,8 @@ public String render(Node root, boolean processExtendRoots) {
return output.getValue();
}
}
StringBuilder ignoredOutput = new StringBuilder();

StringBuilder ignoredOutput = new StringBuilder();
// render all extend parents, keeping the last as the root output
if (processExtendRoots) {
Set<String> extendPaths = new HashSet<>();
Expand Down Expand Up @@ -406,6 +423,7 @@ public String render(Node root, boolean processExtendRoots) {
}

resolveBlockStubs(output);

if (ignoredOutput.length() > 0) {
return (
EagerReconstructionUtils.labelWithNotes(
Expand All @@ -421,7 +439,6 @@ public String render(Node root, boolean processExtendRoots) {
output.getValue()
);
}

return output.getValue();
}

Expand Down Expand Up @@ -907,6 +924,20 @@ private String getWrappedErrorMessage(
}
}

private long clampOutputSizeSafely(long providedLimit) {
long configMaxOutput = config.getMaxOutputSize();

if (configMaxOutput == 0) {
return providedLimit;
}

if (providedLimit <= 0) {
return configMaxOutput;
}

return Math.min(providedLimit, configMaxOutput);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good, can we just get some tests for this logic?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done, thanks a lot Jack. To avoid repeating tests or changing signatures I moved this into a small static utils class which I tested on it's own. Let me know if you'd rather I just make it public in the interpreter or add more tests to the render method as a whole and take it back to being private.

I did like this a bit for simplicity though.


@Override
@SuppressWarnings("unchecked")
public <T extends Appendable & CharSequence> T appendPyishString(T appendable)
Expand Down
26 changes: 26 additions & 0 deletions src/main/java/com/hubspot/jinjava/lib/filter/CloseHtmlFilter.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package com.hubspot.jinjava.lib.filter;

import com.hubspot.jinjava.doc.annotations.JinjavaDoc;
import com.hubspot.jinjava.doc.annotations.JinjavaParam;
import com.hubspot.jinjava.doc.annotations.JinjavaSnippet;
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
import java.util.Objects;
import org.jsoup.Jsoup;

@JinjavaDoc(
value = "Closes open HTML tags in a string",
input = @JinjavaParam(value = "s", desc = "String to close", required = true),
snippets = { @JinjavaSnippet(code = "{{ \"<p> Hello, world\"|closehtml }}") }
)
public class CloseHtmlFilter implements Filter {

@Override
public String getName() {
return "closehtml";
}

@Override
public Object filter(Object var, JinjavaInterpreter interpreter, String... args) {
return Jsoup.parseBodyFragment(Objects.toString(var)).body().html();
}
}
12 changes: 12 additions & 0 deletions src/main/java/com/hubspot/jinjava/lib/filter/RenderFilter.java
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package com.hubspot.jinjava.lib.filter;

import com.hubspot.jinjava.JinjavaConfig;
import com.hubspot.jinjava.doc.annotations.JinjavaDoc;
import com.hubspot.jinjava.doc.annotations.JinjavaParam;
import com.hubspot.jinjava.doc.annotations.JinjavaSnippet;
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
import java.util.Objects;
import org.apache.commons.lang3.math.NumberUtils;

@JinjavaDoc(
value = "Renders a template string early to be used by other filters and functions",
Expand All @@ -24,6 +26,16 @@ public String getName() {

@Override
public Object filter(Object var, JinjavaInterpreter interpreter, String... args) {
if (args.length > 0) {
String firstArg = args[0];
return interpreter.render(
Objects.toString(var),
NumberUtils.toLong(
firstArg,
JinjavaConfig.newBuilder().build().getMaxOutputSize()
)
);
}
return interpreter.render(Objects.toString(var));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package com.hubspot.jinjava.lib.filter;

import static org.assertj.core.api.Assertions.assertThat;

import com.hubspot.jinjava.BaseInterpretingTest;
import org.junit.Before;
import org.junit.Test;

public class CloseHtmlFilterTest extends BaseInterpretingTest {
CloseHtmlFilter f;

@Before
public void setup() {
f = new CloseHtmlFilter();
}

@Test
public void itClosesTags() {
String openTags = "<p>Hello, world!";
assertThat(f.filter(openTags, interpreter)).isEqualTo("<p>Hello, world!</p>");
}

@Test
public void itIgnoresClosedTags() {
String openTags = "<p>Hello, world!</p>";
assertThat(f.filter(openTags, interpreter)).isEqualTo("<p>Hello, world!</p>");
}

@Test
public void itClosesMultipleTags() {
String openTags = "<h1><p>Hello, world!";
assertThat(f.filter(openTags, interpreter))
.isEqualTo("<h1><p>Hello, world!</p></h1>");
}
}
38 changes: 38 additions & 0 deletions src/test/java/com/hubspot/jinjava/lib/filter/RenderFilterTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,42 @@ public void itRendersObject() {

assertThat(filter.filter(stringToRender, interpreter)).isEqualTo("world");
}

@Test
public void itRendersObjectWithinLimit() {
String stringToRender = "{% if null %}Hello{% else %}world{% endif %}";

assertThat(filter.filter(stringToRender, interpreter, "5")).isEqualTo("world");
}

@Test
public void itDoesNotRenderObjectOverLimit() {
String stringToRender = "{% if null %}Hello{% else %}world{% endif %}";

assertThat(filter.filter(stringToRender, interpreter, "4")).isEqualTo("");
}

@Test
public void itRendersPartialObjectOverLimit() {
String stringToRender = "Hello{% if null %}Hello{% else %}world{% endif %}";

assertThat(filter.filter(stringToRender, interpreter, "7")).isEqualTo("Hello");
}

@Test
public void itCountsHtmlTags() {
String stringToRender = "<p>Hello</p>{% if null %}Hello{% else %}world{% endif %}";

assertThat(filter.filter(stringToRender, interpreter, "15"))
.isEqualTo("<p>Hello</p>");
}

@Test
public void itDoesNotAlwaysCompleteHtmlTags() {
String stringToRender =
"<p> Hello, {% if null %}world{% else %}world!{% endif %} </p>";

assertThat(filter.filter(stringToRender, interpreter, "17"))
.isEqualTo("<p> Hello, world!");
}
}