Skip to content

Commit

Permalink
Merge pull request #205 from gestalt-config/feat/include-nesting-limit
Browse files Browse the repository at this point in the history
feat: limit max include node nesting.
  • Loading branch information
credmond-git authored Aug 6, 2024
2 parents 1d4088a + ace3fbe commit 81eedcd
Show file tree
Hide file tree
Showing 10 changed files with 130 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.net.URI;
import java.util.Collection;

import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Arrays.asList;
import static software.amazon.awssdk.http.SdkHttpConfigurationOption.TRUST_ALL_CERTIFICATES;

Expand Down Expand Up @@ -81,7 +82,7 @@ void loadFile() throws GestaltException, IOException {
var allBytes = source.loadStream().readAllBytes();

InputStream fileStream = new FileInputStream(uploadFile);
Assertions.assertEquals(new String(allBytes), new String(fileStream.readAllBytes()));
Assertions.assertEquals(new String(allBytes, UTF_8), new String(fileStream.readAllBytes(), UTF_8));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*
* @author <a href="mailto:colin.redmond@outlook.com"> Colin Redmond </a> (c) 2024.
*/
@SuppressWarnings("OverloadMethodsDeclarationOrder")
public class GestaltCache implements Gestalt, CoreReloadListener {
private final Gestalt delegate;
private final Map<Triple<String, TypeCapture<?>, Tags>, Object> cache = Collections.synchronizedMap(new HashMap<>());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,9 @@ public class GestaltBuilder {
// The keyword that is used to determine if a node is an include from a source
private String nodeIncludeKeyword = null;

// The keyword that is used to determine if a node is an include from a source
private Integer nodeNestedIncludeLimit = null;

// Defines how the proxy decoder works. See the enum for details.
private ProxyDecoderMode proxyDecoderMode = null;

Expand Down Expand Up @@ -1287,6 +1290,17 @@ public GestaltBuilder setNodeIncludeKeyword(String nodeIncludeKeyword) {
return this;
}

/**
* Sets how many nested recursions during including nodes we will check before failing.
*
* @param nodeNestedIncludeLimit how many nested recursions during including nodes we will check before failing
* @return GestaltBuilder builder
*/
public GestaltBuilder setNodeNestedIncludeLimit(Integer nodeNestedIncludeLimit) {
this.nodeNestedIncludeLimit = nodeNestedIncludeLimit;
return this;
}

/**
* Get the mode the for proxy decoder.
*
Expand Down Expand Up @@ -1691,6 +1705,9 @@ private GestaltConfig rebuildConfig() {
newConfig.setNodeIncludeKeyword(Objects.requireNonNullElseGet(nodeIncludeKeyword,
() -> gestaltConfig.getNodeIncludeKeyword()));

newConfig.setNodeNestedIncludeLimit(Objects.requireNonNullElseGet(nodeNestedIncludeLimit,
() -> gestaltConfig.getNodeNestedIncludeLimit()));

return newConfig;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ public class GestaltConfig {

private String nodeIncludeKeyword = "$include";

private Integer nodeNestedIncludeLimit = 5;

// if observations should be enabled
private boolean observationsEnabled = false;

Expand Down Expand Up @@ -339,6 +341,24 @@ public void setNodeIncludeKeyword(String nodeImportKeyword) {
this.nodeIncludeKeyword = nodeImportKeyword;
}

/**
* Gets how many nested recursions during including nodes we will check before failing.
*
* @return how many nested recursions during including nodes we will check before failing
*/
public Integer getNodeNestedIncludeLimit() {
return nodeNestedIncludeLimit;
}

/**
* Sets how many nested recursions during including nodes we will check before failing.
*
* @param nodeNestedIncludeLimit how many nested recursions during including nodes we will check before failing
*/
public void setNodeNestedIncludeLimit(Integer nodeNestedIncludeLimit) {
this.nodeNestedIncludeLimit = nodeNestedIncludeLimit;
}

/**
* Get if the observations are enabled.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1941,5 +1941,24 @@ public String description() {
}
}

/**
* Exception while importing a Config Source.
*/
public static class ConfigNodeImportMaxNested extends ValidationError {
private final String path;
private final Integer numberNested;

public ConfigNodeImportMaxNested(String path, Integer numberNested) {
super(ValidationLevel.ERROR);
this.path = path;
this.numberNested = numberNested;
}

@Override
public String description() {
return "Reached the maximum nested import depth of: " + numberNested + ", on path: " + path +
", if this is intended increase the limit using GestaltBuilder.setNodeNestedIncludeLimit(10)";
}
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import org.github.gestalt.config.node.ConfigNodeService;

/**
* Holds configuration applied to all ConfigNodeFactory
* Holds configuration applied to all ConfigNodeFactory.
*
* @author <a href="mailto:colin.redmond@outlook.com"> Colin Redmond </a> (c) 2024.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import java.util.Map;

/**
* Factory for creating a File Config Node from parameters
* Factory for creating a File Config Node from parameters.
*
* <p>Load a config source from a File then converts it to a config node
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ public class ConfigNodeIncludeProcessor implements ConfigNodeProcessor {
private ConfigNodeFactoryService configNodeFactoryService;
private String nodeImportKeyword;
private SentenceLexer lexer;
private Integer nodeNestedIncludeLimit;

private List<Pair<Integer, ConfigNode>> buildOrderedIncludeNodes(String importKey, GResultOf<List<ConfigNode>> loadedConfigNode) {
// you can order the imports by having $include:3, pull out the order variable.
Expand All @@ -46,7 +47,7 @@ private List<Pair<Integer, ConfigNode>> buildOrderedIncludeNodes(String importKe
}

@SuppressWarnings("Indentation")
private static Map<String, String> convertStringToParameters(String path, String paramtersString, List<ValidationError> errors) {
private Map<String, String> convertStringToParameters(String path, String paramtersString, List<ValidationError> errors) {
return Arrays.stream(paramtersString.split(",")).map(it -> {
var parts = it.split("=");
if (parts.length != 2 || parts[0].isEmpty() || parts[1].isEmpty()) {
Expand All @@ -65,14 +66,23 @@ public void applyConfig(ConfigNodeProcessorConfig config) {
this.configNodeFactoryService = config.getConfigSourceFactoryService();
this.nodeImportKeyword = config.getConfig().getNodeIncludeKeyword();
this.lexer = config.getLexer();
this.nodeNestedIncludeLimit = config.getConfig().getNodeNestedIncludeLimit();
}

@Override
public GResultOf<ConfigNode> process(String path, ConfigNode currentNode) {
return process(path, currentNode, 0);
}

private GResultOf<ConfigNode> process(String path, ConfigNode currentNode, Integer nestedLevel) {
if (configNodeFactoryService == null || !(currentNode instanceof MapNode)) {
return GResultOf.result(currentNode);
}

if (nestedLevel >= nodeNestedIncludeLimit) {
return GResultOf.resultOf(currentNode, List.of(new ValidationError.ConfigNodeImportMaxNested(path, nodeNestedIncludeLimit)));
}

MapNode mapNode = (MapNode) currentNode;

// get the internal map.
Expand Down Expand Up @@ -140,7 +150,7 @@ public GResultOf<ConfigNode> process(String path, ConfigNode currentNode) {
// Merge the imported nodes along with the original config node minus the import entries.
ConfigNode mergedNode = mergeOrderedNodes(path, nodeAndOrderPair, errors);

var nestedNodes = process(path, mergedNode);
var nestedNodes = process(path, mergedNode, nestedLevel + 1);
errors.addAll(nestedNodes.getErrors());

if (nestedNodes.hasResults()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ public void build() throws GestaltException {
.setTreatMissingValuesAsErrors(true)
.setTreatMissingDiscretionaryValuesAsErrors(true)
.setNodeIncludeKeyword("$include")
.setNodeNestedIncludeLimit(10)
.setProxyDecoderMode(ProxyDecoderMode.CACHE);

Assertions.assertEquals(5, builder.getMaxSubstitutionNestedDepth());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import org.github.gestalt.config.entity.GestaltConfig;
import org.github.gestalt.config.entity.ValidationLevel;
import org.github.gestalt.config.exceptions.GestaltException;
import org.github.gestalt.config.lexer.PathLexer;
import org.github.gestalt.config.lexer.SentenceLexer;
import org.github.gestalt.config.node.ConfigNode;
Expand Down Expand Up @@ -42,7 +41,7 @@ public void setup() {
}

@Test
void processOkImportUnder() throws GestaltException {
void processOkImportUnder() {

Map<String, ConfigNode> originalNodeMap = new HashMap<>();
originalNodeMap.put("a", new LeafNode("a"));
Expand Down Expand Up @@ -79,7 +78,7 @@ void processOkImportUnder() throws GestaltException {
}

@Test
void processOkImportUnderDefined() throws GestaltException {
void processOkImportUnderDefined() {

Map<String, ConfigNode> originalNodeMap = new HashMap<>();
originalNodeMap.put("a", new LeafNode("a"));
Expand Down Expand Up @@ -115,7 +114,7 @@ void processOkImportUnderDefined() throws GestaltException {
}

@Test
void processOkImportOver() throws GestaltException {
void processOkImportOver() {

Map<String, ConfigNode> originalNodeMap = new HashMap<>();
originalNodeMap.put("a", new LeafNode("a"));
Expand Down Expand Up @@ -151,7 +150,7 @@ void processOkImportOver() throws GestaltException {
}

@Test
void processSingleNode() throws GestaltException {
void processSingleNode() {

Map<String, ConfigNode> originalNodeMap = new HashMap<>();
originalNodeMap.put("$include:1", new LeafNode("source=node"));
Expand Down Expand Up @@ -251,7 +250,7 @@ void processNotSetup() {
}

@Test
void processOkImportNullKey() throws GestaltException {
void processOkImportNullKey() {

Map<String, ConfigNode> originalNodeMap = new HashMap<>();
originalNodeMap.put("a", new LeafNode("a"));
Expand Down Expand Up @@ -293,7 +292,7 @@ void processOkImportNullKey() throws GestaltException {
}

@Test
void processErrorImportBadNodeType() throws GestaltException {
void processErrorImportBadNodeType() {

Map<String, ConfigNode> originalNodeMap = new HashMap<>();
originalNodeMap.put("a", new LeafNode("a"));
Expand Down Expand Up @@ -334,7 +333,7 @@ void processErrorImportBadNodeType() throws GestaltException {
}

@Test
void processErrorImportBadParametersLong() throws GestaltException {
void processErrorImportBadParametersLong() {

Map<String, ConfigNode> originalNodeMap = new HashMap<>();
originalNodeMap.put("a", new LeafNode("a"));
Expand Down Expand Up @@ -376,7 +375,7 @@ void processErrorImportBadParametersLong() throws GestaltException {
}

@Test
void processErrorImportBadParametersShort() throws GestaltException {
void processErrorImportBadParametersShort() {

Map<String, ConfigNode> originalNodeMap = new HashMap<>();
originalNodeMap.put("a", new LeafNode("a"));
Expand Down Expand Up @@ -419,7 +418,7 @@ void processErrorImportBadParametersShort() throws GestaltException {
}

@Test
void processErrorEmptyImportLeaf() throws GestaltException {
void processErrorEmptyImportLeaf() {

Map<String, ConfigNode> originalNodeMap = new HashMap<>();
originalNodeMap.put("a", new LeafNode("a"));
Expand Down Expand Up @@ -460,7 +459,7 @@ void processErrorEmptyImportLeaf() throws GestaltException {
}

@Test
void processErrorNullImportLeaf() throws GestaltException {
void processErrorNullImportLeaf() {

Map<String, ConfigNode> originalNodeMap = new HashMap<>();
originalNodeMap.put("a", new LeafNode("a"));
Expand Down Expand Up @@ -501,7 +500,7 @@ void processErrorNullImportLeaf() throws GestaltException {
}

@Test
void processErrorImportLeafValueNull() throws GestaltException {
void processErrorImportLeafValueNull() {

Map<String, ConfigNode> originalNodeMap = new HashMap<>();
originalNodeMap.put("a", new LeafNode("a"));
Expand Down Expand Up @@ -535,5 +534,50 @@ void processErrorImportLeafValueNull() throws GestaltException {
Assertions.assertEquals("b", mapResults.getKey("b").get().getValue().get());
Assertions.assertTrue(mapResults.getKey("$include").isEmpty());
}

@Test
void processErrorImportMaxNesting() {

Map<String, ConfigNode> originalNodeMap = new HashMap<>();
originalNodeMap.put("a", new LeafNode("a"));
originalNodeMap.put("b", new LeafNode("b"));
originalNodeMap.put("$include", new LeafNode("source=node"));

Map<String, ConfigNode> importNodeMap = new HashMap<>();
importNodeMap.put("b", new LeafNode("b changed"));
importNodeMap.put("c", new LeafNode("c"));
importNodeMap.put("$include", new LeafNode("source=node"));

ConfigNode originalRoot = new MapNode(originalNodeMap);
ConfigNode importRoot = new MapNode(importNodeMap);

ConfigNodeIncludeProcessor processor = new ConfigNodeIncludeProcessor();

Mockito.when(configNodeFactoryService.build(Mockito.any())).thenReturn(GResultOf.result(List.of(importRoot)));


processor.applyConfig(ppConfig);

var processedNodes = processor.process("test", originalRoot);

Assertions.assertTrue(processedNodes.hasResults());
Assertions.assertTrue(processedNodes.hasErrors());

Assertions.assertEquals(1, processedNodes.getErrors().size());
Assertions.assertEquals(ValidationLevel.ERROR, processedNodes.getErrors().get(0).level());
Assertions.assertEquals("Reached the maximum nested import depth of: 5, on path: test, " +
"if this is intended increase the limit using GestaltBuilder.setNodeNestedIncludeLimit(10)",
processedNodes.getErrors().get(0).description());

var results = processedNodes.results();

Assertions.assertInstanceOf(MapNode.class, results);
var mapResults = (MapNode) results;
Assertions.assertEquals(4, mapResults.size());
Assertions.assertEquals("a", mapResults.getKey("a").get().getValue().get());
Assertions.assertEquals("b", mapResults.getKey("b").get().getValue().get());
Assertions.assertEquals("c", mapResults.getKey("c").get().getValue().get());
Assertions.assertEquals("source=node", mapResults.getKey("$include").get().getValue().get());
}
}

0 comments on commit 81eedcd

Please sign in to comment.