Skip to content

Commit 1f211af

Browse files
chrfwowtoddbaert
andauthored
fix: possible StackOverflow on recursive contexts (#1760)
* Fix StackOverflow on recursive contexts Signed-off-by: christian.lutnik <christian.lutnik@dynatrace.com> * Fix StackOverflow on recursive contexts Signed-off-by: christian.lutnik <christian.lutnik@dynatrace.com> * Fix StackOverflow on recursive contexts Signed-off-by: christian.lutnik <christian.lutnik@dynatrace.com> --------- Signed-off-by: christian.lutnik <christian.lutnik@dynatrace.com> Co-authored-by: Todd Baert <todd.baert@dynatrace.com>
1 parent 1506a10 commit 1f211af

File tree

2 files changed

+30
-1
lines changed

2 files changed

+30
-1
lines changed

src/main/java/dev/openfeature/sdk/HookSupport.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,8 @@ public void executeBeforeHooks(HookSupportData data) {
6363
.orElse(Optional.empty());
6464
if (returnedEvalContext.isPresent()) {
6565
var returnedContext = returnedEvalContext.get();
66-
if (!returnedContext.isEmpty()) {
66+
// yes, we want to check for reference equality here, this prevents recursive layered contexts
67+
if (returnedContext != hookContext.getCtx() && !returnedContext.isEmpty()) {
6768
data.evaluationContext.putHookContext(returnedContext);
6869
}
6970
}

src/test/java/dev/openfeature/sdk/HookSupportTest.java

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package dev.openfeature.sdk;
22

33
import static org.assertj.core.api.Assertions.assertThat;
4+
import static org.assertj.core.api.Assertions.assertThatNoException;
45
import static org.mockito.ArgumentMatchers.any;
56
import static org.mockito.Mockito.mock;
67
import static org.mockito.Mockito.verify;
@@ -113,6 +114,33 @@ void shouldIsolateDataBetweenHooks(FlagValueType flagValueType) {
113114
assertHookData(testHook2, 2, "before", "after", "finallyAfter", "error");
114115
}
115116

117+
@Test
118+
void hookThatReturnsTheGivenContext_doesNotResultInAStackOverflow() {
119+
var hookSupportData = new HookSupportData();
120+
var recursiveHook = new Hook() {
121+
@Override
122+
public Optional<EvaluationContext> before(HookContext ctx, Map hints) {
123+
return Optional.of(ctx.getCtx());
124+
}
125+
};
126+
var emptyHook = new Hook() {
127+
@Override
128+
public Optional<EvaluationContext> before(HookContext ctx, Map hints) {
129+
return Optional.of(ImmutableContext.EMPTY);
130+
}
131+
};
132+
var layeredEvaluationContext =
133+
new LayeredEvaluationContext(evaluationContextWithValue("key", "value"), null, null, null);
134+
hookSupportData.evaluationContext = layeredEvaluationContext;
135+
hookSupport.setHooks(hookSupportData, List.of(recursiveHook, emptyHook), FlagValueType.STRING);
136+
hookSupport.setHookContexts(
137+
hookSupportData, getBaseHookContextForType(FlagValueType.STRING), layeredEvaluationContext);
138+
139+
callAllHooks(hookSupportData);
140+
141+
assertThatNoException().isThrownBy(layeredEvaluationContext::asObjectMap);
142+
}
143+
116144
private static void callAllHooks(HookSupportData hookSupportData) {
117145
hookSupport.executeBeforeHooks(hookSupportData);
118146
hookSupport.executeAfterHooks(

0 commit comments

Comments
 (0)