diff --git a/.gitignore b/.gitignore index d9582c13d2a..815f5dfd322 100644 --- a/.gitignore +++ b/.gitignore @@ -31,3 +31,4 @@ pom.xml.versionsBackup # integration-test (python/expect) integration-test/telnet-stop-leak/work*/ +.codex/ diff --git a/core/src/main/java/com/taobao/arthas/core/command/express/OgnlExpress.java b/core/src/main/java/com/taobao/arthas/core/command/express/OgnlExpress.java index f29bd31d254..708c63abd20 100644 --- a/core/src/main/java/com/taobao/arthas/core/command/express/OgnlExpress.java +++ b/core/src/main/java/com/taobao/arthas/core/command/express/OgnlExpress.java @@ -34,6 +34,16 @@ public OgnlExpress(ClassResolver classResolver) { public Object get(String express) throws ExpressException { try { return Ognl.getValue(express, context, bindObject); + } catch (ognl.OgnlException e) { + // 处理 "source is null for getProperty" 异常 + // 这种情况发生在表达式尝试访问 null 对象的属性时(例如:params[2].field 其中 params[2] 为 null) + // 为了提供类似可选链的行为,返回 null 而不是抛出异常 + if (e.getMessage() != null && e.getMessage().contains("source is null for getProperty")) { + logger.debug("Null-safe property access: {}", e.getMessage()); + return null; + } + logger.error("Error during evaluating the expression:", e); + throw new ExpressException(express, e); } catch (Exception e) { logger.error("Error during evaluating the expression:", e); throw new ExpressException(express, e); diff --git a/core/src/test/java/com/taobao/arthas/core/command/express/OgnlExpressAdviceTest.java b/core/src/test/java/com/taobao/arthas/core/command/express/OgnlExpressAdviceTest.java new file mode 100644 index 00000000000..ece6939fb4f --- /dev/null +++ b/core/src/test/java/com/taobao/arthas/core/command/express/OgnlExpressAdviceTest.java @@ -0,0 +1,111 @@ +package com.taobao.arthas.core.command.express; + +import com.taobao.arthas.core.advisor.Advice; +import com.taobao.arthas.core.advisor.ArthasMethod; +import com.taobao.arthas.core.util.Constants; +import org.junit.Assert; +import org.junit.Test; + +/** + * Integration test for OGNL expression evaluation with Advice objects + * Tests the fix for issue "source is null for getProperty(null, "2")" + */ +public class OgnlExpressAdviceTest { + + @Test + public void testConditionExpressionWithNullParams() throws ExpressException { + // Simulate the scenario from the issue where params might be null or contain null elements + Express express = ExpressFactory.unpooledExpress(OgnlExpressAdviceTest.class.getClassLoader()); + + // Create Advice with null params + Advice advice = createTestAdvice(null, null, null); + + // Bind advice and test various condition expressions that would previously throw exceptions + express.bind(advice).bind(Constants.COST_VARIABLE, 100.0); + + // Test 1: Access params[2] when params is null + Object result1 = express.get("params[2]"); + Assert.assertNull("Accessing params[2] when params is null should return null", result1); + + // Test 2: Access params[2].someField when params is null + Object result2 = express.get("params[2].field"); + Assert.assertNull("Accessing params[2].field when params is null should return null", result2); + + // Test 3: Condition expression that checks if params[2] is not null + // When params is null, params[2] returns null, so we check that directly + Object paramsElement = express.get("params[2]"); + Assert.assertNull("params[2] should be null when params is null", paramsElement); + } + + @Test + public void testConditionExpressionWithNullParamElement() throws ExpressException { + // Test when params array exists but contains null element + Express express = ExpressFactory.unpooledExpress(OgnlExpressAdviceTest.class.getClassLoader()); + + Object[] params = new Object[]{new TestObject("arg0"), null, new TestObject("arg2")}; + Advice advice = createTestAdvice(null, params, null); + + express.bind(advice).bind(Constants.COST_VARIABLE, 100.0); + + // Test: Access params[1].field when params[1] is null + Object result = express.get("params[1].field"); + Assert.assertNull("Accessing params[1].field when params[1] is null should return null", result); + + // Test: Access params[0].field when params[0] is not null + Object result2 = express.get("params[0].field"); + Assert.assertEquals("Should get field value from non-null param", "arg0", result2); + } + + @Test + public void testConditionExpressionWithNullReturnObj() throws ExpressException { + // Test when returnObj is null + Express express = ExpressFactory.unpooledExpress(OgnlExpressAdviceTest.class.getClassLoader()); + + Advice advice = createTestAdvice(null, new Object[]{}, null); + + express.bind(advice).bind(Constants.COST_VARIABLE, 100.0); + + // Test: Access returnObj.field when returnObj is null + Object result = express.get("returnObj.someField"); + Assert.assertNull("Accessing returnObj.someField when returnObj is null should return null", result); + } + + @Test + public void testComplexConditionExpressionWithNullHandling() throws ExpressException { + // Test complex condition expressions that involve null checks + Express express = ExpressFactory.unpooledExpress(OgnlExpressAdviceTest.class.getClassLoader()); + + Object[] params = new Object[]{null, new TestObject("value")}; + Advice advice = createTestAdvice(null, params, null); + + express.bind(advice).bind(Constants.COST_VARIABLE, 150.0); + + // Test: Complex expression with null-safe navigation + // This previously would throw "source is null for getProperty" exception + boolean result1 = express.is("params[0] == null || params[0].field == null"); + Assert.assertTrue("Expression with null check should work", result1); + + boolean result2 = express.is("params[1] != null && params[1].field != null"); + Assert.assertTrue("Expression checking non-null param should work", result2); + + boolean result3 = express.is("#cost > 100"); + Assert.assertTrue("Cost comparison should work", result3); + } + + // Helper method to create test Advice + private Advice createTestAdvice(Object target, Object[] params, Object returnObj) { + ClassLoader loader = getClass().getClassLoader(); + Class clazz = getClass(); + ArthasMethod method = new ArthasMethod(clazz, "testMethod", "()V"); + return Advice.newForAfterReturning(loader, clazz, method, target, params, returnObj); + } + + // Helper class for testing + public static class TestObject { + public String field; + + public TestObject(String field) { + this.field = field; + } + } +} diff --git a/core/src/test/java/com/taobao/arthas/core/command/express/OgnlExpressTest.java b/core/src/test/java/com/taobao/arthas/core/command/express/OgnlExpressTest.java index 05001e21483..3120c84824a 100644 --- a/core/src/test/java/com/taobao/arthas/core/command/express/OgnlExpressTest.java +++ b/core/src/test/java/com/taobao/arthas/core/command/express/OgnlExpressTest.java @@ -50,4 +50,78 @@ public void testInvalidOgnlExpr() { Assert.assertTrue(e.getCause() instanceof ognl.ExpressionSyntaxException); } } + + @Test + public void testNullSourcePropertyAccess() throws ExpressException { + // Test accessing property on null object - should return null instead of throwing exception + Express unpooledExpress = ExpressFactory.unpooledExpress(OgnlExpressTest.class.getClassLoader()); + + // Create a test object with null field + TestObject testObj = new TestObject(); + testObj.nullField = null; + + // This should not throw "source is null for getProperty" exception + Object result = unpooledExpress.bind(testObj).get("nullField.someProperty"); + Assert.assertNull(result); + } + + @Test + public void testNullArrayAccess() throws ExpressException { + // Test accessing index on null array - should return null instead of throwing exception + Express unpooledExpress = ExpressFactory.unpooledExpress(OgnlExpressTest.class.getClassLoader()); + + // Create a test object with null array + TestObject testObj = new TestObject(); + testObj.nullArray = null; + + // This should not throw "source is null for getProperty" exception + Object result = unpooledExpress.bind(testObj).get("nullArray[2]"); + Assert.assertNull(result); + } + + @Test + public void testAdviceWithNullParams() throws ExpressException { + // Simulate the actual scenario from the issue: accessing params[2] when params is null or short + Express unpooledExpress = ExpressFactory.unpooledExpress(OgnlExpressTest.class.getClassLoader()); + + // Simulate Advice object with null params + TestAdvice advice1 = new TestAdvice(); + advice1.params = null; + + // Accessing params[2] should return null instead of throwing exception + Object result1 = unpooledExpress.bind(advice1).get("params[2]"); + Assert.assertNull(result1); + + // Simulate Advice object with params array where element is null + TestAdvice advice2 = new TestAdvice(); + advice2.params = new Object[]{null, null, null}; + + // Accessing params[2].someField should return null instead of throwing exception + Object result2 = unpooledExpress.bind(advice2).get("params[2].someField"); + Assert.assertNull(result2); + + // Test with short array + TestAdvice advice3 = new TestAdvice(); + advice3.params = new Object[]{1}; + + // Accessing params[2] on short array should work normally (OGNL handles array bounds) + try { + unpooledExpress.bind(advice3).get("params[2]"); + } catch (ExpressException e) { + // ArrayIndexOutOfBoundsException is expected and should not be suppressed + Assert.assertTrue(e.getCause() instanceof ArrayIndexOutOfBoundsException || + e.getMessage().contains("ArrayIndexOutOfBoundsException")); + } + } + + // Helper class for testing + public static class TestObject { + public Object nullField; + public Object[] nullArray; + } + + // Helper class to simulate Advice object + public static class TestAdvice { + public Object[] params; + } }