Skip to content

<fix>[ldap]: add more global properties for ldap context source #3264

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MatheMatrix wants to merge 2 commits into
base: 4.8.36
Choose a base branch
from
Open

<fix>[ldap]: add more global properties for ldap context source #3264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4451fde
<fix>[ldap]: add more global properties for ldap context source
AlanJager May 29, 2024
c455b0a
<test>[ldap]: test ldap search with different referral
AlanJager May 29, 2024
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
6 changes: 6 additions & 0 deletions plugin/ldap/src/main/java/org/zstack/ldap/LdapGlobalProperty.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,10 @@ public class LdapGlobalProperty {

@GlobalProperty(name = "Ldap.addServer.readTimeout", defaultValue = "5000")
public static int LDAP_ADD_SERVER_READ_TIMEOUT;

@GlobalProperty(name = "Ldap.referral", defaultValue = "follow")
public static String LDAP_REFERRAL;

@GlobalProperty(name = "Ldap.connect.pool", defaultValue = "false")
public static boolean LDAP_CONNECT_POOL;
}
8 changes: 6 additions & 2 deletions plugin/ldap/src/main/java/org/zstack/ldap/LdapUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.zstack.ldap;

import org.apache.commons.lang.StringUtils;
import org.apache.logging.log4j.util.Strings;
Copy link

@coderabbitai coderabbitai bot Jan 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, find and examine LdapUtil.java
fd -t f "LdapUtil.java" -x cat -n {}

Repository: MatheMatrix/zstack

Length of output: 43329

🏁 Script executed:

# Check how GlobalProperty values are loaded and processed
rg -n "class LdapGlobalProperty|LDAP_REFERRAL" --type=java -B2 -A2 | head -50

Repository: MatheMatrix/zstack

Length of output: 1723

🏁 Script executed:

# Search for GlobalProperty loading mechanism to see if trim is already applied
rg -n "GlobalProperty|loadProperties|loadConf" --type=java -g '*GlobalProperty*' | head -30

Repository: MatheMatrix/zstack

Length of output: 3874

🏁 Script executed:

# Check if StringUtils is used in the project and what variant is available
rg -n "import.*StringUtils" --type=java | head -20

Repository: MatheMatrix/zstack

Length of output: 2237

🏁 Script executed:

# Verify Strings.isEmpty behavior and usage
rg -n "Strings\\.isEmpty|Strings\\.isBlank|trimToNull" --type=java | head -30

Repository: MatheMatrix/zstack

Length of output: 1538

建议对 LDAP_REFERRAL 进行 trim 处理，防止空格导致 JNDI 初始化失败。

当前使用 Strings.isEmpty() 仅判断 null 或空字符串，无法捕获仅包含空格、换行符等不可见字符的配置值。若 LDAP_REFERRAL 被意外配置为包含空白的值（如 " "），该值会被判定为"非空"并传入 setReferral()，导致 JNDI 上下文初始化失败。应使用 StringUtils.trimToNull() 在设置前进行 trim。

♻️ 建议修改 
-import org.apache.logging.log4j.util.Strings;
...
-        if (!Strings.isEmpty(LdapGlobalProperty.LDAP_REFERRAL)) {
-            ldapContextSource.setReferral(LdapGlobalProperty.LDAP_REFERRAL);
-        }
+        String referral = StringUtils.trimToNull(LdapGlobalProperty.LDAP_REFERRAL);
+        if (referral != null) {
+            ldapContextSource.setReferral(referral);
+        }

Also applies to: 171-175

🤖 Prompt for AI Agents 
In `@plugin/ldap/src/main/java/org/zstack/ldap/LdapUtil.java` at line 4, The
LDAP_REFERRAL value is only checked with Strings.isEmpty() which doesn't catch
strings containing only whitespace; trim the input before checking and before
calling setReferral() (in LdapUtil.java) by using
StringUtils.trimToNull(ldapReferral) or equivalent, then call setReferral() only
if the trimmed value is non-null/non-empty; update both occurrences around
LDAP_REFERRAL and the code paths that call setReferral() so whitespace-only
values won't be passed to JNDI.

