Copilot commented on code in PR #17862:
URL: https://github.com/apache/iotdb/pull/17862#discussion_r3385413526
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/conf/SystemPropertiesUtils.java:
##########
@@ -76,10 +92,93 @@ public static void reinitializeStatics() {
/**
* Check if the ConfigNode is restarted.
*
- * @return True if confignode-system.properties file exist.
+ * @return True if both confignode-system.properties and local consensus
state exist.
*/
public static boolean isRestarted() {
- return !systemPropertiesHandler.isFirstStart();
+ return getStartupState() == StartupState.RESTART;
+ }
+
+ public static StartupState getStartupState() {
+ boolean hasSystemProperties = systemPropertiesHandler.fileExist();
+ LocalStorageState consensusState = getConsensusStorageState(new
File(conf.getConsensusDir()));
+
+ if (consensusState == LocalStorageState.UNREADABLE) {
+ return StartupState.CORRUPTED_OR_INCONSISTENT;
+ }
+
+ boolean hasConsensusState = consensusState == LocalStorageState.PRESENT;
+ if (!hasSystemProperties && !hasConsensusState) {
+ return StartupState.FIRST_START;
+ }
+ if (!hasSystemProperties) {
+ return StartupState.PARTIAL_START_CONSENSUS_ONLY;
+ }
+ if (!hasRequiredRestartSystemProperties()) {
+ return StartupState.CORRUPTED_OR_INCONSISTENT;
+ }
+ return hasConsensusState ? StartupState.RESTART :
StartupState.PARTIAL_START_SYSTEM_ONLY;
+ }
+
+ private static LocalStorageState getConsensusStorageState(File file) {
+ if (!file.exists()) {
+ return LocalStorageState.EMPTY;
+ }
+ if (file.isFile() || !file.isDirectory()) {
+ return LocalStorageState.PRESENT;
+ }
Review Comment:
`getConsensusStorageState` treats any non-directory consensus path
(including the consensus root path being a regular file or other non-directory
type) as `PRESENT`, which can misclassify a corrupted/misconfigured consensus
dir as having valid consensus state and lead to an unsafe `RESTART`
classification. It’s safer to treat non-directory (but not regular file
children) as `UNREADABLE` and surface the `CORRUPTED_OR_INCONSISTENT` startup
state.
##########
iotdb-core/ainode/iotdb/ainode/core/config.py:
##########
@@ -325,112 +317,128 @@ def _load_config_from_file(self) -> None:
conf_file
)
)
- return
-
- # noinspection PyBroadException
- try:
- file_configs = load_properties(conf_file)
-
- config_keys = file_configs.keys()
-
- if "ain_rpc_address" in config_keys:
-
self._config.set_ain_rpc_address(file_configs["ain_rpc_address"])
-
- if "ain_rpc_port" in config_keys:
-
self._config.set_ain_rpc_port(int(file_configs["ain_rpc_port"]))
-
- if "ain_inference_batch_interval_in_ms" in config_keys:
- self._config.set_ain_inference_batch_interval_in_ms(
- int(file_configs["ain_inference_batch_interval_in_ms"])
+ else:
+ # noinspection PyBroadException
+ try:
+ file_configs = load_properties(conf_file)
+
+ config_keys = file_configs.keys()
+
+ if "ain_rpc_address" in config_keys:
+
self._config.set_ain_rpc_address(file_configs["ain_rpc_address"])
+
+ if "ain_rpc_port" in config_keys:
+
self._config.set_ain_rpc_port(int(file_configs["ain_rpc_port"]))
+
+ if "ain_inference_batch_interval_in_ms" in config_keys:
+ self._config.set_ain_inference_batch_interval_in_ms(
+ int(file_configs["ain_inference_batch_interval_in_ms"])
+ )
+
+ if "ain_inference_model_mem_usage_map" in config_keys:
+ self._config.set_ain_inference_model_mem_usage_map(
+ eval(file_configs["ain_inference_model_mem_usage_map"])
+ )
Review Comment:
Using `eval(...)` on a value loaded from the AINode properties file allows
arbitrary code execution if the config file is tampered with. This should be
parsed using a safe parser such as `ast.literal_eval` (or JSON) instead of
`eval`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
