IMPORTANT: svn commit: r1331355 - in /ofbiz/trunk/framework/minilang: dtd/simple-methods-v2.xsd src/org/ofbiz/minilang/method/callops/CallScript.java

[Adrian Crum-3]

In this commit I changed the <script> element "location" attribute from
a FlexibleMapAccessor to a String. In other words, I changed the
attribute type from expression to constant. This change was made for
security reasons. Developers - please don't change it back.

-Adrian

On 4/27/2012 12:18 PM, [hidden email] wrote:

> Author: adrianc
> Date: Fri Apr 27 11:18:52 2012
> New Revision: 1331355
>
> URL: http://svn.apache.org/viewvc?rev=1331355&view=rev
> Log:
> Overhauled Mini-language<script>  element. Added a "script" attribute so it can be used for small inline scripts (scriptlets).
>
> Removed code that hid script engine exceptions in the error list. If a script engine throws an exception, it should be passed to the calling process, not hidden in an error message list (which might not get checked).
>
> Modified:
>      ofbiz/trunk/framework/minilang/dtd/simple-methods-v2.xsd
>      ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/CallScript.java
>
> Modified: ofbiz/trunk/framework/minilang/dtd/simple-methods-v2.xsd
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/dtd/simple-methods-v2.xsd?rev=1331355&r1=1331354&r2=1331355&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/minilang/dtd/simple-methods-v2.xsd (original)
> +++ ofbiz/trunk/framework/minilang/dtd/simple-methods-v2.xsd Fri Apr 27 11:18:52 2012
> @@ -854,31 +854,34 @@ under the License.
>       <xs:element name="script" substitutionGroup="CallOperations">
>           <xs:annotation>
>               <xs:documentation>
> -                Runs an external script (minilang, bsh, groovy) from the expanded location provided.
> +                Runs an external script or a short inline script (scriptlet).
>                   Error messages go on the error list and are handled with the check-errors tag.
>               </xs:documentation>
>           </xs:annotation>
>           <xs:complexType mixed="true">
> -<xs:attributeGroup ref="attlist.script"/>
> +<xs:attribute type="xs:string" name="location">
> +<xs:annotation>
> +<xs:documentation>
> +                        The script location. The location attribute accepts the component:// file location
> +                        protocol. Script functions/methods can be invoked by appending a hash (#) and the
> +                        function/method name.
> +&lt;br/&gt;&lt;br/&gt;
> +                        Required if the script attribute is empty. Attribute type: constant.
> +</xs:documentation>
> +</xs:annotation>
> +</xs:attribute>
> +<xs:attribute type="xs:string" name="script">
> +<xs:annotation>
> +<xs:documentation>
> +                        A short script (scriptlet). Can be used instead of a file.
> +                        The script must be prefixed with the script language followed by a colon (&quot;:&quot;).
> +&lt;br/&gt;&lt;br/&gt;
> +                        Required if the location attribute is empty. Attribute type: script.
> +</xs:documentation>
> +</xs:annotation>
> +</xs:attribute>
>           </xs:complexType>
>       </xs:element>
> -<xs:attributeGroup name="attlist.script">
> -<xs:attribute type="xs:string" name="location">
> -<xs:annotation>
> -<xs:documentation>
> -                    Script location (component://...)
> -</xs:documentation>
> -</xs:annotation>
> -</xs:attribute>
> -<xs:attribute type="xs:string" name="error-list-name" default="error_list">
> -<xs:annotation>
> -<xs:documentation>
> -                    The name of the list in the method environment to check for error messages.
> -                    Defaults to "error_list".
> -</xs:documentation>
> -</xs:annotation>
> -</xs:attribute>
> -</xs:attributeGroup>
>       <xs:element name="call-bsh" substitutionGroup="CallOperations">
>           <xs:annotation>
>               <xs:documentation>
> @@ -921,8 +924,7 @@ under the License.
>                   Calls another simple-method in the same context as the current one.
>                   The called simple-method will have the same environment as the calling simple-method,
>                   including all environment fields, and either the event or service objects
> -                that the calling simple-method was called
> -                with.
> +                that the calling simple-method was called with.
>               </xs:documentation>
>           </xs:annotation>
>           <xs:complexType>
> @@ -930,7 +932,7 @@ under the License.
>                   <xs:element ref="result-to-field" minOccurs="0" maxOccurs="unbounded">
>                       <xs:annotation>
>                           <xs:documentation>
> -                            Used when memory-model=&quot;function&quot;. Copies the called method fields
> +                            Used when scope=&quot;function&quot;. Copies the called method fields
>                               to the calling method fields.
>                           </xs:documentation>
>                       </xs:annotation>
>
> Modified: ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/CallScript.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/CallScript.java?rev=1331355&r1=1331354&r2=1331355&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/CallScript.java (original)
> +++ ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/CallScript.java Fri Apr 27 11:18:52 2012
> @@ -18,84 +18,101 @@
>    *******************************************************************************/
>   package org.ofbiz.minilang.method.callops;
>
> -import java.util.List;
> -import java.util.Map;
> -
> -import javolution.util.FastList;
> -
>   import org.ofbiz.base.util.ScriptUtil;
> +import org.ofbiz.base.util.Scriptlet;
> +import org.ofbiz.base.util.StringUtil;
> +import org.ofbiz.base.util.string.FlexibleStringExpander;
>   import org.ofbiz.minilang.MiniLangException;
> +import org.ofbiz.minilang.MiniLangRuntimeException;
> +import org.ofbiz.minilang.MiniLangUtil;
> +import org.ofbiz.minilang.MiniLangValidate;
>   import org.ofbiz.minilang.SimpleMethod;
> -import org.ofbiz.minilang.method.ContextAccessor;
>   import org.ofbiz.minilang.method.MethodContext;
>   import org.ofbiz.minilang.method.MethodOperation;
>   import org.w3c.dom.Element;
>
> -public class CallScript extends MethodOperation {
> +/**
> + * Executes a script.
> + */
> +public final class CallScript extends MethodOperation {
>
>       public static final String module = CallScript.class.getName();
>
> -    private static String getScriptLocation(String combinedName) {
> -        int pos = combinedName.lastIndexOf("#");
> -        if (pos == -1) {
> -            return combinedName;
> -        }
> -        return combinedName.substring(0, pos);
> -    }
> -
> -    private static String getScriptMethodName(String combinedName) {
> -        int pos = combinedName.lastIndexOf("#");
> -        if (pos == -1) {
> -            return null;
> -        }
> -        return combinedName.substring(pos + 1);
> -    }
> -
> -    private ContextAccessor<List<Object>>  errorListAcsr;
> -    private String location;
> -    private String method;
> +    private final String location;
> +    private final String method;
> +    private final Scriptlet scriptlet;
>
>       public CallScript(Element element, SimpleMethod simpleMethod) throws MiniLangException {
>           super(element, simpleMethod);
> -        String scriptLocation = element.getAttribute("location");
> -        this.location = getScriptLocation(scriptLocation);
> -        this.method = getScriptMethodName(scriptLocation);
> -        this.errorListAcsr = new ContextAccessor<List<Object>>(element.getAttribute("error-list-name"), "error_list");
> +        if (MiniLangValidate.validationOn()) {
> +            MiniLangValidate.attributeNames(simpleMethod, element, "location", "script");
> +            MiniLangValidate.requireAnyAttribute(simpleMethod, element, "location", "script");
> +            MiniLangValidate.constantAttributes(simpleMethod, element, "location");
> +            MiniLangValidate.scriptAttributes(simpleMethod, element, "script");
> +            MiniLangValidate.noChildElements(simpleMethod, element);
> +        }
> +        String scriptAttribute = element.getAttribute("script");
> +        if (MiniLangUtil.containsScript(scriptAttribute)) {
> +            this.scriptlet = new Scriptlet(StringUtil.convertOperatorSubstitutions(scriptAttribute));
> +            this.location = null;
> +            this.method = null;
> +        } else {
> +            this.scriptlet = null;
> +            String scriptLocation = element.getAttribute("location");
> +            int pos = scriptLocation.lastIndexOf("#");
> +            if (pos == -1) {
> +                this.location = scriptLocation;
> +                this.method = null;
> +            } else {
> +                this.location = scriptLocation.substring(0, pos);
> +                this.method = scriptLocation.substring(pos + 1);
> +            }
> +        }
>       }
>
>       @Override
>       public boolean exec(MethodContext methodContext) throws MiniLangException {
> -        String location = methodContext.expandString(this.location);
> -        String method = methodContext.expandString(this.method);
> -        List<Object>  messages = errorListAcsr.get(methodContext);
> -        if (messages == null) {
> -            messages = FastList.newInstance();
> -            errorListAcsr.put(methodContext, messages);
> -        }
> -        Map<String, Object>  context = methodContext.getEnvMap();
> -        if (location.endsWith(".xml")) {
> +        if (this.scriptlet != null) {
>               try {
> -                SimpleMethod.runSimpleMethod(location, method, methodContext);
> -            } catch (MiniLangException e) {
> -                messages.add("Error running simple method at location [" + location + "]: " + e.getMessage());
> +                this.scriptlet.executeScript(methodContext.getEnvMap());
> +            } catch (Exception e) {
> +                throw new MiniLangRuntimeException(e.getMessage(), this);
>               }
> +            return true;
> +        }
> +        if (location.endsWith(".xml")) {
> +            SimpleMethod.runSimpleMethod(location, method, methodContext);
>           } else {
> -            ScriptUtil.executeScript(this.location, this.method, context);
> +            ScriptUtil.executeScript(this.location, this.method, methodContext.getEnvMap());
>           }
> -        // update the method environment
> -        methodContext.putAllEnv(context);
> -        // always return true, error messages just go on the error list
>           return true;
>       }
>
>       @Override
>       public String expandedString(MethodContext methodContext) {
> -        return rawString();
> +        return FlexibleStringExpander.expandString(toString(), methodContext.getEnvMap());
>       }
>
>       @Override
>       public String rawString() {
> -        return "<script/>";
> +        return toString();
> +    }
> +
> +    @Override
> +    public String toString() {
> +        StringBuilder sb = new StringBuilder("<script ");
> +        if (this.location != null&&  this.location.length()>  0) {
> +            sb.append("location=\"").append(this.location);
> +            if (this.method != null&&  this.method.length()>  0) {
> +                sb.append("#").append(this.method);
> +            }
> +            sb.append("\" ");
> +        }
> +        if (this.scriptlet != null) {
> +            sb.append("scriptlet=\"").append(this.scriptlet).append("\" ");
> +        }
> +        sb.append("/>");
> +        return sb.toString();
>       }
>
>       public static final class CallScriptFactory implements Factory<CallScript>  {
>
>

[Pierre Smits]

Adrian,

Could you be a bit more explaining as to why developers should change it
back? It will help in understanding the underlying issue (the root and
cause).

Regards,

Pierre

Op 27 april 2012 13:34 schreef Adrian Crum <
[hidden email]> het volgende:

> In this commit I changed the <script> element "location" attribute from a
> FlexibleMapAccessor to a String. In other words, I changed the attribute
> type from expression to constant. This change was made for security
> reasons. Developers - please don't change it back.
>
> -Adrian
>
> On 4/27/2012 12:18 PM, [hidden email] wrote:
>
>> Author: adrianc
>> Date: Fri Apr 27 11:18:52 2012
>> New Revision: 1331355
>>
>> URL: http://svn.apache.org/viewvc?**rev=1331355&view=rev<http://svn.apache.org/viewvc?rev=1331355&view=rev>
>> Log:
>> Overhauled Mini-language<script>  element. Added a "script" attribute so
>> it can be used for small inline scripts (scriptlets).
>>
>> Removed code that hid script engine exceptions in the error list. If a
>> script engine throws an exception, it should be passed to the calling
>> process, not hidden in an error message list (which might not get checked).
>>
>> Modified:
>>     ofbiz/trunk/framework/**minilang/dtd/simple-methods-**v2.xsd
>>     ofbiz/trunk/framework/**minilang/src/org/ofbiz/**
>> minilang/method/callops/**CallScript.java
>>
>> Modified: ofbiz/trunk/framework/**minilang/dtd/simple-methods-**v2.xsd
>> URL: http://svn.apache.org/viewvc/**ofbiz/trunk/framework/**
>> minilang/dtd/simple-methods-**v2.xsd?rev=1331355&r1=1331354&**
>> r2=1331355&view=diff<http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/dtd/simple-methods-v2.xsd?rev=1331355&r1=1331354&r2=1331355&view=diff>
>> ==============================**==============================**
>> ==================
>> --- ofbiz/trunk/framework/**minilang/dtd/simple-methods-**v2.xsd
>> (original)
>> +++ ofbiz/trunk/framework/**minilang/dtd/simple-methods-**v2.xsd Fri Apr
>> 27 11:18:52 2012
>> @@ -854,31 +854,34 @@ under the License.
>>      <xs:element name="script" substitutionGroup="**CallOperations">
>>          <xs:annotation>
>>              <xs:documentation>
>> -                Runs an external script (minilang, bsh, groovy) from the
>> expanded location provided.
>> +                Runs an external script or a short inline script
>> (scriptlet).
>>                  Error messages go on the error list and are handled with
>> the check-errors tag.
>>              </xs:documentation>
>>          </xs:annotation>
>>          <xs:complexType mixed="true">
>> -<xs:attributeGroup ref="attlist.script"/>
>> +<xs:attribute type="xs:string" name="location">
>> +<xs:annotation>
>> +<xs:documentation>
>> +                        The script location. The location attribute
>> accepts the component:// file location
>> +                        protocol. Script functions/methods can be
>> invoked by appending a hash (#) and the
>> +                        function/method name.
>> +&lt;br/&gt;&lt;br/&gt;
>> +                        Required if the script attribute is empty.
>> Attribute type: constant.
>> +</xs:documentation>
>> +</xs:annotation>
>> +</xs:attribute>
>> +<xs:attribute type="xs:string" name="script">
>> +<xs:annotation>
>> +<xs:documentation>
>> +                        A short script (scriptlet). Can be used instead
>> of a file.
>> +                        The script must be prefixed with the script
>> language followed by a colon (&quot;:&quot;).
>> +&lt;br/&gt;&lt;br/&gt;
>> +                        Required if the location attribute is empty.
>> Attribute type: script.
>> +</xs:documentation>
>> +</xs:annotation>
>> +</xs:attribute>
>>          </xs:complexType>
>>      </xs:element>
>> -<xs:attributeGroup name="attlist.script">
>> -<xs:attribute type="xs:string" name="location">
>> -<xs:annotation>
>> -<xs:documentation>
>> -                    Script location (component://...)
>> -</xs:documentation>
>> -</xs:annotation>
>> -</xs:attribute>
>> -<xs:attribute type="xs:string" name="error-list-name"
>> default="error_list">
>> -<xs:annotation>
>> -<xs:documentation>
>> -                    The name of the list in the method environment to
>> check for error messages.
>> -                    Defaults to "error_list".
>> -</xs:documentation>
>> -</xs:annotation>
>> -</xs:attribute>
>> -</xs:attributeGroup>
>>      <xs:element name="call-bsh" substitutionGroup="**CallOperations">
>>          <xs:annotation>
>>              <xs:documentation>
>> @@ -921,8 +924,7 @@ under the License.
>>                  Calls another simple-method in the same context as the
>> current one.
>>                  The called simple-method will have the same environment
>> as the calling simple-method,
>>                  including all environment fields, and either the event
>> or service objects
>> -                that the calling simple-method was called
>> -                with.
>> +                that the calling simple-method was called with.
>>              </xs:documentation>
>>          </xs:annotation>
>>          <xs:complexType>
>> @@ -930,7 +932,7 @@ under the License.
>>                  <xs:element ref="result-to-field" minOccurs="0"
>> maxOccurs="unbounded">
>>                      <xs:annotation>
>>                          <xs:documentation>
>> -                            Used when memory-model=&quot;function&**quot;.
>> Copies the called method fields
>> +                            Used when scope=&quot;function&quot;. Copies
>> the called method fields
>>                              to the calling method fields.
>>                          </xs:documentation>
>>                      </xs:annotation>
>>
>> Modified: ofbiz/trunk/framework/**minilang/src/org/ofbiz/**
>> minilang/method/callops/**CallScript.java
>> URL: http://svn.apache.org/viewvc/**ofbiz/trunk/framework/**
>> minilang/src/org/ofbiz/**minilang/method/callops/**
>> CallScript.java?rev=1331355&**r1=1331354&r2=1331355&view=**diff<http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/CallScript.java?rev=1331355&r1=1331354&r2=1331355&view=diff>
>> ==============================**==============================**
>> ==================
>> --- ofbiz/trunk/framework/**minilang/src/org/ofbiz/**
>> minilang/method/callops/**CallScript.java (original)
>> +++ ofbiz/trunk/framework/**minilang/src/org/ofbiz/**
>> minilang/method/callops/**CallScript.java Fri Apr 27 11:18:52 2012
>> @@ -18,84 +18,101 @@
>>   ****************************************************************
>> *******************/
>>  package org.ofbiz.minilang.method.**callops;
>>
>> -import java.util.List;
>> -import java.util.Map;
>> -
>> -import javolution.util.FastList;
>> -
>>  import org.ofbiz.base.util.**ScriptUtil;
>> +import org.ofbiz.base.util.Scriptlet;
>> +import org.ofbiz.base.util.**StringUtil;
>> +import org.ofbiz.base.util.string.**FlexibleStringExpander;
>>  import org.ofbiz.minilang.**MiniLangException;
>> +import org.ofbiz.minilang.**MiniLangRuntimeException;
>> +import org.ofbiz.minilang.**MiniLangUtil;
>> +import org.ofbiz.minilang.**MiniLangValidate;
>>  import org.ofbiz.minilang.**SimpleMethod;
>> -import org.ofbiz.minilang.method.**ContextAccessor;
>>  import org.ofbiz.minilang.method.**MethodContext;
>>  import org.ofbiz.minilang.method.**MethodOperation;
>>  import org.w3c.dom.Element;
>>
>> -public class CallScript extends MethodOperation {
>> +/**
>> + * Executes a script.
>> + */
>> +public final class CallScript extends MethodOperation {
>>
>>      public static final String module = CallScript.class.getName();
>>
>> -    private static String getScriptLocation(String combinedName) {
>> -        int pos = combinedName.lastIndexOf("#");
>> -        if (pos == -1) {
>> -            return combinedName;
>> -        }
>> -        return combinedName.substring(0, pos);
>> -    }
>> -
>> -    private static String getScriptMethodName(String combinedName) {
>> -        int pos = combinedName.lastIndexOf("#");
>> -        if (pos == -1) {
>> -            return null;
>> -        }
>> -        return combinedName.substring(pos + 1);
>> -    }
>> -
>> -    private ContextAccessor<List<Object>>  errorListAcsr;
>> -    private String location;
>> -    private String method;
>> +    private final String location;
>> +    private final String method;
>> +    private final Scriptlet scriptlet;
>>
>>      public CallScript(Element element, SimpleMethod simpleMethod) throws
>> MiniLangException {
>>          super(element, simpleMethod);
>> -        String scriptLocation = element.getAttribute("**location");
>> -        this.location = getScriptLocation(**scriptLocation);
>> -        this.method = getScriptMethodName(**scriptLocation);
>> -        this.errorListAcsr = new ContextAccessor<List<Object>>(**
>> element.getAttribute("error-**list-name"), "error_list");
>> +        if (MiniLangValidate.**validationOn()) {
>> +            MiniLangValidate.**attributeNames(simpleMethod, element,
>> "location", "script");
>> +            MiniLangValidate.**requireAnyAttribute(**simpleMethod,
>> element, "location", "script");
>> +            MiniLangValidate.**constantAttributes(**simpleMethod,
>> element, "location");
>> +            MiniLangValidate.**scriptAttributes(simpleMethod, element,
>> "script");
>> +            MiniLangValidate.**noChildElements(simpleMethod, element);
>> +        }
>> +        String scriptAttribute = element.getAttribute("script")**;
>> +        if (MiniLangUtil.containsScript(**scriptAttribute)) {
>> +            this.scriptlet = new Scriptlet(StringUtil.**
>> convertOperatorSubstitutions(**scriptAttribute));
>> +            this.location = null;
>> +            this.method = null;
>> +        } else {
>> +            this.scriptlet = null;
>> +            String scriptLocation = element.getAttribute("**location");
>> +            int pos = scriptLocation.lastIndexOf("#"**);
>> +            if (pos == -1) {
>> +                this.location = scriptLocation;
>> +                this.method = null;
>> +            } else {
>> +                this.location = scriptLocation.substring(0, pos);
>> +                this.method = scriptLocation.substring(pos + 1);
>> +            }
>> +        }
>>      }
>>
>>      @Override
>>      public boolean exec(MethodContext methodContext) throws
>> MiniLangException {
>> -        String location = methodContext.expandString(**this.location);
>> -        String method = methodContext.expandString(**this.method);
>> -        List<Object>  messages = errorListAcsr.get(**methodContext);
>> -        if (messages == null) {
>> -            messages = FastList.newInstance();
>> -            errorListAcsr.put(**methodContext, messages);
>> -        }
>> -        Map<String, Object>  context = methodContext.getEnvMap();
>> -        if (location.endsWith(".xml")) {
>> +        if (this.scriptlet != null) {
>>              try {
>> -                SimpleMethod.runSimpleMethod(**location, method,
>> methodContext);
>> -            } catch (MiniLangException e) {
>> -                messages.add("Error running simple method at location ["
>> + location + "]: " + e.getMessage());
>> +                this.scriptlet.executeScript(**
>> methodContext.getEnvMap());
>> +            } catch (Exception e) {
>> +                throw new MiniLangRuntimeException(e.**getMessage(),
>> this);
>>              }
>> +            return true;
>> +        }
>> +        if (location.endsWith(".xml")) {
>> +            SimpleMethod.runSimpleMethod(**location, method,
>> methodContext);
>>          } else {
>> -            ScriptUtil.executeScript(this.**location, this.method,
>> context);
>> +            ScriptUtil.executeScript(this.**location, this.method,
>> methodContext.getEnvMap());
>>          }
>> -        // update the method environment
>> -        methodContext.putAllEnv(**context);
>> -        // always return true, error messages just go on the error list
>>          return true;
>>      }
>>
>>      @Override
>>      public String expandedString(MethodContext methodContext) {
>> -        return rawString();
>> +        return FlexibleStringExpander.**expandString(toString(),
>> methodContext.getEnvMap());
>>      }
>>
>>      @Override
>>      public String rawString() {
>> -        return "<script/>";
>> +        return toString();
>> +    }
>> +
>> +    @Override
>> +    public String toString() {
>> +        StringBuilder sb = new StringBuilder("<script ");
>> +        if (this.location != null&&  this.location.length()>  0) {
>> +            sb.append("location=\"").**append(this.location);
>> +            if (this.method != null&&  this.method.length()>  0) {
>> +                sb.append("#").append(this.**method);
>> +            }
>> +            sb.append("\" ");
>> +        }
>> +        if (this.scriptlet != null) {
>> +            sb.append("scriptlet=\"").**append(this.scriptlet).append(**"\"
>> ");
>> +        }
>> +        sb.append("/>");
>> +        return sb.toString();
>>      }
>>
>>      public static final class CallScriptFactory implements
>> Factory<CallScript>  {
>>
>>
>>

[Adrian Crum-3]

Developers should NOT change it back.

If the script location is contained in a context variable, then it is
possible for a user to change that variable (through a vulnerable web
page) to execute ANY script.

Overall, script invocations should be hard-coded. Scripts should not be
invoked from a variable - there is too much risk in doing things that way.

If you need the ability to specify a script location at run-time, then
wrap the target scripts in service calls and invoke the service via a
variable.

-Adrian


On 4/27/2012 1:21 PM, Pierre Smits wrote:

> Adrian,
>
> Could you be a bit more explaining as to why developers should change it
> back? It will help in understanding the underlying issue (the root and
> cause).
>
> Regards,
>
> Pierre
>
> Op 27 april 2012 13:34 schreef Adrian Crum<
> [hidden email]>  het volgende:
>
>> In this commit I changed the<script>  element "location" attribute from a
>> FlexibleMapAccessor to a String. In other words, I changed the attribute
>> type from expression to constant. This change was made for security
>> reasons. Developers - please don't change it back.
>>
>> -Adrian
>>
>> On 4/27/2012 12:18 PM, [hidden email] wrote:
>>
>>> Author: adrianc
>>> Date: Fri Apr 27 11:18:52 2012
>>> New Revision: 1331355
>>>
>>> URL: http://svn.apache.org/viewvc?**rev=1331355&view=rev<http://svn.apache.org/viewvc?rev=1331355&view=rev>
>>> Log:
>>> Overhauled Mini-language<script>   element. Added a "script" attribute so
>>> it can be used for small inline scripts (scriptlets).
>>>
>>> Removed code that hid script engine exceptions in the error list. If a
>>> script engine throws an exception, it should be passed to the calling
>>> process, not hidden in an error message list (which might not get checked).
>>>
>>> Modified:
>>>      ofbiz/trunk/framework/**minilang/dtd/simple-methods-**v2.xsd
>>>      ofbiz/trunk/framework/**minilang/src/org/ofbiz/**
>>> minilang/method/callops/**CallScript.java
>>>
>>> Modified: ofbiz/trunk/framework/**minilang/dtd/simple-methods-**v2.xsd
>>> URL: http://svn.apache.org/viewvc/**ofbiz/trunk/framework/**
>>> minilang/dtd/simple-methods-**v2.xsd?rev=1331355&r1=1331354&**
>>> r2=1331355&view=diff<http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/dtd/simple-methods-v2.xsd?rev=1331355&r1=1331354&r2=1331355&view=diff>
>>> ==============================**==============================**
>>> ==================
>>> --- ofbiz/trunk/framework/**minilang/dtd/simple-methods-**v2.xsd
>>> (original)
>>> +++ ofbiz/trunk/framework/**minilang/dtd/simple-methods-**v2.xsd Fri Apr
>>> 27 11:18:52 2012
>>> @@ -854,31 +854,34 @@ under the License.
>>>       <xs:element name="script" substitutionGroup="**CallOperations">
>>>           <xs:annotation>
>>>               <xs:documentation>
>>> -                Runs an external script (minilang, bsh, groovy) from the
>>> expanded location provided.
>>> +                Runs an external script or a short inline script
>>> (scriptlet).
>>>                   Error messages go on the error list and are handled with
>>> the check-errors tag.
>>>               </xs:documentation>
>>>           </xs:annotation>
>>>           <xs:complexType mixed="true">
>>> -<xs:attributeGroup ref="attlist.script"/>
>>> +<xs:attribute type="xs:string" name="location">
>>> +<xs:annotation>
>>> +<xs:documentation>
>>> +                        The script location. The location attribute
>>> accepts the component:// file location
>>> +                        protocol. Script functions/methods can be
>>> invoked by appending a hash (#) and the
>>> +                        function/method name.
>>> +&lt;br/&gt;&lt;br/&gt;
>>> +                        Required if the script attribute is empty.
>>> Attribute type: constant.
>>> +</xs:documentation>
>>> +</xs:annotation>
>>> +</xs:attribute>
>>> +<xs:attribute type="xs:string" name="script">
>>> +<xs:annotation>
>>> +<xs:documentation>
>>> +                        A short script (scriptlet). Can be used instead
>>> of a file.
>>> +                        The script must be prefixed with the script
>>> language followed by a colon (&quot;:&quot;).
>>> +&lt;br/&gt;&lt;br/&gt;
>>> +                        Required if the location attribute is empty.
>>> Attribute type: script.
>>> +</xs:documentation>
>>> +</xs:annotation>
>>> +</xs:attribute>
>>>           </xs:complexType>
>>>       </xs:element>
>>> -<xs:attributeGroup name="attlist.script">
>>> -<xs:attribute type="xs:string" name="location">
>>> -<xs:annotation>
>>> -<xs:documentation>
>>> -                    Script location (component://...)
>>> -</xs:documentation>
>>> -</xs:annotation>
>>> -</xs:attribute>
>>> -<xs:attribute type="xs:string" name="error-list-name"
>>> default="error_list">
>>> -<xs:annotation>
>>> -<xs:documentation>
>>> -                    The name of the list in the method environment to
>>> check for error messages.
>>> -                    Defaults to "error_list".
>>> -</xs:documentation>
>>> -</xs:annotation>
>>> -</xs:attribute>
>>> -</xs:attributeGroup>
>>>       <xs:element name="call-bsh" substitutionGroup="**CallOperations">
>>>           <xs:annotation>
>>>               <xs:documentation>
>>> @@ -921,8 +924,7 @@ under the License.
>>>                   Calls another simple-method in the same context as the
>>> current one.
>>>                   The called simple-method will have the same environment
>>> as the calling simple-method,
>>>                   including all environment fields, and either the event
>>> or service objects
>>> -                that the calling simple-method was called
>>> -                with.
>>> +                that the calling simple-method was called with.
>>>               </xs:documentation>
>>>           </xs:annotation>
>>>           <xs:complexType>
>>> @@ -930,7 +932,7 @@ under the License.
>>>                   <xs:element ref="result-to-field" minOccurs="0"
>>> maxOccurs="unbounded">
>>>                       <xs:annotation>
>>>                           <xs:documentation>
>>> -                            Used when memory-model=&quot;function&**quot;.
>>> Copies the called method fields
>>> +                            Used when scope=&quot;function&quot;. Copies
>>> the called method fields
>>>                               to the calling method fields.
>>>                           </xs:documentation>
>>>                       </xs:annotation>
>>>
>>> Modified: ofbiz/trunk/framework/**minilang/src/org/ofbiz/**
>>> minilang/method/callops/**CallScript.java
>>> URL: http://svn.apache.org/viewvc/**ofbiz/trunk/framework/**
>>> minilang/src/org/ofbiz/**minilang/method/callops/**
>>> CallScript.java?rev=1331355&**r1=1331354&r2=1331355&view=**diff<http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/CallScript.java?rev=1331355&r1=1331354&r2=1331355&view=diff>
>>> ==============================**==============================**
>>> ==================
>>> --- ofbiz/trunk/framework/**minilang/src/org/ofbiz/**
>>> minilang/method/callops/**CallScript.java (original)
>>> +++ ofbiz/trunk/framework/**minilang/src/org/ofbiz/**
>>> minilang/method/callops/**CallScript.java Fri Apr 27 11:18:52 2012
>>> @@ -18,84 +18,101 @@
>>>    ****************************************************************
>>> *******************/
>>>   package org.ofbiz.minilang.method.**callops;
>>>
>>> -import java.util.List;
>>> -import java.util.Map;
>>> -
>>> -import javolution.util.FastList;
>>> -
>>>   import org.ofbiz.base.util.**ScriptUtil;
>>> +import org.ofbiz.base.util.Scriptlet;
>>> +import org.ofbiz.base.util.**StringUtil;
>>> +import org.ofbiz.base.util.string.**FlexibleStringExpander;
>>>   import org.ofbiz.minilang.**MiniLangException;
>>> +import org.ofbiz.minilang.**MiniLangRuntimeException;
>>> +import org.ofbiz.minilang.**MiniLangUtil;
>>> +import org.ofbiz.minilang.**MiniLangValidate;
>>>   import org.ofbiz.minilang.**SimpleMethod;
>>> -import org.ofbiz.minilang.method.**ContextAccessor;
>>>   import org.ofbiz.minilang.method.**MethodContext;
>>>   import org.ofbiz.minilang.method.**MethodOperation;
>>>   import org.w3c.dom.Element;
>>>
>>> -public class CallScript extends MethodOperation {
>>> +/**
>>> + * Executes a script.
>>> + */
>>> +public final class CallScript extends MethodOperation {
>>>
>>>       public static final String module = CallScript.class.getName();
>>>
>>> -    private static String getScriptLocation(String combinedName) {
>>> -        int pos = combinedName.lastIndexOf("#");
>>> -        if (pos == -1) {
>>> -            return combinedName;
>>> -        }
>>> -        return combinedName.substring(0, pos);
>>> -    }
>>> -
>>> -    private static String getScriptMethodName(String combinedName) {
>>> -        int pos = combinedName.lastIndexOf("#");
>>> -        if (pos == -1) {
>>> -            return null;
>>> -        }
>>> -        return combinedName.substring(pos + 1);
>>> -    }
>>> -
>>> -    private ContextAccessor<List<Object>>   errorListAcsr;
>>> -    private String location;
>>> -    private String method;
>>> +    private final String location;
>>> +    private final String method;
>>> +    private final Scriptlet scriptlet;
>>>
>>>       public CallScript(Element element, SimpleMethod simpleMethod) throws
>>> MiniLangException {
>>>           super(element, simpleMethod);
>>> -        String scriptLocation = element.getAttribute("**location");
>>> -        this.location = getScriptLocation(**scriptLocation);
>>> -        this.method = getScriptMethodName(**scriptLocation);
>>> -        this.errorListAcsr = new ContextAccessor<List<Object>>(**
>>> element.getAttribute("error-**list-name"), "error_list");
>>> +        if (MiniLangValidate.**validationOn()) {
>>> +            MiniLangValidate.**attributeNames(simpleMethod, element,
>>> "location", "script");
>>> +            MiniLangValidate.**requireAnyAttribute(**simpleMethod,
>>> element, "location", "script");
>>> +            MiniLangValidate.**constantAttributes(**simpleMethod,
>>> element, "location");
>>> +            MiniLangValidate.**scriptAttributes(simpleMethod, element,
>>> "script");
>>> +            MiniLangValidate.**noChildElements(simpleMethod, element);
>>> +        }
>>> +        String scriptAttribute = element.getAttribute("script")**;
>>> +        if (MiniLangUtil.containsScript(**scriptAttribute)) {
>>> +            this.scriptlet = new Scriptlet(StringUtil.**
>>> convertOperatorSubstitutions(**scriptAttribute));
>>> +            this.location = null;
>>> +            this.method = null;
>>> +        } else {
>>> +            this.scriptlet = null;
>>> +            String scriptLocation = element.getAttribute("**location");
>>> +            int pos = scriptLocation.lastIndexOf("#"**);
>>> +            if (pos == -1) {
>>> +                this.location = scriptLocation;
>>> +                this.method = null;
>>> +            } else {
>>> +                this.location = scriptLocation.substring(0, pos);
>>> +                this.method = scriptLocation.substring(pos + 1);
>>> +            }
>>> +        }
>>>       }
>>>
>>>       @Override
>>>       public boolean exec(MethodContext methodContext) throws
>>> MiniLangException {
>>> -        String location = methodContext.expandString(**this.location);
>>> -        String method = methodContext.expandString(**this.method);
>>> -        List<Object>   messages = errorListAcsr.get(**methodContext);
>>> -        if (messages == null) {
>>> -            messages = FastList.newInstance();
>>> -            errorListAcsr.put(**methodContext, messages);
>>> -        }
>>> -        Map<String, Object>   context = methodContext.getEnvMap();
>>> -        if (location.endsWith(".xml")) {
>>> +        if (this.scriptlet != null) {
>>>               try {
>>> -                SimpleMethod.runSimpleMethod(**location, method,
>>> methodContext);
>>> -            } catch (MiniLangException e) {
>>> -                messages.add("Error running simple method at location ["
>>> + location + "]: " + e.getMessage());
>>> +                this.scriptlet.executeScript(**
>>> methodContext.getEnvMap());
>>> +            } catch (Exception e) {
>>> +                throw new MiniLangRuntimeException(e.**getMessage(),
>>> this);
>>>               }
>>> +            return true;
>>> +        }
>>> +        if (location.endsWith(".xml")) {
>>> +            SimpleMethod.runSimpleMethod(**location, method,
>>> methodContext);
>>>           } else {
>>> -            ScriptUtil.executeScript(this.**location, this.method,
>>> context);
>>> +            ScriptUtil.executeScript(this.**location, this.method,
>>> methodContext.getEnvMap());
>>>           }
>>> -        // update the method environment
>>> -        methodContext.putAllEnv(**context);
>>> -        // always return true, error messages just go on the error list
>>>           return true;
>>>       }
>>>
>>>       @Override
>>>       public String expandedString(MethodContext methodContext) {
>>> -        return rawString();
>>> +        return FlexibleStringExpander.**expandString(toString(),
>>> methodContext.getEnvMap());
>>>       }
>>>
>>>       @Override
>>>       public String rawString() {
>>> -        return "<script/>";
>>> +        return toString();
>>> +    }
>>> +
>>> +    @Override
>>> +    public String toString() {
>>> +        StringBuilder sb = new StringBuilder("<script ");
>>> +        if (this.location != null&&   this.location.length()>   0) {
>>> +            sb.append("location=\"").**append(this.location);
>>> +            if (this.method != null&&   this.method.length()>   0) {
>>> +                sb.append("#").append(this.**method);
>>> +            }
>>> +            sb.append("\" ");
>>> +        }
>>> +        if (this.scriptlet != null) {
>>> +            sb.append("scriptlet=\"").**append(this.scriptlet).append(**"\"
>>> ");
>>> +        }
>>> +        sb.append("/>");
>>> +        return sb.toString();
>>>       }
>>>
>>>       public static final class CallScriptFactory implements
>>> Factory<CallScript>   {
>>>
>>>
>>>

[Pierre Smits]

Adrian,

Thanks. My apologies for not stating 'NOT'.

Regards,

Pierre

Op 27 april 2012 14:34 schreef Adrian Crum <
[hidden email]> het volgende:

> Developers should NOT change it back.
>
> If the script location is contained in a context variable, then it is
> possible for a user to change that variable (through a vulnerable web page)
> to execute ANY script.
>
> Overall, script invocations should be hard-coded. Scripts should not be
> invoked from a variable - there is too much risk in doing things that way.
>
> If you need the ability to specify a script location at run-time, then
> wrap the target scripts in service calls and invoke the service via a
> variable.
>
> -Adrian
>
>
>
> On 4/27/2012 1:21 PM, Pierre Smits wrote:
>
>> Adrian,
>>
>> Could you be a bit more explaining as to why developers should change it
>> back? It will help in understanding the underlying issue (the root and
>> cause).
>>
>> Regards,
>>
>> Pierre
>>
>> Op 27 april 2012 13:34 schreef Adrian Crum<
>> adrian.crum@sandglass-**software.com <[hidden email]>>
>>  het volgende:
>>
>>  In this commit I changed the<script>  element "location" attribute from a
>>> FlexibleMapAccessor to a String. In other words, I changed the attribute
>>> type from expression to constant. This change was made for security
>>> reasons. Developers - please don't change it back.
>>>
>>> -Adrian
>>>
>>> On 4/27/2012 12:18 PM, [hidden email] wrote:
>>>
>>>  Author: adrianc
>>>> Date: Fri Apr 27 11:18:52 2012
>>>> New Revision: 1331355
>>>>
>>>> URL: http://svn.apache.org/viewvc?****rev=1331355&view=rev<http://svn.apache.org/viewvc?**rev=1331355&view=rev>
>>>> <http://**svn.apache.org/viewvc?rev=**1331355&view=rev<http://svn.apache.org/viewvc?rev=1331355&view=rev>
>>>> >
>>>>
>>>> Log:
>>>> Overhauled Mini-language<script>   element. Added a "script" attribute
>>>> so
>>>> it can be used for small inline scripts (scriptlets).
>>>>
>>>> Removed code that hid script engine exceptions in the error list. If a
>>>> script engine throws an exception, it should be passed to the calling
>>>> process, not hidden in an error message list (which might not get
>>>> checked).
>>>>
>>>> Modified:
>>>>     ofbiz/trunk/framework/****minilang/dtd/simple-methods-****v2.xsd
>>>>     ofbiz/trunk/framework/****minilang/src/org/ofbiz/**
>>>> minilang/method/callops/****CallScript.java
>>>>
>>>> Modified: ofbiz/trunk/framework/****minilang/dtd/simple-methods-****
>>>> v2.xsd
>>>> URL: http://svn.apache.org/viewvc/****ofbiz/trunk/framework/**<http://svn.apache.org/viewvc/**ofbiz/trunk/framework/**>
>>>> minilang/dtd/simple-methods-****v2.xsd?rev=1331355&r1=1331354&****
>>>> r2=1331355&view=diff<http://**svn.apache.org/viewvc/ofbiz/**
>>>> trunk/framework/minilang/dtd/**simple-methods-v2.xsd?rev=**
>>>> 1331355&r1=1331354&r2=1331355&**view=diff<http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/dtd/simple-methods-v2.xsd?rev=1331355&r1=1331354&r2=1331355&view=diff>
>>>> >
>>>> ==============================****============================**==**
>>>> ==================
>>>> --- ofbiz/trunk/framework/****minilang/dtd/simple-methods-****v2.xsd
>>>> (original)
>>>> +++ ofbiz/trunk/framework/****minilang/dtd/simple-methods-****v2.xsd
>>>> Fri Apr
>>>>
>>>> 27 11:18:52 2012
>>>> @@ -854,31 +854,34 @@ under the License.
>>>>      <xs:element name="script" substitutionGroup="****CallOperations">
>>>>
>>>>          <xs:annotation>
>>>>              <xs:documentation>
>>>> -                Runs an external script (minilang, bsh, groovy) from
>>>> the
>>>> expanded location provided.
>>>> +                Runs an external script or a short inline script
>>>> (scriptlet).
>>>>                  Error messages go on the error list and are handled
>>>> with
>>>> the check-errors tag.
>>>>              </xs:documentation>
>>>>          </xs:annotation>
>>>>          <xs:complexType mixed="true">
>>>> -<xs:attributeGroup ref="attlist.script"/>
>>>> +<xs:attribute type="xs:string" name="location">
>>>> +<xs:annotation>
>>>> +<xs:documentation>
>>>> +                        The script location. The location attribute
>>>> accepts the component:// file location
>>>> +                        protocol. Script functions/methods can be
>>>> invoked by appending a hash (#) and the
>>>> +                        function/method name.
>>>> +&lt;br/&gt;&lt;br/&gt;
>>>> +                        Required if the script attribute is empty.
>>>> Attribute type: constant.
>>>> +</xs:documentation>
>>>> +</xs:annotation>
>>>> +</xs:attribute>
>>>> +<xs:attribute type="xs:string" name="script">
>>>> +<xs:annotation>
>>>> +<xs:documentation>
>>>> +                        A short script (scriptlet). Can be used instead
>>>> of a file.
>>>> +                        The script must be prefixed with the script
>>>> language followed by a colon (&quot;:&quot;).
>>>> +&lt;br/&gt;&lt;br/&gt;
>>>> +                        Required if the location attribute is empty.
>>>> Attribute type: script.
>>>> +</xs:documentation>
>>>> +</xs:annotation>
>>>> +</xs:attribute>
>>>>          </xs:complexType>
>>>>      </xs:element>
>>>> -<xs:attributeGroup name="attlist.script">
>>>> -<xs:attribute type="xs:string" name="location">
>>>> -<xs:annotation>
>>>> -<xs:documentation>
>>>> -                    Script location (component://...)
>>>> -</xs:documentation>
>>>> -</xs:annotation>
>>>> -</xs:attribute>
>>>> -<xs:attribute type="xs:string" name="error-list-name"
>>>> default="error_list">
>>>> -<xs:annotation>
>>>> -<xs:documentation>
>>>> -                    The name of the list in the method environment to
>>>> check for error messages.
>>>> -                    Defaults to "error_list".
>>>> -</xs:documentation>
>>>> -</xs:annotation>
>>>> -</xs:attribute>
>>>> -</xs:attributeGroup>
>>>>      <xs:element name="call-bsh" substitutionGroup="****
>>>> CallOperations">
>>>>
>>>>          <xs:annotation>
>>>>              <xs:documentation>
>>>> @@ -921,8 +924,7 @@ under the License.
>>>>                  Calls another simple-method in the same context as the
>>>> current one.
>>>>                  The called simple-method will have the same environment
>>>> as the calling simple-method,
>>>>                  including all environment fields, and either the event
>>>> or service objects
>>>> -                that the calling simple-method was called
>>>> -                with.
>>>> +                that the calling simple-method was called with.
>>>>              </xs:documentation>
>>>>          </xs:annotation>
>>>>          <xs:complexType>
>>>> @@ -930,7 +932,7 @@ under the License.
>>>>                  <xs:element ref="result-to-field" minOccurs="0"
>>>> maxOccurs="unbounded">
>>>>                      <xs:annotation>
>>>>                          <xs:documentation>
>>>> -                            Used when memory-model=&quot;function&****
>>>> quot;.
>>>>
>>>> Copies the called method fields
>>>> +                            Used when scope=&quot;function&quot;.
>>>> Copies
>>>> the called method fields
>>>>                              to the calling method fields.
>>>>                          </xs:documentation>
>>>>                      </xs:annotation>
>>>>
>>>> Modified: ofbiz/trunk/framework/****minilang/src/org/ofbiz/**
>>>> minilang/method/callops/****CallScript.java
>>>> URL: http://svn.apache.org/viewvc/****ofbiz/trunk/framework/**<http://svn.apache.org/viewvc/**ofbiz/trunk/framework/**>
>>>> minilang/src/org/ofbiz/****minilang/method/callops/**
>>>> CallScript.java?rev=1331355&****r1=1331354&r2=1331355&view=****diff<
>>>> http://svn.apache.org/**viewvc/ofbiz/trunk/framework/**
>>>> minilang/src/org/ofbiz/**minilang/method/callops/**
>>>> CallScript.java?rev=1331355&**r1=1331354&r2=1331355&view=**diff<http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/CallScript.java?rev=1331355&r1=1331354&r2=1331355&view=diff>
>>>> >
>>>> ==============================****============================**==**
>>>> ==================
>>>> --- ofbiz/trunk/framework/****minilang/src/org/ofbiz/**
>>>> minilang/method/callops/****CallScript.java (original)
>>>> +++ ofbiz/trunk/framework/****minilang/src/org/ofbiz/**
>>>> minilang/method/callops/****CallScript.java Fri Apr 27 11:18:52 2012
>>>>
>>>> @@ -18,84 +18,101 @@
>>>>   ********************************************************************
>>>> *******************/
>>>>  package org.ofbiz.minilang.method.****callops;
>>>>
>>>>
>>>> -import java.util.List;
>>>> -import java.util.Map;
>>>> -
>>>> -import javolution.util.FastList;
>>>> -
>>>>  import org.ofbiz.base.util.****ScriptUtil;
>>>> +import org.ofbiz.base.util.Scriptlet;
>>>> +import org.ofbiz.base.util.****StringUtil;
>>>> +import org.ofbiz.base.util.string.****FlexibleStringExpander;
>>>>  import org.ofbiz.minilang.****MiniLangException;
>>>> +import org.ofbiz.minilang.****MiniLangRuntimeException;
>>>> +import org.ofbiz.minilang.****MiniLangUtil;
>>>> +import org.ofbiz.minilang.****MiniLangValidate;
>>>>  import org.ofbiz.minilang.****SimpleMethod;
>>>> -import org.ofbiz.minilang.method.****ContextAccessor;
>>>>  import org.ofbiz.minilang.method.****MethodContext;
>>>>  import org.ofbiz.minilang.method.****MethodOperation;
>>>>
>>>>  import org.w3c.dom.Element;
>>>>
>>>> -public class CallScript extends MethodOperation {
>>>> +/**
>>>> + * Executes a script.
>>>> + */
>>>> +public final class CallScript extends MethodOperation {
>>>>
>>>>      public static final String module = CallScript.class.getName();
>>>>
>>>> -    private static String getScriptLocation(String combinedName) {
>>>> -        int pos = combinedName.lastIndexOf("#");
>>>> -        if (pos == -1) {
>>>> -            return combinedName;
>>>> -        }
>>>> -        return combinedName.substring(0, pos);
>>>> -    }
>>>> -
>>>> -    private static String getScriptMethodName(String combinedName) {
>>>> -        int pos = combinedName.lastIndexOf("#");
>>>> -        if (pos == -1) {
>>>> -            return null;
>>>> -        }
>>>> -        return combinedName.substring(pos + 1);
>>>> -    }
>>>> -
>>>> -    private ContextAccessor<List<Object>>   errorListAcsr;
>>>> -    private String location;
>>>> -    private String method;
>>>> +    private final String location;
>>>> +    private final String method;
>>>> +    private final Scriptlet scriptlet;
>>>>
>>>>      public CallScript(Element element, SimpleMethod simpleMethod)
>>>> throws
>>>> MiniLangException {
>>>>          super(element, simpleMethod);
>>>> -        String scriptLocation = element.getAttribute("****location");
>>>> -        this.location = getScriptLocation(****scriptLocation);
>>>> -        this.method = getScriptMethodName(****scriptLocation);
>>>> -        this.errorListAcsr = new ContextAccessor<List<Object>>(****
>>>> element.getAttribute("error-****list-name"), "error_list");
>>>> +        if (MiniLangValidate.****validationOn()) {
>>>> +            MiniLangValidate.****attributeNames(simpleMethod, element,
>>>> "location", "script");
>>>> +            MiniLangValidate.****requireAnyAttribute(****simpleMethod,
>>>>
>>>> element, "location", "script");
>>>> +            MiniLangValidate.****constantAttributes(****simpleMethod,
>>>> element, "location");
>>>> +            MiniLangValidate.****scriptAttributes(simpleMethod,
>>>> element,
>>>> "script");
>>>> +            MiniLangValidate.****noChildElements(simpleMethod,
>>>> element);
>>>> +        }
>>>> +        String scriptAttribute = element.getAttribute("script")****;
>>>> +        if (MiniLangUtil.containsScript(****scriptAttribute)) {
>>>> +            this.scriptlet = new Scriptlet(StringUtil.**
>>>> convertOperatorSubstitutions(****scriptAttribute));
>>>>
>>>> +            this.location = null;
>>>> +            this.method = null;
>>>> +        } else {
>>>> +            this.scriptlet = null;
>>>> +            String scriptLocation = element.getAttribute("****
>>>> location");
>>>> +            int pos = scriptLocation.lastIndexOf("#"****);
>>>>
>>>> +            if (pos == -1) {
>>>> +                this.location = scriptLocation;
>>>> +                this.method = null;
>>>> +            } else {
>>>> +                this.location = scriptLocation.substring(0, pos);
>>>> +                this.method = scriptLocation.substring(pos + 1);
>>>> +            }
>>>> +        }
>>>>      }
>>>>
>>>>      @Override
>>>>      public boolean exec(MethodContext methodContext) throws
>>>> MiniLangException {
>>>> -        String location = methodContext.expandString(****
>>>> this.location);
>>>> -        String method = methodContext.expandString(****this.method);
>>>> -        List<Object>   messages = errorListAcsr.get(****
>>>> methodContext);
>>>>
>>>> -        if (messages == null) {
>>>> -            messages = FastList.newInstance();
>>>> -            errorListAcsr.put(****methodContext, messages);
>>>>
>>>> -        }
>>>> -        Map<String, Object>   context = methodContext.getEnvMap();
>>>> -        if (location.endsWith(".xml")) {
>>>> +        if (this.scriptlet != null) {
>>>>              try {
>>>> -                SimpleMethod.runSimpleMethod(****location, method,
>>>>
>>>> methodContext);
>>>> -            } catch (MiniLangException e) {
>>>> -                messages.add("Error running simple method at location
>>>> ["
>>>> + location + "]: " + e.getMessage());
>>>> +                this.scriptlet.executeScript(****
>>>>
>>>> methodContext.getEnvMap());
>>>> +            } catch (Exception e) {
>>>> +                throw new MiniLangRuntimeException(e.****getMessage(),
>>>>
>>>> this);
>>>>              }
>>>> +            return true;
>>>> +        }
>>>> +        if (location.endsWith(".xml")) {
>>>> +            SimpleMethod.runSimpleMethod(****location, method,
>>>> methodContext);
>>>>          } else {
>>>> -            ScriptUtil.executeScript(this.****location, this.method,
>>>> context);
>>>> +            ScriptUtil.executeScript(this.****location, this.method,
>>>>
>>>> methodContext.getEnvMap());
>>>>          }
>>>> -        // update the method environment
>>>> -        methodContext.putAllEnv(****context);
>>>>
>>>> -        // always return true, error messages just go on the error list
>>>>          return true;
>>>>      }
>>>>
>>>>      @Override
>>>>      public String expandedString(MethodContext methodContext) {
>>>> -        return rawString();
>>>> +        return FlexibleStringExpander.****expandString(toString(),
>>>>
>>>> methodContext.getEnvMap());
>>>>      }
>>>>
>>>>      @Override
>>>>      public String rawString() {
>>>> -        return "<script/>";
>>>> +        return toString();
>>>> +    }
>>>> +
>>>> +    @Override
>>>> +    public String toString() {
>>>> +        StringBuilder sb = new StringBuilder("<script ");
>>>> +        if (this.location != null&&   this.location.length()>   0) {
>>>> +            sb.append("location=\"").****append(this.location);
>>>>
>>>> +            if (this.method != null&&   this.method.length()>   0) {
>>>> +                sb.append("#").append(this.****method);
>>>>
>>>> +            }
>>>> +            sb.append("\" ");
>>>> +        }
>>>> +        if (this.scriptlet != null) {
>>>> +            sb.append("scriptlet=\"").****
>>>> append(this.scriptlet).append(****"\"
>>>>
>>>> ");
>>>> +        }
>>>> +        sb.append("/>");
>>>> +        return sb.toString();
>>>>      }
>>>>
>>>>      public static final class CallScriptFactory implements
>>>> Factory<CallScript>   {
>>>>
>>>>
>>>>
>>>>

[Scott Gray-2]

You could always add a warning comment in the code, it's likely to be more noticeable in the future than this thread will be :-)

Regards
Scott

On 27/04/2012, at 11:34 PM, Adrian Crum wrote:

> In this commit I changed the <script> element "location" attribute from a FlexibleMapAccessor to a String. In other words, I changed the attribute type from expression to constant. This change was made for security reasons. Developers - please don't change it back.
>
> -Adrian
>
> On 4/27/2012 12:18 PM, [hidden email] wrote:
>> Author: adrianc
>> Date: Fri Apr 27 11:18:52 2012
>> New Revision: 1331355
>>
>> URL: http://svn.apache.org/viewvc?rev=1331355&view=rev
>> Log:
>> Overhauled Mini-language<script>  element. Added a "script" attribute so it can be used for small inline scripts (scriptlets).
>>
>> Removed code that hid script engine exceptions in the error list. If a script engine throws an exception, it should be passed to the calling process, not hidden in an error message list (which might not get checked).
>>
>> Modified:
>>     ofbiz/trunk/framework/minilang/dtd/simple-methods-v2.xsd
>>     ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/CallScript.java
>>
>> Modified: ofbiz/trunk/framework/minilang/dtd/simple-methods-v2.xsd
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/dtd/simple-methods-v2.xsd?rev=1331355&r1=1331354&r2=1331355&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/minilang/dtd/simple-methods-v2.xsd (original)
>> +++ ofbiz/trunk/framework/minilang/dtd/simple-methods-v2.xsd Fri Apr 27 11:18:52 2012
>> @@ -854,31 +854,34 @@ under the License.
>>      <xs:element name="script" substitutionGroup="CallOperations">
>>          <xs:annotation>
>>              <xs:documentation>
>> -                Runs an external script (minilang, bsh, groovy) from the expanded location provided.
>> +                Runs an external script or a short inline script (scriptlet).
>>                  Error messages go on the error list and are handled with the check-errors tag.
>>              </xs:documentation>
>>          </xs:annotation>
>>          <xs:complexType mixed="true">
>> -<xs:attributeGroup ref="attlist.script"/>
>> +<xs:attribute type="xs:string" name="location">
>> +<xs:annotation>
>> +<xs:documentation>
>> +                        The script location. The location attribute accepts the component:// file location
>> +                        protocol. Script functions/methods can be invoked by appending a hash (#) and the
>> +                        function/method name.
>> +&lt;br/&gt;&lt;br/&gt;
>> +                        Required if the script attribute is empty. Attribute type: constant.
>> +</xs:documentation>
>> +</xs:annotation>
>> +</xs:attribute>
>> +<xs:attribute type="xs:string" name="script">
>> +<xs:annotation>
>> +<xs:documentation>
>> +                        A short script (scriptlet). Can be used instead of a file.
>> +                        The script must be prefixed with the script language followed by a colon (&quot;:&quot;).
>> +&lt;br/&gt;&lt;br/&gt;
>> +                        Required if the location attribute is empty. Attribute type: script.
>> +</xs:documentation>
>> +</xs:annotation>
>> +</xs:attribute>
>>          </xs:complexType>
>>      </xs:element>
>> -<xs:attributeGroup name="attlist.script">
>> -<xs:attribute type="xs:string" name="location">
>> -<xs:annotation>
>> -<xs:documentation>
>> -                    Script location (component://...)
>> -</xs:documentation>
>> -</xs:annotation>
>> -</xs:attribute>
>> -<xs:attribute type="xs:string" name="error-list-name" default="error_list">
>> -<xs:annotation>
>> -<xs:documentation>
>> -                    The name of the list in the method environment to check for error messages.
>> -                    Defaults to "error_list".
>> -</xs:documentation>
>> -</xs:annotation>
>> -</xs:attribute>
>> -</xs:attributeGroup>
>>      <xs:element name="call-bsh" substitutionGroup="CallOperations">
>>          <xs:annotation>
>>              <xs:documentation>
>> @@ -921,8 +924,7 @@ under the License.
>>                  Calls another simple-method in the same context as the current one.
>>                  The called simple-method will have the same environment as the calling simple-method,
>>                  including all environment fields, and either the event or service objects
>> -                that the calling simple-method was called
>> -                with.
>> +                that the calling simple-method was called with.
>>              </xs:documentation>
>>          </xs:annotation>
>>          <xs:complexType>
>> @@ -930,7 +932,7 @@ under the License.
>>                  <xs:element ref="result-to-field" minOccurs="0" maxOccurs="unbounded">
>>                      <xs:annotation>
>>                          <xs:documentation>
>> -                            Used when memory-model=&quot;function&quot;. Copies the called method fields
>> +                            Used when scope=&quot;function&quot;. Copies the called method fields
>>                              to the calling method fields.
>>                          </xs:documentation>
>>                      </xs:annotation>
>>
>> Modified: ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/CallScript.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/CallScript.java?rev=1331355&r1=1331354&r2=1331355&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/CallScript.java (original)
>> +++ ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/CallScript.java Fri Apr 27 11:18:52 2012
>> @@ -18,84 +18,101 @@
>>   *******************************************************************************/
>>  package org.ofbiz.minilang.method.callops;
>>
>> -import java.util.List;
>> -import java.util.Map;
>> -
>> -import javolution.util.FastList;
>> -
>>  import org.ofbiz.base.util.ScriptUtil;
>> +import org.ofbiz.base.util.Scriptlet;
>> +import org.ofbiz.base.util.StringUtil;
>> +import org.ofbiz.base.util.string.FlexibleStringExpander;
>>  import org.ofbiz.minilang.MiniLangException;
>> +import org.ofbiz.minilang.MiniLangRuntimeException;
>> +import org.ofbiz.minilang.MiniLangUtil;
>> +import org.ofbiz.minilang.MiniLangValidate;
>>  import org.ofbiz.minilang.SimpleMethod;
>> -import org.ofbiz.minilang.method.ContextAccessor;
>>  import org.ofbiz.minilang.method.MethodContext;
>>  import org.ofbiz.minilang.method.MethodOperation;
>>  import org.w3c.dom.Element;
>>
>> -public class CallScript extends MethodOperation {
>> +/**
>> + * Executes a script.
>> + */
>> +public final class CallScript extends MethodOperation {
>>
>>      public static final String module = CallScript.class.getName();
>>
>> -    private static String getScriptLocation(String combinedName) {
>> -        int pos = combinedName.lastIndexOf("#");
>> -        if (pos == -1) {
>> -            return combinedName;
>> -        }
>> -        return combinedName.substring(0, pos);
>> -    }
>> -
>> -    private static String getScriptMethodName(String combinedName) {
>> -        int pos = combinedName.lastIndexOf("#");
>> -        if (pos == -1) {
>> -            return null;
>> -        }
>> -        return combinedName.substring(pos + 1);
>> -    }
>> -
>> -    private ContextAccessor<List<Object>>  errorListAcsr;
>> -    private String location;
>> -    private String method;
>> +    private final String location;
>> +    private final String method;
>> +    private final Scriptlet scriptlet;
>>
>>      public CallScript(Element element, SimpleMethod simpleMethod) throws MiniLangException {
>>          super(element, simpleMethod);
>> -        String scriptLocation = element.getAttribute("location");
>> -        this.location = getScriptLocation(scriptLocation);
>> -        this.method = getScriptMethodName(scriptLocation);
>> -        this.errorListAcsr = new ContextAccessor<List<Object>>(element.getAttribute("error-list-name"), "error_list");
>> +        if (MiniLangValidate.validationOn()) {
>> +            MiniLangValidate.attributeNames(simpleMethod, element, "location", "script");
>> +            MiniLangValidate.requireAnyAttribute(simpleMethod, element, "location", "script");
>> +            MiniLangValidate.constantAttributes(simpleMethod, element, "location");
>> +            MiniLangValidate.scriptAttributes(simpleMethod, element, "script");
>> +            MiniLangValidate.noChildElements(simpleMethod, element);
>> +        }
>> +        String scriptAttribute = element.getAttribute("script");
>> +        if (MiniLangUtil.containsScript(scriptAttribute)) {
>> +            this.scriptlet = new Scriptlet(StringUtil.convertOperatorSubstitutions(scriptAttribute));
>> +            this.location = null;
>> +            this.method = null;
>> +        } else {
>> +            this.scriptlet = null;
>> +            String scriptLocation = element.getAttribute("location");
>> +            int pos = scriptLocation.lastIndexOf("#");
>> +            if (pos == -1) {
>> +                this.location = scriptLocation;
>> +                this.method = null;
>> +            } else {
>> +                this.location = scriptLocation.substring(0, pos);
>> +                this.method = scriptLocation.substring(pos + 1);
>> +            }
>> +        }
>>      }
>>
>>      @Override
>>      public boolean exec(MethodContext methodContext) throws MiniLangException {
>> -        String location = methodContext.expandString(this.location);
>> -        String method = methodContext.expandString(this.method);
>> -        List<Object>  messages = errorListAcsr.get(methodContext);
>> -        if (messages == null) {
>> -            messages = FastList.newInstance();
>> -            errorListAcsr.put(methodContext, messages);
>> -        }
>> -        Map<String, Object>  context = methodContext.getEnvMap();
>> -        if (location.endsWith(".xml")) {
>> +        if (this.scriptlet != null) {
>>              try {
>> -                SimpleMethod.runSimpleMethod(location, method, methodContext);
>> -            } catch (MiniLangException e) {
>> -                messages.add("Error running simple method at location [" + location + "]: " + e.getMessage());
>> +                this.scriptlet.executeScript(methodContext.getEnvMap());
>> +            } catch (Exception e) {
>> +                throw new MiniLangRuntimeException(e.getMessage(), this);
>>              }
>> +            return true;
>> +        }
>> +        if (location.endsWith(".xml")) {
>> +            SimpleMethod.runSimpleMethod(location, method, methodContext);
>>          } else {
>> -            ScriptUtil.executeScript(this.location, this.method, context);
>> +            ScriptUtil.executeScript(this.location, this.method, methodContext.getEnvMap());
>>          }
>> -        // update the method environment
>> -        methodContext.putAllEnv(context);
>> -        // always return true, error messages just go on the error list
>>          return true;
>>      }
>>
>>      @Override
>>      public String expandedString(MethodContext methodContext) {
>> -        return rawString();
>> +        return FlexibleStringExpander.expandString(toString(), methodContext.getEnvMap());
>>      }
>>
>>      @Override
>>      public String rawString() {
>> -        return "<script/>";
>> +        return toString();
>> +    }
>> +
>> +    @Override
>> +    public String toString() {
>> +        StringBuilder sb = new StringBuilder("<script ");
>> +        if (this.location != null&&  this.location.length()>  0) {
>> +            sb.append("location=\"").append(this.location);
>> +            if (this.method != null&&  this.method.length()>  0) {
>> +                sb.append("#").append(this.method);
>> +            }
>> +            sb.append("\" ");
>> +        }
>> +        if (this.scriptlet != null) {
>> +            sb.append("scriptlet=\"").append(this.scriptlet).append("\" ");
>> +        }
>> +        sb.append("/>");
>> +        return sb.toString();
>>      }
>>
>>      public static final class CallScriptFactory implements Factory<CallScript>  {
>>
>>

[Adrian Crum-3]

Done. Thanks!

-Adrian

On 4/27/2012 2:03 PM, Scott Gray wrote:

> You could always add a warning comment in the code, it's likely to be more noticeable in the future than this thread will be :-)
>
> Regards
> Scott
>
> On 27/04/2012, at 11:34 PM, Adrian Crum wrote:
>
>> In this commit I changed the<script>  element "location" attribute from a FlexibleMapAccessor to a String. In other words, I changed the attribute type from expression to constant. This change was made for security reasons. Developers - please don't change it back.
>>
>> -Adrian
>>
>> On 4/27/2012 12:18 PM, [hidden email] wrote:
>>> Author: adrianc
>>> Date: Fri Apr 27 11:18:52 2012
>>> New Revision: 1331355
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1331355&view=rev
>>> Log:
>>> Overhauled Mini-language<script>   element. Added a "script" attribute so it can be used for small inline scripts (scriptlets).
>>>
>>> Removed code that hid script engine exceptions in the error list. If a script engine throws an exception, it should be passed to the calling process, not hidden in an error message list (which might not get checked).
>>>
>>> Modified:
>>>      ofbiz/trunk/framework/minilang/dtd/simple-methods-v2.xsd
>>>      ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/CallScript.java
>>>
>>> Modified: ofbiz/trunk/framework/minilang/dtd/simple-methods-v2.xsd
>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/dtd/simple-methods-v2.xsd?rev=1331355&r1=1331354&r2=1331355&view=diff
>>> ==============================================================================
>>> --- ofbiz/trunk/framework/minilang/dtd/simple-methods-v2.xsd (original)
>>> +++ ofbiz/trunk/framework/minilang/dtd/simple-methods-v2.xsd Fri Apr 27 11:18:52 2012
>>> @@ -854,31 +854,34 @@ under the License.
>>>       <xs:element name="script" substitutionGroup="CallOperations">
>>>           <xs:annotation>
>>>               <xs:documentation>
>>> -                Runs an external script (minilang, bsh, groovy) from the expanded location provided.
>>> +                Runs an external script or a short inline script (scriptlet).
>>>                   Error messages go on the error list and are handled with the check-errors tag.
>>>               </xs:documentation>
>>>           </xs:annotation>
>>>           <xs:complexType mixed="true">
>>> -<xs:attributeGroup ref="attlist.script"/>
>>> +<xs:attribute type="xs:string" name="location">
>>> +<xs:annotation>
>>> +<xs:documentation>
>>> +                        The script location. The location attribute accepts the component:// file location
>>> +                        protocol. Script functions/methods can be invoked by appending a hash (#) and the
>>> +                        function/method name.
>>> +&lt;br/&gt;&lt;br/&gt;
>>> +                        Required if the script attribute is empty. Attribute type: constant.
>>> +</xs:documentation>
>>> +</xs:annotation>
>>> +</xs:attribute>
>>> +<xs:attribute type="xs:string" name="script">
>>> +<xs:annotation>
>>> +<xs:documentation>
>>> +                        A short script (scriptlet). Can be used instead of a file.
>>> +                        The script must be prefixed with the script language followed by a colon (&quot;:&quot;).
>>> +&lt;br/&gt;&lt;br/&gt;
>>> +                        Required if the location attribute is empty. Attribute type: script.
>>> +</xs:documentation>
>>> +</xs:annotation>
>>> +</xs:attribute>
>>>           </xs:complexType>
>>>       </xs:element>
>>> -<xs:attributeGroup name="attlist.script">
>>> -<xs:attribute type="xs:string" name="location">
>>> -<xs:annotation>
>>> -<xs:documentation>
>>> -                    Script location (component://...)
>>> -</xs:documentation>
>>> -</xs:annotation>
>>> -</xs:attribute>
>>> -<xs:attribute type="xs:string" name="error-list-name" default="error_list">
>>> -<xs:annotation>
>>> -<xs:documentation>
>>> -                    The name of the list in the method environment to check for error messages.
>>> -                    Defaults to "error_list".
>>> -</xs:documentation>
>>> -</xs:annotation>
>>> -</xs:attribute>
>>> -</xs:attributeGroup>
>>>       <xs:element name="call-bsh" substitutionGroup="CallOperations">
>>>           <xs:annotation>
>>>               <xs:documentation>
>>> @@ -921,8 +924,7 @@ under the License.
>>>                   Calls another simple-method in the same context as the current one.
>>>                   The called simple-method will have the same environment as the calling simple-method,
>>>                   including all environment fields, and either the event or service objects
>>> -                that the calling simple-method was called
>>> -                with.
>>> +                that the calling simple-method was called with.
>>>               </xs:documentation>
>>>           </xs:annotation>
>>>           <xs:complexType>
>>> @@ -930,7 +932,7 @@ under the License.
>>>                   <xs:element ref="result-to-field" minOccurs="0" maxOccurs="unbounded">
>>>                       <xs:annotation>
>>>                           <xs:documentation>
>>> -                            Used when memory-model=&quot;function&quot;. Copies the called method fields
>>> +                            Used when scope=&quot;function&quot;. Copies the called method fields
>>>                               to the calling method fields.
>>>                           </xs:documentation>
>>>                       </xs:annotation>
>>>
>>> Modified: ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/CallScript.java
>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/CallScript.java?rev=1331355&r1=1331354&r2=1331355&view=diff
>>> ==============================================================================
>>> --- ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/CallScript.java (original)
>>> +++ ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/CallScript.java Fri Apr 27 11:18:52 2012
>>> @@ -18,84 +18,101 @@
>>>    *******************************************************************************/
>>>   package org.ofbiz.minilang.method.callops;
>>>
>>> -import java.util.List;
>>> -import java.util.Map;
>>> -
>>> -import javolution.util.FastList;
>>> -
>>>   import org.ofbiz.base.util.ScriptUtil;
>>> +import org.ofbiz.base.util.Scriptlet;
>>> +import org.ofbiz.base.util.StringUtil;
>>> +import org.ofbiz.base.util.string.FlexibleStringExpander;
>>>   import org.ofbiz.minilang.MiniLangException;
>>> +import org.ofbiz.minilang.MiniLangRuntimeException;
>>> +import org.ofbiz.minilang.MiniLangUtil;
>>> +import org.ofbiz.minilang.MiniLangValidate;
>>>   import org.ofbiz.minilang.SimpleMethod;
>>> -import org.ofbiz.minilang.method.ContextAccessor;
>>>   import org.ofbiz.minilang.method.MethodContext;
>>>   import org.ofbiz.minilang.method.MethodOperation;
>>>   import org.w3c.dom.Element;
>>>
>>> -public class CallScript extends MethodOperation {
>>> +/**
>>> + * Executes a script.
>>> + */
>>> +public final class CallScript extends MethodOperation {
>>>
>>>       public static final String module = CallScript.class.getName();
>>>
>>> -    private static String getScriptLocation(String combinedName) {
>>> -        int pos = combinedName.lastIndexOf("#");
>>> -        if (pos == -1) {
>>> -            return combinedName;
>>> -        }
>>> -        return combinedName.substring(0, pos);
>>> -    }
>>> -
>>> -    private static String getScriptMethodName(String combinedName) {
>>> -        int pos = combinedName.lastIndexOf("#");
>>> -        if (pos == -1) {
>>> -            return null;
>>> -        }
>>> -        return combinedName.substring(pos + 1);
>>> -    }
>>> -
>>> -    private ContextAccessor<List<Object>>   errorListAcsr;
>>> -    private String location;
>>> -    private String method;
>>> +    private final String location;
>>> +    private final String method;
>>> +    private final Scriptlet scriptlet;
>>>
>>>       public CallScript(Element element, SimpleMethod simpleMethod) throws MiniLangException {
>>>           super(element, simpleMethod);
>>> -        String scriptLocation = element.getAttribute("location");
>>> -        this.location = getScriptLocation(scriptLocation);
>>> -        this.method = getScriptMethodName(scriptLocation);
>>> -        this.errorListAcsr = new ContextAccessor<List<Object>>(element.getAttribute("error-list-name"), "error_list");
>>> +        if (MiniLangValidate.validationOn()) {
>>> +            MiniLangValidate.attributeNames(simpleMethod, element, "location", "script");
>>> +            MiniLangValidate.requireAnyAttribute(simpleMethod, element, "location", "script");
>>> +            MiniLangValidate.constantAttributes(simpleMethod, element, "location");
>>> +            MiniLangValidate.scriptAttributes(simpleMethod, element, "script");
>>> +            MiniLangValidate.noChildElements(simpleMethod, element);
>>> +        }
>>> +        String scriptAttribute = element.getAttribute("script");
>>> +        if (MiniLangUtil.containsScript(scriptAttribute)) {
>>> +            this.scriptlet = new Scriptlet(StringUtil.convertOperatorSubstitutions(scriptAttribute));
>>> +            this.location = null;
>>> +            this.method = null;
>>> +        } else {
>>> +            this.scriptlet = null;
>>> +            String scriptLocation = element.getAttribute("location");
>>> +            int pos = scriptLocation.lastIndexOf("#");
>>> +            if (pos == -1) {
>>> +                this.location = scriptLocation;
>>> +                this.method = null;
>>> +            } else {
>>> +                this.location = scriptLocation.substring(0, pos);
>>> +                this.method = scriptLocation.substring(pos + 1);
>>> +            }
>>> +        }
>>>       }
>>>
>>>       @Override
>>>       public boolean exec(MethodContext methodContext) throws MiniLangException {
>>> -        String location = methodContext.expandString(this.location);
>>> -        String method = methodContext.expandString(this.method);
>>> -        List<Object>   messages = errorListAcsr.get(methodContext);
>>> -        if (messages == null) {
>>> -            messages = FastList.newInstance();
>>> -            errorListAcsr.put(methodContext, messages);
>>> -        }
>>> -        Map<String, Object>   context = methodContext.getEnvMap();
>>> -        if (location.endsWith(".xml")) {
>>> +        if (this.scriptlet != null) {
>>>               try {
>>> -                SimpleMethod.runSimpleMethod(location, method, methodContext);
>>> -            } catch (MiniLangException e) {
>>> -                messages.add("Error running simple method at location [" + location + "]: " + e.getMessage());
>>> +                this.scriptlet.executeScript(methodContext.getEnvMap());
>>> +            } catch (Exception e) {
>>> +                throw new MiniLangRuntimeException(e.getMessage(), this);
>>>               }
>>> +            return true;
>>> +        }
>>> +        if (location.endsWith(".xml")) {
>>> +            SimpleMethod.runSimpleMethod(location, method, methodContext);
>>>           } else {
>>> -            ScriptUtil.executeScript(this.location, this.method, context);
>>> +            ScriptUtil.executeScript(this.location, this.method, methodContext.getEnvMap());
>>>           }
>>> -        // update the method environment
>>> -        methodContext.putAllEnv(context);
>>> -        // always return true, error messages just go on the error list
>>>           return true;
>>>       }
>>>
>>>       @Override
>>>       public String expandedString(MethodContext methodContext) {
>>> -        return rawString();
>>> +        return FlexibleStringExpander.expandString(toString(), methodContext.getEnvMap());
>>>       }
>>>
>>>       @Override
>>>       public String rawString() {
>>> -        return "<script/>";
>>> +        return toString();
>>> +    }
>>> +
>>> +    @Override
>>> +    public String toString() {
>>> +        StringBuilder sb = new StringBuilder("<script ");
>>> +        if (this.location != null&&   this.location.length()>   0) {
>>> +            sb.append("location=\"").append(this.location);
>>> +            if (this.method != null&&   this.method.length()>   0) {
>>> +                sb.append("#").append(this.method);
>>> +            }
>>> +            sb.append("\" ");
>>> +        }
>>> +        if (this.scriptlet != null) {
>>> +            sb.append("scriptlet=\"").append(this.scriptlet).append("\" ");
>>> +        }
>>> +        sb.append("/>");
>>> +        return sb.toString();
>>>       }
>>>
>>>       public static final class CallScriptFactory implements Factory<CallScript>   {
>>>
>>>