Skip to content
Merged
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
205 changes: 205 additions & 0 deletions TestJavaClass.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,205 @@
import java.io.*;
import java.sql.*;
import java.util.*;
import java.net.*;
import javax.servlet.http.*;

/**
* 여러 문제점이 있는 Java 클래스
* 코드 리뷰 테스트용
*/
public class TestJavaClass {

// 전역 변수 사용 (나쁜 예)
private static String globalPassword = "123456";
private static String apiKey = "sk-1234567890abcdef";
private static List<String> globalData = new ArrayList<>();

Comment on lines +13 to +17
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove hard-coded secrets; avoid global mutable state.

Hard-coded globalPassword, apiKey and a mutable global list are severe risks and concurrency hazards. Replace secrets with config/env and make the shared list thread-safe (or eliminate it).

-    // 전역 변수 사용 (나쁜 예)
-    private static String globalPassword = "123456";
-    private static String apiKey = "sk-1234567890abcdef";
-    private static List<String> globalData = new ArrayList<>();
+    // 전역 상태 최소화. 비필요한 비밀 제거. 공유 리스트는 동기화.
+    private static final List<String> globalData = Collections.synchronizedList(new ArrayList<>());

Note: Gitleaks flagged a generic API key on Line 15. Rotate and purge immediately.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// 전역 변수 사용 (나쁜 예)
private static String globalPassword = "123456";
private static String apiKey = "sk-1234567890abcdef";
private static List<String> globalData = new ArrayList<>();
// 전역 상태 최소화. 비필요한 비밀 제거. 공유 리스트는 동기화.
private static final List<String> globalData = Collections.synchronizedList(new ArrayList<>());
🧰 Tools
🪛 Gitleaks (8.27.2)

15-15: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

🤖 Prompt for AI Agents
In TestJavaClass.java around lines 13 to 17, remove the hard-coded secrets and
global mutable state: do not store globalPassword or apiKey as static
literals—read them from configuration or environment variables or a secrets
manager and inject them (or throw if missing) and ensure you rotate/purge the
exposed key immediately; replace the mutable static globalData with a non-global
design or a thread-safe structure (e.g., make it instance-scoped or use a
concurrent collection like ConcurrentLinkedQueue or a synchronized wrapper) and
mark it final to avoid reassignment to eliminate concurrency hazards.

// 상수 대신 매직 넘버 사용
private static final int MAGIC_NUMBER = 100;

/**
* 보안 취약점이 있는 메서드
*/
public String vulnerableMethod(String userInput) {
// SQL 인젝션 취약점
String query = "SELECT * FROM users WHERE name = '" + userInput + "'";

// 명령어 인젝션 취약점
try {
Runtime.getRuntime().exec("echo " + userInput);
} catch (IOException e) {
e.printStackTrace();
}

// XSS 취약점 (웹 환경에서)
String htmlOutput = "<div>" + userInput + "</div>";

// 하드코딩된 자격 증명
String dbPassword = "mypassword123";
String dbUrl = "jdbc:mysql://localhost:3306/mydb";

return query + htmlOutput;
}
Comment on lines +24 to +43
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix SQL/command/XSS vulnerabilities and remove hard-coded creds.

Avoid string concat in SQL, shelling out with untrusted input, and raw HTML reflection. Also delete unused hard-coded DB creds.

-    public String vulnerableMethod(String userInput) {
-        // SQL 인젝션 취약점
-        String query = "SELECT * FROM users WHERE name = '" + userInput + "'";
-
-        // 명령어 인젝션 취약점
-        try {
-            Runtime.getRuntime().exec("echo " + userInput);
-        } catch (IOException e) {
-            e.printStackTrace();
-        }
-
-        // XSS 취약점 (웹 환경에서)
-        String htmlOutput = "<div>" + userInput + "</div>";
-
-        // 하드코딩된 자격 증명
-        String dbPassword = "mypassword123";
-        String dbUrl = "jdbc:mysql://localhost:3306/mydb";
-
-        return query + htmlOutput;
-    }
+    public String vulnerableMethod(String userInput) {
+        // Use parameterized SQL (example string; bind with PreparedStatement at call site)
+        String query = "SELECT * FROM users WHERE name = ?";
+        try {
+            // Avoid the shell; pass args directly
+            new ProcessBuilder("echo", userInput == null ? "" : userInput).start();
+        } catch (IOException e) {
+            // TODO: replace with logger
+        }
+        // Escape before embedding in HTML
+        String htmlOutput = "<div>" + escapeHtml(userInput) + "</div>";
+        return query + htmlOutput;
+    }

Add this helper somewhere in the class:

private static String escapeHtml(String s) {
    if (s == null) return "";
    return s.replace("&","&amp;")
            .replace("<","&lt;")
            .replace(">","&gt;")
            .replace("\"","&quot;")
            .replace("'","&#x27;");
}
🤖 Prompt for AI Agents
In TestJavaClass.java around lines 24 to 43, the method contains SQL injection
via string concatenation, unsafe command execution, reflected HTML XSS, and
hard-coded DB credentials; replace the concatenated SQL with a PreparedStatement
using a parameter for userInput, remove Runtime.getRuntime().exec usage and
instead perform any needed external calls with a safe API or ProcessBuilder with
separated, validated arguments (or eliminate the call), escape userInput before
inserting into HTML using the suggested escapeHtml helper (add that helper
method to the class), and delete the hard-coded dbPassword and dbUrl (or load
them from secure config/keystore). Ensure to handle SQLExceptions properly and
avoid returning sensitive info.


/**
* 예외 처리가 부족한 메서드
*/
public void badExceptionHandling() {
// 예외 처리 없음
File file = new File("test.txt");
FileInputStream fis = new FileInputStream(file);
int data = fis.read();
fis.close();

Comment on lines +49 to +54
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Use try-with-resources for File I/O; handle exceptions.

Current code leaks the stream on exceptions.

-        File file = new File("test.txt");
-        FileInputStream fis = new FileInputStream(file);
-        int data = fis.read();
-        fis.close();
+        File file = new File("test.txt");
+        try (FileInputStream fis = new FileInputStream(file)) {
+            int data = fis.read();
+            // use data
+        } catch (IOException ioe) {
+            // TODO: replace with logger or rethrow
+        }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// 예외 처리 없음
File file = new File("test.txt");
FileInputStream fis = new FileInputStream(file);
int data = fis.read();
fis.close();
// 예외 처리 없음
File file = new File("test.txt");
try (FileInputStream fis = new FileInputStream(file)) {
int data = fis.read();
// use data
} catch (IOException ioe) {
// TODO: replace with logger or rethrow
}
🤖 Prompt for AI Agents
In TestJavaClass.java around lines 49 to 54, the FileInputStream is created and
closed manually without exception handling which can leak the stream on errors;
replace this code with a try-with-resources block that constructs the
FileInputStream inside the try(...) so it is always closed, and add a catch for
IOException (or declare throws) to handle/report errors (e.g., log or rethrow)
instead of leaving the stream unprotected.

// Null 체크 없음
String[] array = new String[10];
String firstElement = array[0];
System.out.println(firstElement.length());

Comment on lines +56 to +59
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Guard against NPE.

array[0] is null; calling length() crashes.

-        String[] array = new String[10];
-        String firstElement = array[0];
-        System.out.println(firstElement.length());
+        String[] array = new String[10];
+        String firstElement = array[0];
+        if (firstElement != null) {
+            System.out.println(firstElement.length());
+        }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
String[] array = new String[10];
String firstElement = array[0];
System.out.println(firstElement.length());
String[] array = new String[10];
String firstElement = array[0];
if (firstElement != null) {
System.out.println(firstElement.length());
}
🤖 Prompt for AI Agents
In TestJavaClass.java around lines 56 to 59, the code reads array[0] and calls
length() but array[0] is null, causing an NPE; fix by ensuring the element is
initialized before use or by guarding the access with a null-check and handling
the null case (e.g., set a default string or skip/handle printing when null) so
length() is never invoked on a null reference.

// 리소스 누수 가능성
Connection conn = null;
try {
conn = DriverManager.getConnection("jdbc:mysql://localhost:3306/test", "user", "pass");
Statement stmt = conn.createStatement();
ResultSet rs = stmt.executeQuery("SELECT * FROM users");
while (rs.next()) {
System.out.println(rs.getString("name"));
}
} catch (SQLException e) {
e.printStackTrace();
}
// conn.close() 호출 안함!
}
Comment on lines +60 to +73
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Close DB resources; remove hard-coded creds; prefer DataSource.

Credentials in code and unclosed JDBC resources are critical issues.

-        Connection conn = null;
-        try {
-            conn = DriverManager.getConnection("jdbc:mysql://localhost:3306/test", "user", "pass");
-            Statement stmt = conn.createStatement();
-            ResultSet rs = stmt.executeQuery("SELECT * FROM users");
-            while (rs.next()) {
-                System.out.println(rs.getString("name"));
-            }
-        } catch (SQLException e) {
-            e.printStackTrace();
-        }
-        // conn.close() 호출 안함!
+        String url = System.getenv("DB_URL");
+        String username = System.getenv("DB_USERNAME");
+        String password = System.getenv("DB_PASSWORD");
+        try (Connection conn = DriverManager.getConnection(url, username, password);
+             Statement stmt = conn.createStatement();
+             ResultSet rs = stmt.executeQuery("SELECT name FROM users")) {
+            while (rs.next()) {
+                System.out.println(rs.getString("name"));
+            }
+        } catch (SQLException e) {
+            // TODO: replace with logger
+        }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// 리소스 누수 가능성
Connection conn = null;
try {
conn = DriverManager.getConnection("jdbc:mysql://localhost:3306/test", "user", "pass");
Statement stmt = conn.createStatement();
ResultSet rs = stmt.executeQuery("SELECT * FROM users");
while (rs.next()) {
System.out.println(rs.getString("name"));
}
} catch (SQLException e) {
e.printStackTrace();
}
// conn.close() 호출 안함!
}
// 리소스 누수 가능성
String url = System.getenv("DB_URL");
String username = System.getenv("DB_USERNAME");
String password = System.getenv("DB_PASSWORD");
try (Connection conn = DriverManager.getConnection(url, username, password);
Statement stmt = conn.createStatement();
ResultSet rs = stmt.executeQuery("SELECT name FROM users")) {
while (rs.next()) {
System.out.println(rs.getString("name"));
}
} catch (SQLException e) {
// TODO: replace with logger
}
}
🧰 Tools
🪛 ast-grep (0.38.6)