import org.springframework.ldap.NamingException;
import org.springframework.ldap.control.PagedResultsDirContextProcessor;
import org.springframework.ldap.core.DirContextOperations;
Expand Down Expand Up @@ -167,8 +168,11 @@ LdapContextSource buildLdapContextSource(LdapServerInventory inv, Map<String, Ob
}
}
ldapContextSource.setCacheEnvironmentProperties(false);
ldapContextSource.setPooled(false);
ldapContextSource.setReferral("follow");
ldapContextSource.setPooled(LdapGlobalProperty.LDAP_CONNECT_POOL);

if (!Strings.isEmpty(LdapGlobalProperty.LDAP_REFERRAL)) {
ldapContextSource.setReferral(LdapGlobalProperty.LDAP_REFERRAL);
}

if (baseEnvironmentProperties != null && !baseEnvironmentProperties.isEmpty()) {
ldapContextSource.setBaseEnvironmentProperties(baseEnvironmentProperties);
Expand Down
78 changes: 78 additions & 0 deletions test/src/test/groovy/org/zstack/test/unittest/ldap/TestLdapSearchCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
package org.zstack.test.unittest.ldap;

import com.unboundid.ldap.sdk.*;
import org.junit.ClassRule;
import org.junit.Test;
import org.zapodot.junit.ldap.EmbeddedLdapRule;
Comment on lines +3 to +6
Copy link

@coderabbitai coderabbitai bot Jan 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find and examine the test file
find . -path "*test/src/test/groovy/org/zstack/test/unittest/ldap/TestLdapSearchCase.java" -o -path "*TestLdapSearchCase.java" | head -20

Repository: MatheMatrix/zstack

Length of output: 138

🏁 Script executed:

#!/bin/bash
# Search for assert statements in the codebase
rg -n "assert\s+" --type java -g "*TestLdapSearchCase*" -A 1 -B 1

Repository: MatheMatrix/zstack

Length of output: 1457

🏁 Script executed:

#!/bin/bash
# Check for -ea flag in build configuration files
rg -in "(-ea|enableAssertions|enable-assertions)" -g 'pom.xml' -g '*.gradle*' -g '*.kts' -C 2

Repository: MatheMatrix/zstack

Length of output: 44

🏁 Script executed:

#!/bin/bash
# Read the test file to check imports and see all content
head -80 "test/src/test/groovy/org/zstack/test/unittest/ldap/TestLdapSearchCase.java" | cat -n

Repository: MatheMatrix/zstack

Length of output: 3749

使用 JUnit 断言替代 Java assert 语句，避免在默认配置下被忽略

多数测试运行环境不会启用 -ea 标志。当前代码中的 assert 语句在默认情况下会被禁用，导致测试失效并误通过。建议使用 Assert.assertEquals() 和 Assert.fail() 替代。

需要进行以下修改：

  1. 添加导入：import org.junit.Assert;
  2. 替换断言语句：
    • 第 46 行：assert searchResult.getEntryCount() == 6; → Assert.assertEquals(6, searchResult.getEntryCount());
    • 第 48 行：assert false : "Unexpected error during testLdapWithIgnore"; → Assert.fail("Unexpected error during testLdapWithIgnore");
    • 第 70 行：assert searchResult.getReferenceCount() == 1024; → Assert.assertEquals(1024, searchResult.getReferenceCount());
    • 第 76 行：assert false : "Unexpected here"; → Assert.fail("Unexpected here");
🤖 Prompt for AI Agents 
In `@test/src/test/groovy/org/zstack/test/unittest/ldap/TestLdapSearchCase.java`
around lines 3 - 6, Add JUnit Assert imports and replace Java assert statements
with JUnit assertions in TestLdapSearchCase: add import org.junit.Assert; then
in the method where searchResult is used (references to
searchResult.getEntryCount() and searchResult.getReferenceCount()) replace
assert searchResult.getEntryCount() == 6; with Assert.assertEquals(6,
searchResult.getEntryCount()); and replace assert
searchResult.getReferenceCount() == 1024; with Assert.assertEquals(1024,
searchResult.getReferenceCount()); also replace the two failure asserts (assert
false : "Unexpected error during testLdapWithIgnore"; and assert false :
"Unexpected here";) with Assert.fail("Unexpected error during
testLdapWithIgnore"); and Assert.fail("Unexpected here"); respectively to ensure
tests fail reliably without -ea.

import org.zapodot.junit.ldap.EmbeddedLdapRuleBuilder;

import java.util.Arrays;

public class TestLdapSearchCase {
public static String DOMAIN_DSN = "dc=example,dc=com";

static {
System.setProperty("com.unboundid.ldap.sdk.debug.enabled", "true");
System.setProperty("com.unboundid.ldap.sdk.debug.level", "FINEST");
System.setProperty("com.unboundid.ldap.sdk.LDAPConnectionOptions.followReferrals", "true");
}

@ClassRule
public static EmbeddedLdapRule embeddedLdapRule = EmbeddedLdapRuleBuilder
.newInstance()
.usingDomainDsn("dc=example,dc=com")
.importingLdifs("users-import.ldif")
.bindingToPort(10389)
.build();

@Test
public void testLdapWithIgnore() {
try (LDAPConnection ldapConnection = embeddedLdapRule.unsharedLdapConnection()) {
ldapConnection.getConnectionOptions().setFollowReferrals(false);
SearchResult searchResult = ldapConnection.search(
DOMAIN_DSN,
SearchScope.SUB,
"(objectclass=*)");

System.out.printf("Found %d entries%n", searchResult.getEntryCount());
for (SearchResultEntry entry : searchResult.getSearchEntries()) {
System.out.println(entry.getDN());
for (Attribute attribute : entry.getAttributes()) {
System.out.printf(" %s: %s%n", attribute.getName(), attribute.getValue());
}
}

System.out.println("Found referral URLs:" + Arrays.toString(searchResult.getReferralURLs()));
assert searchResult.getEntryCount() == 6;
} catch (Exception e) {
assert false : "Unexpected error during testLdapWithIgnore";
}
}

@Test
public void testLdapWithReferral() throws LDAPException {
try (LDAPConnection ldapConnection = embeddedLdapRule.unsharedLdapConnection()) {
ldapConnection.getConnectionOptions().setFollowReferrals(true);
SearchResult searchResult = ldapConnection.search(
DOMAIN_DSN,
SearchScope.SUB,
"(objectclass=*)");

System.out.printf("Found %d entries%n", searchResult.getEntryCount());
for (SearchResultEntry entry : searchResult.getSearchEntries()) {
System.out.println(entry.getDN());
for (Attribute attribute : entry.getAttributes()) {
System.out.printf(" %s: %s%n", attribute.getName(), attribute.getValue());
}
}

System.out.println("Found referral URLs:" + Arrays.toString(searchResult.getReferralURLs()));
assert searchResult.getReferenceCount() == 1024;
} catch (StackOverflowError e) {
System.out.print("stack over flow expected");
return;
}

assert false : "Unexpected here";
}
}
9 changes: 8 additions & 1 deletion test/src/test/resources/users-import.ldif
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,11 @@ objectClass: inetOrgPerson
cn: John Steinbeck
sn: Steinbeck
uid: jsteinbeck
userPassword: password
userPassword: password

dn: ou=referral test,dc=example,dc=com
objectClass: extensibleObject
objectClass: referral
objectClass: top
objectClass: person
ref: ldap://localhost:10389/dc=example,dc=com