Skip to content
Open
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 changes/en-us/2.x.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ Add changes here for all PR submitted to the 2.x branch.
- [[#7965](https://github.com/apache/incubator-seata/pull/7965)] fix the issue where different element order in two lists causes failure to set the index as the primary key
- [[#7992](https://github.com/apache/incubator-seata/pull/7992)] fix report branch transaction status without setting branch type
- [[#8035](https://github.com/apache/incubator-seata/pull/8035)] fix IllegalArgumentException when GET request has request body
- [[#8072](https://github.com/apache/incubator-seata/pull/8072)] fixes SQL Server row locks are not released in Redis mode

### optimize:

Expand Down
1 change: 1 addition & 0 deletions changes/zh-cn/2.x.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
- [[#7965](https://github.com/apache/incubator-seata/pull/7965)] 修复在 dm 和 kingbase 中当没有将索引设置为主键索引时出现的问题
- [[#7992](https://github.com/apache/incubator-seata/pull/7992)] 修复报告分支事务状态时没有设置分支类型
- [[#8035](https://github.com/apache/incubator-seata/pull/8035)] 修复GET请求有请求体的时候出现 IllegalArgumentException
- [[#8072](https://github.com/apache/incubator-seata/pull/8072)] 修复SQL Server 行锁在redis模式下不释放的问题


### optimize:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
import java.util.ArrayList;
import java.util.List;

import static org.apache.seata.common.Constants.ROW_LOCK_KEY_SPLIT_CHAR;

/**
* The type Abstract locker.
*
Expand Down Expand Up @@ -79,13 +81,18 @@ protected LockDO convertToLockDO(RowLock rowLock) {
* @return the string
*/
protected String getRowKey(String resourceId, String tableName, String pk) {
return new StringBuilder()
String rowKey = new StringBuilder()
.append(resourceId)
.append(LOCK_SPLIT)
.append(tableName)
.append(LOCK_SPLIT)
.append(pk)
.toString();

if (rowKey.indexOf(';') >= 0) {
rowKey = rowKey.replace(ROW_LOCK_KEY_SPLIT_CHAR, LOCK_SPLIT);
}
return rowKey;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,68 @@ public void updateLockStatus(String xid, LockStatus lockStatus) {}

@Test
public void testGetRowKey() {
AbstractLocker locker = new AbstractLocker() {
AbstractLocker locker = newLocker();

// Call the getRowKey method
String rowKey = locker.getRowKey("resource1", "table1", "123");

// Assert that the row key is constructed correctly
Assertions.assertEquals("resource1^^^table1^^^123", rowKey);
}

/**
* Verify that a SQL Server JDBC URL (which uses ';' as property delimiter, e.g.
* "jdbc:sqlserver://127.0.0.1:1433;databaseName=lxk") is sanitized inside the rowKey.
* The Redis storage backend joins multiple rowKeys with ROW_LOCK_KEY_SPLIT_CHAR (";"),
* so any ';' inside a single rowKey would otherwise corrupt the join/split round-trip
* and leak row locks.
*/
@Test
public void testGetRowKeyWithSqlServerResourceId() {
AbstractLocker locker = newLocker();

String rowKey = locker.getRowKey("jdbc:sqlserver://127.0.0.1:1433;databaseName=lxk", "BPM_ACT_RU_TASK", "123");

Assertions.assertFalse(
rowKey.contains(";"), "rowKey must not contain ';' to avoid collision with ROW_LOCK_KEY_SPLIT_CHAR");
Assertions.assertEquals("jdbc:sqlserver://127.0.0.1:1433^^^databaseName=lxk^^^BPM_ACT_RU_TASK^^^123", rowKey);
}

/**
* Verify the sanitization still holds when the SQL Server URL carries multiple
* properties (every ';' must be replaced).
*/
@Test
public void testGetRowKeyWithSqlServerMultiPropertyUrl() {
AbstractLocker locker = newLocker();

String rowKey = locker.getRowKey(
"jdbc:sqlserver://127.0.0.1:1433;databaseName=lxk;encrypt=false;trustServerCertificate=true",
"BPM_ACT_RU_TASK",
"123");

Assertions.assertFalse(
rowKey.contains(";"), "rowKey must not contain ';' even when resourceId carries multiple properties");
}

/**
* Verify the sanitization also covers the pk segment, which is end-user data and may
* theoretically contain any character. The mapping is still idempotent for identical
* inputs.
*/
@Test
public void testGetRowKeyWithSemicolonInPk() {
AbstractLocker locker = newLocker();

String rowKey1 = locker.getRowKey("jdbc:mysql://127.0.0.1:3306/seata", "ORDER", "1001;abnormal");
String rowKey2 = locker.getRowKey("jdbc:mysql://127.0.0.1:3306/seata", "ORDER", "1001;abnormal");

Assertions.assertFalse(rowKey1.contains(";"), "rowKey must not contain ';' even when pk carries it");
Assertions.assertEquals(rowKey1, rowKey2, "getRowKey must be idempotent for identical inputs");
}

private static AbstractLocker newLocker() {
return new AbstractLocker() {
@Override
public boolean acquireLock(List<RowLock> rowLock) {
return false;
Expand All @@ -110,12 +171,6 @@ public boolean isLockable(List<RowLock> rowLock) {
@Override
public void updateLockStatus(String xid, LockStatus lockStatus) {}
};

// Call the getRowKey method
String rowKey = locker.getRowKey("resource1", "table1", "123");

// Assert that the row key is constructed correctly
Assertions.assertEquals("resource1^^^table1^^^123", rowKey);
}

private static List<RowLock> getRowLocks() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,44 @@ public void updateLockStatus() throws TransactionException {
Assertions.assertTrue(lockManager.releaseLock(branchSession));
}

/**
* Regression test for the SQL Server row-lock leak: a SQL Server JDBC URL contains ';'
* (the same character used as the row-lock join/split delimiter on Redis), which used
* to cause release paths to issue DEL against fragmentary keys, leaving the real lock
* keys leaked in Redis. After the fix in {@code AbstractLocker#getRowKey}, releaseLock
* MUST cleanly remove every lock key written during acquireLock.
*/
@Test
public void releaseLockOfSqlServerResourceId() throws TransactionException {
BranchSession branchSession = new BranchSession();
branchSession.setXid("abc-sqlserver:1");
branchSession.setTransactionId(987654321L);
branchSession.setBranchId(123456L);
branchSession.setResourceId("jdbc:sqlserver://127.0.0.1:1433;databaseName=lxk");
branchSession.setLockKey("BPM_ACT_RU_TASK:123,456");

try {
Assertions.assertTrue(lockManager.acquireLock(branchSession));
Assertions.assertTrue(lockManager.isLockable(
branchSession.getXid(), branchSession.getResourceId(), branchSession.getLockKey()));
} finally {
Assertions.assertTrue(lockManager.releaseLock(branchSession));
}

BranchSession verifier = new BranchSession();
verifier.setXid("abc-sqlserver:2");
verifier.setTransactionId(987654322L);
verifier.setBranchId(123457L);
verifier.setResourceId("jdbc:sqlserver://127.0.0.1:1433;databaseName=lxk");
verifier.setLockKey("BPM_ACT_RU_TASK:123,456");
Assertions.assertTrue(
lockManager.acquireLock(verifier),
"Re-acquiring the same SQL Server row from a different xid must succeed; "
+ "if it fails, the previous releaseLock leaked the row lock - "
+ "this is the regression we are guarding against.");
Assertions.assertTrue(lockManager.releaseLock(verifier));
}

public static class RedisLockManagerForTest extends RedisLockManager {

public RedisLockManagerForTest() {}
Expand Down
Loading