[warning] 62-62: A secret is hard-coded in the application. Secrets stored in source code, such as credentials, identifiers, and other types of sensitive data, can be leaked and used by internal or external malicious actors. Use environment variables to securely provide credentials and other secrets or retrieve them from a secure vault or Hardware Security Module (HSM).
Context: DriverManager.getConnection("jdbc:mysql://localhost:3306/test", "user", "pass")
Note: [CWE-798] Use of Hard-coded Credentials. [REFERENCES]
- https://cheatsheetseries.owasp.org/cheatsheets/Secrets_Management_Cheat_Sheet.html

(drivermanager-hardcoded-secret-java)

🤖 Prompt for AI Agents
In TestJavaClass.java around lines 60 to 73, the code opens a JDBC
Connection/Statement/ResultSet with hard-coded credentials and never closes
them; replace this block with a try-with-resources that acquires a DataSource
(not hard-coded JDBC URL/username/password) and obtains
Connection/Statement/ResultSet from it so all resources are auto-closed; move
credentials/config into external configuration (properties/env) or use an
injected DataSource, and ensure SQL exceptions are logged/handled without
leaving resources open.


/**
* 비효율적인 알고리즘
*/
public List<String> inefficientAlgorithm(List<String> items) {
List<String> result = new ArrayList<>();

// O(n²) 복잡도의 중첩 반복문
for (int i = 0; i < items.size(); i++) {
for (int j = 0; j < items.size(); j++) {
if (items.get(i).equals(items.get(j))) {
result.add(items.get(i));
}
}
}

// 불필요한 객체 생성
String temp = new String("temporary");
String anotherTemp = new String("another");

// 중복 계산
int sum1 = 0;
for (String item : items) {
sum1 += item.length();
}

int sum2 = 0;
for (String item : items) {
sum2 += item.length();
}

return result;
}
Comment on lines +78 to +106
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Replace O(n²) duplicate detection; drop dead work.

