Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,4 @@ pom.xml.versionsBackup

# integration-test (python/expect)
integration-test/telnet-stop-leak/work*/
.codex/
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}