Index: ../xwiki-commons/xwiki-commons-core/xwiki-commons-velocity/src/main/java/org/xwiki/velocity/internal/DefaultVelocityConfiguration.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- ../xwiki-commons/xwiki-commons-core/xwiki-commons-velocity/src/main/java/org/xwiki/velocity/internal/DefaultVelocityConfiguration.java (revision b4a1e19f72e6a922f1ba6adedf59a7de667307ad) +++ ../xwiki-commons/xwiki-commons-core/xwiki-commons-velocity/src/main/java/org/xwiki/velocity/internal/DefaultVelocityConfiguration.java (revision ) @@ -38,6 +38,7 @@ import org.xwiki.velocity.internal.util.RestrictParseLocationEventHandler; import org.xwiki.velocity.introspection.ChainingUberspector; import org.xwiki.velocity.introspection.DeprecatedCheckUberspector; +import org.xwiki.velocity.introspection.ExceptionCatchingUberspector; import org.xwiki.velocity.introspection.MethodArgumentsUberspector; import org.xwiki.velocity.introspection.SecureUberspector; import org.xwiki.velocity.tools.CollectionsTool; @@ -51,7 +52,7 @@ /** * All configuration options for the Velocity subsystem. * - * @version $Id$ + * @version $Id: aed2b968b54b666d3ea4974b5d65b7c4979d67d4 $ * @since 2.0M1 */ @Component @@ -110,7 +111,7 @@ this.defaultProperties.setProperty("runtime.introspector.uberspect", ChainingUberspector.class.getName()); this.defaultProperties.setProperty("runtime.introspector.uberspect.chainClasses", StringUtils.join( new String[] { SecureUberspector.class.getName(), DeprecatedCheckUberspector.class.getName(), - MethodArgumentsUberspector.class.getName() }, ',')); + MethodArgumentsUberspector.class.getName(), ExceptionCatchingUberspector.class.getName() }, ',')); // Enable the extra scope variables $template and $macro, similar to $foreach this.defaultProperties.setProperty("template.provide.scope.control", Boolean.TRUE.toString()); this.defaultProperties.setProperty("macro.provide.scope.control", Boolean.TRUE.toString()); Index: ../xwiki-commons/xwiki-commons-core/xwiki-commons-velocity/src/test/java/org/xwiki/velocity/introspection/ExceptionCatchingUberspectorTest.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- ../xwiki-commons/xwiki-commons-core/xwiki-commons-velocity/src/test/java/org/xwiki/velocity/introspection/ExceptionCatchingUberspectorTest.java (revision ) +++ ../xwiki-commons/xwiki-commons-core/xwiki-commons-velocity/src/test/java/org/xwiki/velocity/introspection/ExceptionCatchingUberspectorTest.java (revision ) @@ -0,0 +1,146 @@ +/* + * See the NOTICE file distributed with this work for additional + * information regarding copyright ownership. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This software is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this software; if not, write to the Free + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA, or see the FSF site: http://www.fsf.org. + */ +package org.xwiki.velocity.introspection; + +import java.io.StringReader; +import java.io.StringWriter; +import java.util.Properties; + +import org.apache.commons.lang3.exception.ExceptionUtils; +import org.apache.velocity.VelocityContext; +import org.junit.Rule; +import org.junit.Test; +import org.xwiki.context.Execution; +import org.xwiki.context.ExecutionContext; +import org.xwiki.test.annotation.BeforeComponent; +import org.xwiki.test.annotation.ComponentList; +import org.xwiki.test.mockito.MockitoComponentManagerRule; +import org.xwiki.text.StringUtils; +import org.xwiki.velocity.VelocityEngine; +import org.xwiki.velocity.internal.DefaultVelocityConfiguration; +import org.xwiki.velocity.internal.DefaultVelocityContextFactory; +import org.xwiki.velocity.internal.DefaultVelocityEngine; +import org.xwiki.velocity.internal.VelocityExecutionContextInitializer; + +import static org.junit.Assert.*; +import static org.mockito.Mockito.when; + +/** + * Unit tests for {@link ExceptionCatchingUberspector}. + * + * @version $Id$ + * @since 8.0M1 + */ +@ComponentList({ + DefaultVelocityEngine.class, + DefaultVelocityContextFactory.class, + DefaultVelocityConfiguration.class +}) +public class ExceptionCatchingUberspectorTest +{ + @Rule + public MockitoComponentManagerRule componentManager = new MockitoComponentManagerRule(); + + private VelocityEngine velocityEngine; + + private VelocityContext vcontext; + + public class ExceptionThrowingClass + { + public void throwException1() throws Exception + { + throw new Exception("exception1"); + } + + public void throwException2() throws Exception + { + throw new Exception("exception2"); + } + } + + @BeforeComponent + public void setUpComponents() throws Exception + { + this.componentManager.registerMemoryConfigurationSource(); + } + + public void setUp(boolean withExecutionComponent) throws Exception + { + this.velocityEngine = this.componentManager.getInstance(VelocityEngine.class); + Properties properties = new Properties(); + // Override default value to not execute all other uberspectors defined in the default configuration + properties.setProperty("runtime.introspector.uberspect.chainClasses", StringUtils.join( + new String[] { SecureUberspector.class.getName(), ExceptionCatchingUberspector.class.getName() }, ',')); + + this.vcontext = new VelocityContext(); + this.vcontext.put("var", new ExceptionThrowingClass()); + + if (withExecutionComponent) { + Execution execution = this.componentManager.registerMockComponent(Execution.class); + ExecutionContext ec = new ExecutionContext(); + ec.setProperty(VelocityExecutionContextInitializer.VELOCITY_CONTEXT_ID, this.vcontext); + when(execution.getContext()).thenReturn(ec); + } + + this.velocityEngine.initialize(properties); + } + + @Test + public void catchExceptionOk() throws Exception + { + setUp(true); + + StringWriter writer = new StringWriter(); + this.velocityEngine.evaluate(this.vcontext, writer, "template", new StringReader( + "#set ($discard = $var.throwException1())" + + "${error.isHandled()}-${error.exception.message}-${error.isHandled()}")); + assertEquals("false-exception1-true", writer.toString()); + } + + /** + * Verify that the Exception Catching Uberspector doesn't interfere with the #try() directive and that with the + * following example, an exception is raised for the {@code throwException1()} call (i.e. that + * {@code throwException2()} is not called. + *
+     *   #try()
+     *     $var.throwException1()
+     *     $var.throwException2()
+     *   #end
+     *   $exception.message
+     * 
+ */ + @Test + public void catchExceptionWhenInTryDirectiveAndExceptionNotHandled() throws Exception + { + setUp(true); + + StringWriter writer = new StringWriter(); + this.velocityEngine.evaluate(this.vcontext, writer, "template", + new StringReader("#try()\n" + + "#set ($discard = $var.throwException1())\n" + + "$error vvvv $error.exception.message" + + "#set ($discard = $var.throwException2())\n" + + "#end\n" + + "$exception.message")); +// this.vcontext.put("exceptiontool", ExceptionUtils.class.getName()); +//TODO: show full stack trace and have a message like: "previous call has thrown an exception, not executing current call to..." + assertEquals("exception1", writer.toString()); + } +} Index: ../xwiki-commons/xwiki-commons-core/xwiki-commons-velocity/src/test/java/org/xwiki/velocity/internal/DefaultVelocityConfigurationTest.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- ../xwiki-commons/xwiki-commons-core/xwiki-commons-velocity/src/test/java/org/xwiki/velocity/internal/DefaultVelocityConfigurationTest.java (revision b4a1e19f72e6a922f1ba6adedf59a7de667307ad) +++ ../xwiki-commons/xwiki-commons-core/xwiki-commons-velocity/src/test/java/org/xwiki/velocity/internal/DefaultVelocityConfigurationTest.java (revision ) @@ -30,6 +30,7 @@ import org.xwiki.test.mockito.MockitoComponentMockingRule; import org.xwiki.velocity.introspection.ChainingUberspector; import org.xwiki.velocity.introspection.DeprecatedCheckUberspector; +import org.xwiki.velocity.introspection.ExceptionCatchingUberspector; import org.xwiki.velocity.introspection.MethodArgumentsUberspector; import org.xwiki.velocity.introspection.SecureUberspector; @@ -39,7 +40,7 @@ /** * Unit tests for {@link DefaultVelocityConfiguration}. * - * @version $Id$ + * @version $Id: e7ee450878bf670295c338ab8754f30cfcbcddfe $ * @since 2.4RC1 */ public class DefaultVelocityConfigurationTest @@ -70,7 +71,8 @@ assertEquals(ChainingUberspector.class.getName(), this.mocker.getComponentUnderTest().getProperties().getProperty("runtime.introspector.uberspect")); assertEquals(StringUtils.join(new String[] { SecureUberspector.class.getName(), - DeprecatedCheckUberspector.class.getName(), MethodArgumentsUberspector.class.getName() }, ','), + DeprecatedCheckUberspector.class.getName(), MethodArgumentsUberspector.class.getName(), + ExceptionCatchingUberspector.class.getName() }, ','), this.mocker.getComponentUnderTest().getProperties().getProperty( "runtime.introspector.uberspect.chainClasses")); Index: ../xwiki-commons/xwiki-commons-core/xwiki-commons-velocity/src/main/java/org/xwiki/velocity/introspection/ExceptionCatchingUberspector.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- ../xwiki-commons/xwiki-commons-core/xwiki-commons-velocity/src/main/java/org/xwiki/velocity/introspection/ExceptionCatchingUberspector.java (revision ) +++ ../xwiki-commons/xwiki-commons-core/xwiki-commons-velocity/src/main/java/org/xwiki/velocity/introspection/ExceptionCatchingUberspector.java (revision ) @@ -0,0 +1,172 @@ +/* + * See the NOTICE file distributed with this work for additional + * information regarding copyright ownership. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This software is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this software; if not, write to the Free + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA, or see the FSF site: http://www.fsf.org. + */ +package org.xwiki.velocity.introspection; + +import java.lang.reflect.InvocationTargetException; + +import org.apache.velocity.VelocityContext; +import org.apache.velocity.runtime.RuntimeServices; +import org.apache.velocity.runtime.log.Log; +import org.apache.velocity.util.RuntimeServicesAware; +import org.apache.velocity.util.introspection.Info; +import org.apache.velocity.util.introspection.VelMethod; +import org.xwiki.component.manager.ComponentLookupException; +import org.xwiki.component.manager.ComponentManager; +import org.xwiki.context.Execution; +import org.xwiki.velocity.internal.VelocityExecutionContextInitializer; + +/** + * Chainable Velocity Uberspector that catches Exception and make them available through a {@code $lastException} + * Context binding. + * + * @since 8.0M1 + * @version $Id$ + * @see ChainableUberspector + */ +public class ExceptionCatchingUberspector extends AbstractChainableUberspector implements RuntimeServicesAware +{ + /** + * The key under which the {@link ExceptionCatchingError} object will be put in the Velocity Context. + */ + private static final String VELOCITY_ERROR_BINDING = "error"; + + private Execution execution; + + private boolean isDisabled; + + @Override + public void setRuntimeServices(RuntimeServices runtimeServices) + { + ComponentManager componentManager = + (ComponentManager) runtimeServices.getApplicationAttribute(ComponentManager.class.getName()); + try { + this.execution = componentManager.getInstance(Execution.class); + } catch (ComponentLookupException e) { + // There's no component implementation in the system. We continue and disable exception catching. + // Note: Right now this is done because we don't provide an implementation of VelocityManager in commons. + // See the comment inside the DefaultVelocityManager class in xwiki-platform. + this.log.error(String.format( + "No implementation of [%s] found. Automatic exception-catching in Velocity is disabled.", + Execution.class.getName())); + isDisabled = true; + } + } + + @Override + public VelMethod getMethod(Object obj, String methodName, Object[] args, Info i) throws Exception + { + VelMethod result = super.getMethod(obj, methodName, args, i); + if (result != null && !this.isDisabled) { + result = new ExceptionCatchingVelMethod(result, this.execution, this.log); + } + return result; + } + + /** + * Wrapper for a real VelMethod that catches Exceptions. + * + * @version $Id$ + */ + private class ExceptionCatchingVelMethod implements VelMethod + { + /** The real method that performs the actual call. */ + private VelMethod innerMethod; + + private Execution execution; + + private Log log; + + /** + * Constructor. + * + * @param realMethod the real method to wrap + */ + ExceptionCatchingVelMethod(VelMethod realMethod, Execution execution, Log log) + { + this.innerMethod = realMethod; + this.execution = execution; + this.log = log; + } + + @Override + public Object invoke(Object o, Object[] params) throws Exception + { + // If there's a previously unhandled exception then raise it now! + VelocityContext vcontext = (VelocityContext) this.execution.getContext().getProperty( + VelocityExecutionContextInitializer.VELOCITY_CONTEXT_ID); + if (vcontext != null) { + ExceptionCatchingError error = (ExceptionCatchingError) vcontext.get(VELOCITY_ERROR_BINDING); + // Note: Don't raise an unhandled exception if we're calling any method from $error + // (isHandled(), getException(), etc) + if (error != null && !error.isHandled() && !(o instanceof ExceptionCatchingError)) { + throw new RuntimeException(String.format("Previous Velocity call has thrown an exception, " + + "not executing current call to [%s]", this.innerMethod.getMethodName()), + error.getException()); + } + } + + try { + // Call the method! + return this.innerMethod.invoke(o, params); + } catch (Exception e) { + // Note that the exception thrown by the script API is wrapped into an InvocationTargetException + // since Velocity calls the script API using reflection! Also note that if we store this exception + // as is in the Velocity Context then the script won't be able to access it since it would use + // something like "$error.exception.getMessage()" and since "$error.exception" would be a class located + // in the java.lang.reflect package, the Secure Uberspector would prevent the script from accessing it! + Exception targetException = e; + if (e instanceof InvocationTargetException) { + Throwable targetThrowable = ((InvocationTargetException) e).getTargetException(); + if (targetThrowable instanceof Exception) { + targetException = (Exception) targetThrowable; + } + } + + if (vcontext != null) { + vcontext.put(VELOCITY_ERROR_BINDING, new ExceptionCatchingError(targetException)); + } else { + this.log.error(String.format("No Velocity Context found in the Execution Context. The exception " + + "raised by the last script API call ([%s]) couldn't be saved in the Velocity Context and " + + "has been raised instead.", this.innerMethod.getMethodName())); + throw targetException; + } + return null; + } + } + + @Override + public boolean isCacheable() + { + return this.innerMethod.isCacheable(); + } + + @Override + public String getMethodName() + { + return this.innerMethod.getMethodName(); + } + + @Override + public Class getReturnType() + { + return this.innerMethod.getReturnType(); + } + } +} Index: ../xwiki-commons/xwiki-commons-core/xwiki-commons-velocity/src/main/java/org/xwiki/velocity/introspection/ExceptionCatchingError.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- ../xwiki-commons/xwiki-commons-core/xwiki-commons-velocity/src/main/java/org/xwiki/velocity/introspection/ExceptionCatchingError.java (revision ) +++ ../xwiki-commons/xwiki-commons-core/xwiki-commons-velocity/src/main/java/org/xwiki/velocity/introspection/ExceptionCatchingError.java (revision ) @@ -0,0 +1,43 @@ +/* + * See the NOTICE file distributed with this work for additional + * information regarding copyright ownership. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This software is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this software; if not, write to the Free + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA, or see the FSF site: http://www.fsf.org. + */ +package org.xwiki.velocity.introspection; + +public class ExceptionCatchingError +{ + private Exception exception; + + private boolean isHandled; + + public ExceptionCatchingError(Exception exception) + { + this.exception = exception; + } + + public Exception getException() + { + this.isHandled = true; + return this.exception; + } + + public boolean isHandled() + { + return this.isHandled; + } +}