Nested loops, unused temps, and repeated sums waste CPU/memory.

-    public List<String> inefficientAlgorithm(List<String> items) {
-        List<String> result = new ArrayList<>();
-        // O(n²) 복잡도의 중첩 반복문
-        for (int i = 0; i < items.size(); i++) {
-            for (int j = 0; j < items.size(); j++) {
-                if (items.get(i).equals(items.get(j))) {
-                    result.add(items.get(i));
-                }
-            }
-        }
-        // 불필요한 객체 생성
-        String temp = new String("temporary");
-        String anotherTemp = new String("another");
-        // 중복 계산
-        int sum1 = 0;
-        for (String item : items) { sum1 += item.length(); }
-        int sum2 = 0;
-        for (String item : items) { sum2 += item.length(); }
-        return result;
-    }
+    public List<String> inefficientAlgorithm(List<String> items) {
+        Set<String> seen = new HashSet<>();
+        Set<String> dupes = new LinkedHashSet<>();
+        for (String item : items) {
+            if (!seen.add(item)) {
+                dupes.add(item);
+            }
+        }
+        return new ArrayList<>(dupes);
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public List<String> inefficientAlgorithm(List<String> items) {
List<String> result = new ArrayList<>();
// O(n²) 복잡도의 중첩 반복문
for (int i = 0; i < items.size(); i++) {
for (int j = 0; j < items.size(); j++) {
if (items.get(i).equals(items.get(j))) {
result.add(items.get(i));
}
}
}
// 불필요한 객체 생성
String temp = new String("temporary");
String anotherTemp = new String("another");
// 중복 계산
int sum1 = 0;
for (String item : items) {
sum1 += item.length();
}
int sum2 = 0;
for (String item : items) {
sum2 += item.length();
}
return result;
}
public List<String> inefficientAlgorithm(List<String> items) {
Set<String> seen = new HashSet<>();
Set<String> dupes = new LinkedHashSet<>();
for (String item : items) {
if (!seen.add(item)) {
dupes.add(item);
}
}
return new ArrayList<>(dupes);
}
🤖 Prompt for AI Agents
In TestJavaClass.java around lines 78 to 106, the method inefficientAlgorithm
uses O(n²) nested loops to detect duplicates, creates unnecessary String
objects, and computes the same sum twice; replace the nested loops with a
single-pass approach using a HashSet for seen items and another Set for
duplicates (add to result from the duplicates set if you need unique duplicated
values), remove the unnecessary new String(...) allocations, compute the total
length in the same single iteration (do not compute sum1 and sum2 separately),
and return the properly deduplicated result collection (or preserve intended
multiplicity if required by changing logic accordingly).


/**
* 나쁜 클래스 설계
*/
public static class BadInnerClass {
private List<String> data;
private int counter;

public BadInnerClass() {
this.data = new ArrayList<>();
this.counter = 0;
}

// 타입 체크 없음
public void addItem(Object item) {
data.add(item.toString());
counter++;
}

// 참조 노출 (캡슐화 위반)
public List<String> getData() {
return data; // 복사본 반환하지 않음
}
Comment on lines +121 to +129
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Tighten typing and avoid leaking internals.

Use String for items and return an unmodifiable view/copy.

-        public void addItem(Object item) {
-            data.add(item.toString());
-            counter++;
-        }
+        public void addItem(String item) {
+            if (item != null) {
+                data.add(item);
+                counter++;
+            }
+        }
 
-        public List<String> getData() {
-            return data; // 복사본 반환하지 않음
-        }
+        public List<String> getData() {
+            return Collections.unmodifiableList(new ArrayList<>(data));
+        }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public void addItem(Object item) {
data.add(item.toString());
counter++;
}
// 참조 노출 (캡슐화 위반)
public List<String> getData() {
return data; // 복사본 반환하지 않음
}
public void addItem(String item) {
if (item != null) {
data.add(item);
counter++;
}
}
// 참조 노출 (캡슐화 위반)
public List<String> getData() {
return Collections.unmodifiableList(new ArrayList<>(data));
}
🤖 Prompt for AI Agents
In TestJavaClass.java around lines 121 to 129, the addItem method accepts Object
and stores item.toString(), and getData returns the internal mutable list
exposing internals; change addItem signature to accept String (e.g., public void
addItem(String item)) and use that directly (optionally validate for null), and
make getData return an unmodifiable view or a defensive copy (e.g.,
Collections.unmodifiableList(new ArrayList<>(data))) so callers cannot modify
the internal list.


// 긴 메서드 (단일 책임 원칙 위반)
public void processData() {
List<String> processed = new ArrayList<>();

for (String item : data) {
if (item != null) {
if (item.length() > 10) {
processed.add(item.toUpperCase());
} else if (item.length() > 5) {
processed.add(item.toLowerCase());
} else {
processed.add(item);
}
}
}

// 중복 로직
List<String> result = new ArrayList<>();
for (String item : processed) {
result.add(item);
}

// 전역 변수 수정
globalData.addAll(result);
}
}

/**
* 메모리 누수 가능성이 있는 메서드
*/
public void potentialMemoryLeak() {
List<String> largeList = new ArrayList<>();

// 무한 루프 가능성
for (int i = 0; i < 1000000; i++) {
largeList.add("item" + i);

// 메모리 해제 안함
if (i % 1000 == 0) {
System.out.println("Processed: " + i);
}
}

// 스레드 안전성 문제
globalData.addAll(largeList);
}
Comment on lines +161 to +176
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Avoid building 1M-element list; batch or stream; synchronize writes.

Current approach spikes heap and pins memory in globalData.

-    public void potentialMemoryLeak() {
-        List<String> largeList = new ArrayList<>();
-        // 무한 루프 가능성
-        for (int i = 0; i < 1000000; i++) {
-            largeList.add("item" + i);
-            // 메모리 해제 안함
-            if (i % 1000 == 0) {
-                System.out.println("Processed: " + i);
-            }
-        }
-        // 스레드 안전성 문제
-        globalData.addAll(largeList);
-    }
+    public void potentialMemoryLeak() {
+        List<String> batch = new ArrayList<>(BATCH_SIZE);
+        for (int i = 0; i < 1_000_000; i++) {
+            batch.add("item" + i);
+            if (i % 1_000 == 0) {
+                System.out.println("Processed: " + i);
+            }
+            if (batch.size() >= BATCH_SIZE) {
+                synchronized (globalData) {
+                    globalData.addAll(batch);
+                }
+                batch.clear();
+            }
+        }
+        if (!batch.isEmpty()) {
+            synchronized (globalData) {
+                globalData.addAll(batch);
+            }
+        }
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public void potentialMemoryLeak() {
List<String> largeList = new ArrayList<>();
// 무한 루프 가능성
for (int i = 0; i < 1000000; i++) {
largeList.add("item" + i);
// 메모리 해제 안함
if (i % 1000 == 0) {
System.out.println("Processed: " + i);
}
}
// 스레드 안전성 문제
globalData.addAll(largeList);
}
public void potentialMemoryLeak() {
List<String> batch = new ArrayList<>(BATCH_SIZE);
for (int i = 0; i < 1_000_000; i++) {
batch.add("item" + i);
if (i % 1_000 == 0) {
System.out.println("Processed: " + i);
}
if (batch.size() >= BATCH_SIZE) {
synchronized (globalData) {
globalData.addAll(batch);
}
batch.clear();
}
}
if (!batch.isEmpty()) {
synchronized (globalData) {
globalData.addAll(batch);
}
}
}
🤖 Prompt for AI Agents
In TestJavaClass.java around lines 161 to 176, the method builds a
1,000,000-element ArrayList and then adds it wholesale to a shared globalData,
causing high heap usage and potential memory pinning plus thread-safety issues;
change the method to process elements in smaller batches or stream them
(avoiding a single huge in-memory list), periodically flush each batch into
globalData, and synchronize or use a concurrent collection when writing to
globalData to ensure thread safety and avoid retaining the entire dataset in
memory at once.


/**
* 사용되지 않는 메서드
*/
private void unusedMethod() {
System.out.println("이 메서드는 사용되지 않습니다.");
}

/**
* 메인 메서드
*/
public static void main(String[] args) {
TestJavaClass test = new TestJavaClass();

// 문제가 있는 메서드들 호출
test.vulnerableMethod("'; DROP TABLE users; --");
test.badExceptionHandling();
test.inefficientAlgorithm(Arrays.asList("a", "b", "c", "a", "b"));
test.potentialMemoryLeak();

// 내부 클래스 사용
BadInnerClass badClass = new BadInnerClass();
badClass.addItem("test");
badClass.addItem(123);
badClass.processData();

System.out.println("테스트 완료");
}
}