imbajin commented on code in PR #2921:
URL: 
https://github.com/apache/incubator-hugegraph/pull/2921#discussion_r2659462956


##########
hugegraph-cluster-test/hugegraph-clustertest-dist/src/assembly/static/conf/rest-server.properties.template:
##########
@@ -36,7 +36,12 @@ arthas.disabled_commands=jad
 # authentication configs
 # choose 'org.apache.hugegraph.auth.StandardAuthenticator' or
 # 'org.apache.hugegraph.auth.ConfigAuthenticator'
-#auth.authenticator=
+auth.authenticator=org.apache.hugegraph.auth.StandardAuthenticator
+# true if using hstore mode
+usePD=true
+
+# default password
+auth.admin_pa=pa

Review Comment:
   **‼️ Critical: Hardcoded credentials in configuration**
   
   The default password 'pa' is hardcoded in the template configuration. This 
is a security vulnerability that should not be committed to version control.
   
   **Recommendation:**
   - Remove the hardcoded password
   - Use environment variables or secure credential management
   - Document password setup in deployment guide instead



##########
hugegraph-cluster-test/hugegraph-clustertest-minicluster/src/main/java/org/apache/hugegraph/ct/node/ServerNodeWrapper.java:
##########
@@ -67,6 +73,36 @@ private static void addJarsToClasspath(File directory, 
List<String> classpath) {
         }
     }
 
+    private static void addCpJarsToClasspath(File directory, List<String> 
classpath) {
+        // Add jar starts with hugegraph in proper order
+        String path = directory.getAbsolutePath();
+        for (String jar : hgJars) {
+            classpath.add(path + File.separator + jar);
+        }
+        if (directory.exists() && directory.isDirectory()) {
+            File[] files =
+                    directory.listFiles((dir, name) -> name.endsWith(".jar") 
&& !name.contains(
+                            "hugegraph"));
+            if (files != null) {
+                for (File file : files) {
+                    classpath.add(file.getAbsolutePath());
+                }
+            }
+        }
+    }
+

Review Comment:
   **⚠️ Potential issue: Missing null check**
   
   The InputStream 'is' could be null if the resource '/jar.txt' doesn't exist. 
This will cause a NullPointerException when creating the BufferedReader.
   
   **Suggestion:**
   ```java
   InputStream is = ServerNodeWrapper.class.getResourceAsStream("/jar.txt");
   if (is == null) {
       throw new IllegalStateException("Required resource /jar.txt not found");
   }
   try (BufferedReader reader = new BufferedReader(new InputStreamReader(is, 
StandardCharsets.UTF_8))) {
       String line;
       while ((line = reader.readLine()) != null) {
           hgJars.add(line);
       }
   } catch (IOException e) {
       throw new RuntimeException("Failed to load jar list", e);
   }
   ```



##########
hugegraph-cluster-test/hugegraph-clustertest-minicluster/src/main/java/org/apache/hugegraph/ct/node/ServerNodeWrapper.java:
##########
@@ -67,6 +73,36 @@ private static void addJarsToClasspath(File directory, 
List<String> classpath) {
         }
     }
 
+    private static void addCpJarsToClasspath(File directory, List<String> 
classpath) {
+        // Add jar starts with hugegraph in proper order
+        String path = directory.getAbsolutePath();
+        for (String jar : hgJars) {
+            classpath.add(path + File.separator + jar);
+        }
+        if (directory.exists() && directory.isDirectory()) {
+            File[] files =
+                    directory.listFiles((dir, name) -> name.endsWith(".jar") 
&& !name.contains(
+                            "hugegraph"));
+            if (files != null) {
+                for (File file : files) {
+                    classpath.add(file.getAbsolutePath());
+                }
+            }
+        }
+    }
+
+    private void loadHgJars(){
+        try (InputStream is = 
ServerNodeWrapper.class.getResourceAsStream("/jar.txt");
+             BufferedReader reader = new BufferedReader(new 
InputStreamReader(is, StandardCharsets.UTF_8))) {
+            String line;
+            while ((line = reader.readLine()) != null) {
+                hgJars.add(line);
+            }

Review Comment:
   **⚠️ Code quality: printStackTrace() is used instead of proper logging**
   
   Using printStackTrace() instead of proper logging makes debugging harder in 
production and loses stack trace context.
   
   **Suggestion:**
   ```java
   private static final Logger LOG = 
LoggerFactory.getLogger(ServerNodeWrapper.class);
   
   private void loadHgJars() {
       try (InputStream is = 
ServerNodeWrapper.class.getResourceAsStream("/jar.txt");
            BufferedReader reader = new BufferedReader(new 
InputStreamReader(is, StandardCharsets.UTF_8))) {
           String line;
           while ((line = reader.readLine()) != null) {
               hgJars.add(line);
           }
       } catch (IOException e) {
           LOG.error("Failed to load hugegraph jar list", e);
           throw new RuntimeException("Failed to load jar list", e);
       }
   }
   ```



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to