This is an automated email from the ASF dual-hosted git repository. henrib pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-jexl.git
The following commit(s) were added to refs/heads/master by this push: new 9800c05 JEXL-307: as a default, copy options handled of context used at runtime to reduce the possibility of artificially complex mistakes; add an explicit flag to still allow purposeful usage Task #JEXL-307 - Variable redeclaration option 9800c05 is described below commit 9800c05277675e3edbfe5d1246141cdfab0e78e1 Author: henrib <hen...@apache.org> AuthorDate: Mon Nov 18 17:31:11 2019 +0100 JEXL-307: as a default, copy options handled of context used at runtime to reduce the possibility of artificially complex mistakes; add an explicit flag to still allow purposeful usage Task #JEXL-307 - Variable redeclaration option --- .../java/org/apache/commons/jexl3/JexlContext.java | 8 ++-- .../java/org/apache/commons/jexl3/JexlOptions.java | 25 +++++++++++- .../org/apache/commons/jexl3/internal/Engine.java | 44 ++++++++++++---------- .../org/apache/commons/jexl3/AnnotationTest.java | 1 + .../java/org/apache/commons/jexl3/LexicalTest.java | 17 +++++++++ 5 files changed, 69 insertions(+), 26 deletions(-) diff --git a/src/main/java/org/apache/commons/jexl3/JexlContext.java b/src/main/java/org/apache/commons/jexl3/JexlContext.java index ce730f7..2f4d69e 100644 --- a/src/main/java/org/apache/commons/jexl3/JexlContext.java +++ b/src/main/java/org/apache/commons/jexl3/JexlContext.java @@ -151,10 +151,10 @@ public interface JexlContext { /** * Retrieves the current set of options though the context. * <p> - * This method will be called once at beginning of evaluation and the - * JexlOptions instance kept as a property of the evaluator; - * the JexlOptions instance is free to alter its boolean flags during - * execution. + * This method will be called once at beginning of evaluation and an interpreter private copy + * of the context handled JexlOptions instance used for the duration of the execution; + * the context handled JexlOptions instance being only used as the source of that copy, + * it can safely alter its boolean flags during execution with no effect, avoiding any behavior ambiguity. * @return the engine options */ JexlOptions getEngineOptions(); diff --git a/src/main/java/org/apache/commons/jexl3/JexlOptions.java b/src/main/java/org/apache/commons/jexl3/JexlOptions.java index c650e9b..0682e4e 100644 --- a/src/main/java/org/apache/commons/jexl3/JexlOptions.java +++ b/src/main/java/org/apache/commons/jexl3/JexlOptions.java @@ -31,12 +31,15 @@ import org.apache.commons.jexl3.internal.Engine; * <li>lexicalShade: whether local variables shade global ones even outside their scope</li> * <li>strict: whether unknown or unsolvable identifiers are errors</li> * <li>strictArithmetic: whether null as operand is an error</li> + * <li>immutable: whether these options can be modified at runtime during execution (expert)</li> * </ul> * The sensible default is cancellable, strict and strictArithmetic. * <p>This interface replaces the now deprecated JexlEngine.Options. * @since 3.2 */ public final class JexlOptions { + /** The shared isntance bit. */ + private static final int SHARED = 7; /** The local shade bit. */ private static final int SHADE = 6; /** The antish var bit. */ @@ -53,10 +56,10 @@ public final class JexlOptions { private static final int CANCELLABLE = 0; /** The flags names ordered. */ private static final String[] NAMES = { - "cancellable", "strict", "silent", "safe", "lexical", "antish", "lexicalShade" + "cancellable", "strict", "silent", "safe", "lexical", "antish", "lexicalShade", "sharedInstance" }; /** Default mask .*/ - private static int DEFAULT = 1 /*<< CANCELLABLE*/ | 1 << STRICT | 1 << ANTISH | 1 << SAFE; + private static int DEFAULT = 1 /*<< CANCELLABLE*/ | 1 << STRICT | 1 << ANTISH | 1 << SAFE | 1 << SHARED; /** The arithmetic math context. */ private MathContext mathContext = null; /** The arithmetic math scale. */ @@ -334,6 +337,24 @@ public final class JexlOptions { } /** + * Whether these options are immutable at runtime. + * <p>Expert mode; allows instance handled through context to be shared + * instead of copied. + * @param flag true if shared, false if not + */ + public void setSharedInstance(boolean flag) { + flags = set(SHARED, flags, flag); + } + + /** + * @return false if a copy of these options is used during execution, + * true if those can potentially be modified + */ + public boolean isSharedInstance() { + return isSet(SHARED, flags); + } + + /** * Set options from engine. * @param jexl the engine * @return this instance diff --git a/src/main/java/org/apache/commons/jexl3/internal/Engine.java b/src/main/java/org/apache/commons/jexl3/internal/Engine.java index e068eb1..0296777 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/Engine.java +++ b/src/main/java/org/apache/commons/jexl3/internal/Engine.java @@ -301,32 +301,36 @@ public class Engine extends JexlEngine { } /** - * Extracts the engine evaluation options from context. + * Extracts the engine evaluation options from context if available, the engine + * options otherwise. + * <p>This creates a copy of the options so they are immutable during + * execution. * @param context the context * @return the options if any */ JexlOptions options(JexlContext context) { - JexlOptions jexlo = null; + // Make a copy of the handled options if any if (context instanceof JexlContext.OptionsHandle) { - jexlo = ((JexlContext.OptionsHandle) context).getEngineOptions(); + JexlOptions jexlo = ((JexlContext.OptionsHandle) context).getEngineOptions(); + if (jexlo != null) { + return jexlo.isSharedInstance()? jexlo : jexlo.copy(); + } } - if (jexlo == null) { - jexlo = options; - /** The following block for compatibility between 3.1 and 3.2*/ - if (context instanceof JexlEngine.Options) { - jexlo = jexlo.copy(); - JexlEngine jexl = this; - JexlEngine.Options opts = (JexlEngine.Options) context; - jexlo.setCancellable(option(opts.isCancellable(), jexl.isCancellable())); - jexlo.setSilent(option(opts.isSilent(), jexl.isSilent())); - jexlo.setStrict(option(opts.isStrict(), jexl.isStrict())); - JexlArithmetic jexla = jexl.getArithmetic(); - jexlo.setStrictArithmetic(option(opts.isStrictArithmetic(), jexla.isStrict())); - jexlo.setMathContext(opts.getArithmeticMathContext()); - jexlo.setMathScale(opts.getArithmeticMathScale()); - } + // The following block for compatibility between 3.1 and 3.2 + else if (context instanceof JexlEngine.Options) { + JexlOptions jexlo = options.copy(); + JexlEngine jexl = this; + JexlEngine.Options opts = (JexlEngine.Options) context; + jexlo.setCancellable(option(opts.isCancellable(), jexl.isCancellable())); + jexlo.setSilent(option(opts.isSilent(), jexl.isSilent())); + jexlo.setStrict(option(opts.isStrict(), jexl.isStrict())); + JexlArithmetic jexla = jexl.getArithmetic(); + jexlo.setStrictArithmetic(option(opts.isStrictArithmetic(), jexla.isStrict())); + jexlo.setMathContext(opts.getArithmeticMathContext()); + jexlo.setMathScale(opts.getArithmeticMathScale()); + return jexlo; } - return jexlo; + return options; } /** @@ -382,7 +386,7 @@ public class Engine extends JexlEngine { for(Map.Entry<String, Object> pragma : pragmas.entrySet()) { String key = pragma.getKey(); Object value = pragma.getValue(); - if (PRAGMA_OPTIONS.equals(key) && opts != null) { + if (PRAGMA_OPTIONS.equals(key)) { if (value instanceof String) { String[] vs = ((String) value).split(" "); opts.setFlags(vs); diff --git a/src/test/java/org/apache/commons/jexl3/AnnotationTest.java b/src/test/java/org/apache/commons/jexl3/AnnotationTest.java index a4aebb5..e77acef 100644 --- a/src/test/java/org/apache/commons/jexl3/AnnotationTest.java +++ b/src/test/java/org/apache/commons/jexl3/AnnotationTest.java @@ -121,6 +121,7 @@ public class AnnotationTest extends JexlTestCase { OptAnnotationContext jc = new OptAnnotationContext(); JexlOptions options = jc.getEngineOptions(); jc.getEngineOptions().set(JEXL); + options.setSharedInstance(true); JexlScript e; Object r; e = JEXL.createScript("(s, v)->{ @strict(s) @silent(v) var x = y ; 42; }"); diff --git a/src/test/java/org/apache/commons/jexl3/LexicalTest.java b/src/test/java/org/apache/commons/jexl3/LexicalTest.java index 891393a..d950d3a 100644 --- a/src/test/java/org/apache/commons/jexl3/LexicalTest.java +++ b/src/test/java/org/apache/commons/jexl3/LexicalTest.java @@ -387,4 +387,21 @@ public class LexicalTest { } } + @Test + public void testLexicalOption() throws Exception { + JexlFeatures f= new JexlFeatures(); + JexlEngine jexl = new JexlBuilder().features(f).strict(true).create(); + JexlEvalContext ctxt = new JexlEvalContext(); + JexlOptions options = ctxt.getEngineOptions(); + options.setLexical(true); + options.setLexicalShade(true); + ctxt.set("options", options); + JexlScript script = jexl.createScript("{var x = 42;} options.lexical = false; x"); + try { + Object result = script.execute(ctxt); + Assert.fail("setting options.lexical should have no effect during execution"); + } catch(JexlException xf) { + Assert.assertNotNull(xf); + } + } }