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
13 changes: 13 additions & 0 deletions conf/app_config.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
Bootstrap configuration for ZStack
This file determines which properties file to load at startup

The propertiesFile value can be replaced by build script for OEM customization:
- Default: zstack.properties
- Custom: will be replaced by Ant during build (e.g., myapp.properties)
-->
<bootstrap>
<!-- BUILD_REPLACE_MARKER: Do not modify manually, will be replaced during build -->
<propertiesFile>zstack.properties</propertiesFile>
</bootstrap>
5 changes: 4 additions & 1 deletion conf/zstack.xml
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,12 @@
<property name="systemPropertiesModeName" value="SYSTEM_PROPERTIES_MODE_OVERRIDE"/>
<property name="locations">
<list>
<value>classpath:zstack.properties</value>
<!-- Properties file name is dynamically determined by app_config.xml -->
<!-- Platform.java sets the system property 'app.properties.file' at startup -->
<value>classpath:${app.properties.file}</value>
</list>
</property>
<property name="ignoreResourceNotFound" value="true"/>
<property name="ignoreUnresolvablePlaceholders" value="true"/>
</bean>

Expand Down
3 changes: 2 additions & 1 deletion conf/zstackSimulator.xml
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@
<property name="systemPropertiesModeName" value="SYSTEM_PROPERTIES_MODE_OVERRIDE"/>
<property name="locations">
<list>
<value>classpath:zstack.properties</value>
<value>classpath:${app.properties.file}</value>
</list>
</property>
<property name="ignoreResourceNotFound" value="true"/>
<property name="ignoreUnresolvablePlaceholders" value="true" />
</bean>

Expand Down
14 changes: 13 additions & 1 deletion core/src/main/java/org/zstack/core/Platform.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.zstack.core.statemachine.StateMachine;
import org.zstack.core.statemachine.StateMachineImpl;
import org.zstack.core.thread.ThreadFacade;
import org.zstack.core.config.AppConfig;
import org.zstack.header.Component;
import org.zstack.header.core.StaticInit;
import org.zstack.header.core.encrypt.ENCRYPT;
Expand Down Expand Up @@ -52,6 +53,10 @@
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.net.Inet4Address;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import org.w3c.dom.Document;
import org.w3c.dom.NodeList;
import java.net.InetAddress;
import java.net.NetworkInterface;
import java.net.SocketException;
Expand Down Expand Up @@ -488,7 +493,14 @@ private static void prepareHibernateSearchProperties() {
}
}

File globalPropertiesFile = PathUtil.findFileOnClassPath("zstack.properties", true);
// Load properties file name from app configuration
// This allows OEM customization by replacing app_config.xml during build
String propertiesFileName = AppConfig.getPropertiesFileName();

// Set system property for Spring to use the same file
System.setProperty("app.properties.file", propertiesFileName);

File globalPropertiesFile = PathUtil.findFileOnClassPath(propertiesFileName, true);

in = new FileInputStream(globalPropertiesFile);
System.getProperties().load(in);
Expand Down
68 changes: 68 additions & 0 deletions core/src/main/java/org/zstack/core/config/AppConfig.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package org.zstack.core.config;

import org.w3c.dom.Document;
import org.w3c.dom.NodeList;
import org.zstack.utils.path.PathUtil;

import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import java.io.File;

/**
* Centralized configuration reader for app_config.xml
* This class provides a single source of truth for the properties file name
* All other components should use this class instead of hardcoding "zstack.properties"
*/
public class AppConfig {
private static final String DEFAULT_PROPERTIES_FILE = "zstack.properties";
private static volatile String propertiesFileName = null;

/**
* Get the properties file name from app_config.xml
* This method is thread-safe and caches the result
*
* @return properties file name (e.g., "zstack.properties", "myapp.properties")
*/
public static String getPropertiesFileName() {
if (propertiesFileName == null) {
synchronized (AppConfig.class) {
if (propertiesFileName == null) {
propertiesFileName = loadPropertiesFileNameFromConfig();
}
}
}
return propertiesFileName;
}

/**
* Load properties file name from app_config.xml
* Falls back to "zstack.properties" if app_config.xml is not found or cannot be parsed
*/
private static String loadPropertiesFileNameFromConfig() {
try {
File appConfigFile = PathUtil.findFileOnClassPath("app_config.xml", true);
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
DocumentBuilder builder = factory.newDocumentBuilder();
Document doc = builder.parse(appConfigFile);
Comment on lines +44 to +46
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

潜在的 XXE(XML 外部实体注入)漏洞

DocumentBuilderFactory 在解析 XML 时未禁用外部实体处理,可能存在 XXE 漏洞风险。虽然 app_config.xml 是内部配置文件,但建议遵循安全最佳实践。

🔎 建议添加 XXE 防护
         try {
             File appConfigFile = PathUtil.findFileOnClassPath("app_config.xml", true);
             DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
+            // Disable XXE
+            factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+            factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
+            factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
             DocumentBuilder builder = factory.newDocumentBuilder();
             Document doc = builder.parse(appConfigFile);
🤖 Prompt for AI Agents
In @core/src/main/java/org/zstack/core/config/AppConfig.java around lines 44 -
46, AppConfig currently creates a DocumentBuilderFactory and parses
appConfigFile without disabling XML external entity handling; update the
DocumentBuilderFactory configuration before calling factory.newDocumentBuilder()
to mitigate XXE: enable XMLConstants.FEATURE_SECURE_PROCESSING, set
"http://apache.org/xml/features/disallow-doctype-decl" to true, set
"http://xml.org/sax/features/external-general-entities" and
"http://xml.org/sax/features/external-parameter-entities" to false, set
XIncludeAware to false and setAttribute for
"http://javax.xml.XMLConstants/property/accessExternalDTD" and
"http://javax.xml.XMLConstants/property/accessExternalSchema" to an empty
string; then create the DocumentBuilder and call builder.parse(appConfigFile) as
before.


NodeList nodes = doc.getElementsByTagName("propertiesFile");
if (nodes.getLength() > 0) {
String fileName = nodes.item(0).getTextContent().trim();
System.out.println("[AppConfig] Using properties file: " + fileName);
return fileName;
}
Comment on lines +48 to +53
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

缺少空字符串校验

propertiesFile 节点存在但内容为空时,trim() 后会返回空字符串,这可能导致后续加载属性文件失败。

🔎 建议添加空字符串校验
             NodeList nodes = doc.getElementsByTagName("propertiesFile");
             if (nodes.getLength() > 0) {
                 String fileName = nodes.item(0).getTextContent().trim();
-                System.out.println("[AppConfig] Using properties file: " + fileName);
-                return fileName;
+                if (!fileName.isEmpty()) {
+                    System.out.println("[AppConfig] Using properties file: " + fileName);
+                    return fileName;
+                }
             }
🤖 Prompt for AI Agents
In @core/src/main/java/org/zstack/core/config/AppConfig.java around lines 48 -
53, The code in AppConfig that reads the propertiesFile node assigns fileName =
nodes.item(0).getTextContent().trim() without checking for an empty string; add
a check after trimming to verify fileName.isEmpty() (or length()==0) and handle
it (e.g., skip returning it and continue searching or log a warning and return
null) instead of proceeding to use an empty filename; update the block around
nodes, fileName and the System.out.println call to only print/return when
fileName is non-empty.

} catch (Exception e) {
System.err.println("[AppConfig] Failed to load app_config.xml, using default: " + DEFAULT_PROPERTIES_FILE);
e.printStackTrace();
}

return DEFAULT_PROPERTIES_FILE;
}
Comment on lines +41 to +60
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

代码重复:loadPropertiesFileNameFromConfig()Platform.loadPropertiesFileNameFromBootstrap() 逻辑几乎完全一致

Platform.java 中的 loadPropertiesFileNameFromBootstrap() 方法(Lines 285-303)与此方法功能重复。既然已经创建了 AppConfig 作为集中配置读取器,Platform.java 应该直接调用 AppConfig.getPropertiesFileName() 而不是再实现一遍相同的逻辑。

当前 Platform.java 已经在 static block 中调用了 AppConfig.getPropertiesFileName(),但仍保留了 loadPropertiesFileNameFromBootstrap() 方法,这是死代码。

🔎 建议:移除 Platform.java 中的冗余方法

Platform.java 中删除 loadPropertiesFileNameFromBootstrap() 方法(Lines 280-303),因为该功能已由 AppConfig 提供。

🤖 Prompt for AI Agents
In @core/src/main/java/org/zstack/core/config/AppConfig.java around lines 41 -
60, Remove the redundant bootstrap loader by deleting
Platform.loadPropertiesFileNameFromBootstrap() and any calls to it, and ensure
Platform's static initialization uses AppConfig.getPropertiesFileName()
exclusively; specifically, keep AppConfig.loadPropertiesFileNameFromConfig()
(invoked via AppConfig.getPropertiesFileName()), remove the duplicate method in
Platform, and update Platform's static block to reference
AppConfig.getPropertiesFileName() if it does not already, eliminating dead code.


/**
* Reset cached value (mainly for testing)
*/
public static void reset() {
propertiesFileName = null;
}
}
3 changes: 2 additions & 1 deletion test/src/test/java/org/zstack/test/DBUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import org.apache.commons.io.FileUtils;
import org.zstack.core.Platform;
import org.zstack.core.config.AppConfig;
import org.zstack.header.exception.CloudRuntimeException;
import org.zstack.utils.ShellResult;
import org.zstack.utils.ShellUtils;
Expand All @@ -28,7 +29,7 @@ public static void reDeployDB() {
Properties prop = new Properties();

try {
prop.load(DBUtil.class.getClassLoader().getResourceAsStream("zstack.properties"));
prop.load(DBUtil.class.getClassLoader().getResourceAsStream(AppConfig.getPropertiesFileName()));

String user = System.getProperty("DB.user");
if (user == null) {
Expand Down
3 changes: 2 additions & 1 deletion test/src/test/resources/zstack-template.xml
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@
<property name="systemPropertiesModeName" value="SYSTEM_PROPERTIES_MODE_OVERRIDE"/>
<property name="locations">
<list>
<value>classpath:zstack.properties</value>
<value>classpath:${app.properties.file}</value>
</list>
</property>
<property name="ignoreResourceNotFound" value="true"/>
<property name="ignoreUnresolvablePlaceholders" value="true" />
</bean>

Expand Down