hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-17 01:03:14 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #20190 from Napalys/java/jvm-exit-query-promotion

Browse Source 
Java: Enhance `java/jvm-exit` query and add to quality
This commit is contained in:
Napalys Klicius
2025-08-27 13:23:02 +02:00
committed by GitHub
parent f232335618 b3f90bbdfc
commit a3aacfb688
11 changed files with 246 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
java/ql/integration-tests/java/query-suite/java-code-quality-extended.qls.expected
View File

@@ -82,6 +82,7 @@ ql/java/ql/src/Violations of Best Practice/Records/IgnoredSerializationMembersOf
ql/java/ql/src/Violations of Best Practice/SpecialCharactersInLiterals/NonExplicitControlAndWhitespaceCharsInLiterals.ql
ql/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToRunFinalizersOnExit.ql
ql/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToStringToString.ql
ql/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToSystemExit.ql
ql/java/ql/src/Violations of Best Practice/Undesirable Calls/DefaultToString.ql
ql/java/ql/src/Violations of Best Practice/Undesirable Calls/DoNotCallFinalize.ql
ql/java/ql/src/Violations of Best Practice/Undesirable Calls/PrintLnArray.ql

1
java/ql/integration-tests/java/query-suite/not_included_in_qls.expected
View File

@@ -186,7 +186,6 @@ ql/java/ql/src/Violations of Best Practice/Magic Constants/MagicNumbersUseConsta
ql/java/ql/src/Violations of Best Practice/Magic Constants/MagicStringsUseConstant.ql
ql/java/ql/src/Violations of Best Practice/Naming Conventions/ConfusingOverridesNames.ql
ql/java/ql/src/Violations of Best Practice/Naming Conventions/LocalShadowsField.ql
ql/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToSystemExit.ql
ql/java/ql/src/Violations of Best Practice/Undesirable Calls/GarbageCollection.ql
ql/java/ql/src/Violations of Best Practice/legacy/AutoBoxing.ql
ql/java/ql/src/Violations of Best Practice/legacy/FinallyMayNotComplete.ql

29
java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToSystemExit.java
View File

@@ -4,7 +4,7 @@ class FileOutput {
try {
output.write(s.getBytes());
} catch (IOException e) {
System.exit(1);
System.exit(1); // BAD: Should handle or propagate error instead of exiting
}
return true;
}
@@ -16,9 +16,30 @@ class Action {
// ...
// Perform tasks ...
// ...
System.exit(0);
System.exit(0); // BAD: Should return status or throw exception
}
public static void main(String[] args) {
new Action(args).run();
new Action().run();
}
}
}
// Good example: Proper error handling
class BetterAction {
public int run() throws Exception {
// ...
// Perform tasks ...
// ...
return 0; // Return status instead of calling System.exit
}
public static void main(String[] args) {
try {
BetterAction action = new BetterAction();
int exitCode = action.run();
System.exit(exitCode); // GOOD: Exit from main method
} catch (Exception e) {
System.err.println("Error: " + e.getMessage());
System.exit(1); // GOOD: Exit from main method on error
}
}
}

27
java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToSystemExit.qhelp
View File

@@ -13,17 +13,20 @@ program state from being written to disk consistently.</p>
<p>It is sometimes considered acceptable to call <code>System.exit</code>
from a program's <code>main</code> method in order to indicate the overall exit status
of the program. Such calls are an exception to this rule.</p>
of the program. The <code>main</code> method should be the primary place
where exit conditions are handled, as it represents the natural termination point
of the application. Such calls are an exception to this rule.</p>
</overview>
<recommendation>
<p>It is usually preferable to use a different mechanism for reporting
failure conditions. Consider returning a special value (perhaps
<code>null</code>) that users of the current method check for and
recover from appropriately. Alternatively, throw a suitable exception, which
unwinds the stack and allows properly written code to clean up after itself,
while leaving other threads undisturbed.</p>
<p>Instead of calling <code>System.exit</code> from non-main methods, prefer to propagate
errors upward to the <code>main</code> method where they can be handled appropriately.
Consider returning a special value (perhaps <code>null</code>) that users of the current
method check for and recover from appropriately. Alternatively, throw a suitable exception,
which unwinds the stack and allows properly written code to clean up after itself,
while leaving other threads undisturbed. The <code>main</code> method can then catch
these exceptions and decide whether to exit the program and with what exit code.</p>
</recommendation>
<example>
@@ -38,12 +41,14 @@ upwards and be handled by a method that knows how to recover.</p>
<p>Problem 2 is more subtle. In this example, there is just one entry point to
the program (the <code>main</code> method), which constructs an
<code>Action</code> and performs it. <code>Action.run</code> calls
<code>System.exit</code> to indicate successful completion. Consider,
however, how this code might be integrated in an application server that
constructs <code>Action</code> instances and calls
<code>System.exit</code> to indicate successful completion. Instead, the
<code>run</code> method should return a status code or throw an exception
on failure, allowing the <code>main</code> method to decide whether to exit
and with what exit code. Consider how this code might be integrated in an
application server that constructs <code>Action</code> instances and calls
<code>run</code> on them without going through <code>main</code>.
The fact that <code>run</code> terminates the JVM instead of returning its
exit code as an integer makes that use-case impossible.</p>
exit code makes that use-case impossible.</p>
<sample src="CallsToSystemExit.java" />

88
java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToSystemExit.ql
View File

@@ -4,26 +4,80 @@
* reuse and prevent important cleanup steps from running.
* @kind problem
* @problem.severity warning
* @precision low
* @precision medium
* @id java/jvm-exit
* @tags reliability
* maintainability
* @previous-id java/jvm-exit-prevents-cleanup-and-reuse
* @tags quality
* reliability
* correctness
* external/cwe/cwe-382
*/
import java
from Method m, MethodCall sysexitCall, Method sysexit, Class system
where
sysexitCall = m.getACallSite(sysexit) and
(sysexit.hasName("exit") or sysexit.hasName("halt")) and
sysexit.getDeclaringType() = system and
(
system.hasQualifiedName("java.lang", "System") or
system.hasQualifiedName("java.lang", "Runtime")
) and
m.fromSource() and
not m instanceof MainMethod
select sysexitCall,
"Avoid calls to " + sysexit.getDeclaringType().getName() + "." + sysexit.getName() +
"() as this makes code harder to reuse."
/**
* A `Method` which, when called, causes the JVM to exit or halt.
*
* Explicitly includes these methods from the java standard library:
* - `java.lang.System.exit`
* - `java.lang.Runtime.halt`
* - `java.lang.Runtime.exit`
*/
class ExitOrHaltMethod extends Method {
ExitOrHaltMethod() {
exists(Class system | this.getDeclaringType() = system |
this.hasName("exit") and
system.hasQualifiedName("java.lang", ["System", "Runtime"])
or
this.hasName("halt") and
system.hasQualifiedName("java.lang", "Runtime")
)
}
}
/** A `MethodCall` to an `ExitOrHaltMethod`, which causes the JVM to exit abruptly. */
class ExitOrHaltMethodCall extends MethodCall {
ExitOrHaltMethodCall() {
exists(ExitOrHaltMethod exitMethod | this.getMethod() = exitMethod |
exists(SourceMethodNotMainOrTest srcMethod | this = srcMethod.getACallSite(exitMethod))
)
}
}
/**
* An intentional `MethodCall` to a system or runtime "exit" method, such as for
* functions which exist for the purpose of exiting the program. Assumes that an exit method
* call within a method is intentional if the exit code is passed from a parameter of the
* enclosing method.
*/
class IntentionalExitMethodCall extends ExitOrHaltMethodCall {
IntentionalExitMethodCall() {
this.getMethod().hasName("exit") and
this.getAnArgument() = this.getEnclosingCallable().getAParameter().getAnAccess()
}
}
/**
* A `Method` that is defined in source code and is not a `MainMethod` or a `LikelyTestMethod`.
*/
class SourceMethodNotMainOrTest extends Method {
SourceMethodNotMainOrTest() {
this.fromSource() and
not this instanceof MainMethod and
not (
this.getEnclosingCallable*() instanceof LikelyTestMethod
or
this.getDeclaringType()
.getEnclosingType*()
.(LocalClassOrInterface)
.getLocalTypeDeclStmt()
.getEnclosingCallable() instanceof LikelyTestMethod
)
}
}
from ExitOrHaltMethodCall mc
where not mc instanceof IntentionalExitMethodCall
select mc,
"Avoid calls to " + mc.getMethod().getDeclaringType().getName() + "." + mc.getMethod().getName() +
"() as this prevents runtime cleanup and makes code harder to reuse."

6
java/ql/test/query-tests/CallsToSystemExit/CallsToSystemExit.expected Normal file
View File

@@ -0,0 +1,6 @@
| ExampleRuntimeExit.java:22:17:22:44 | exit(...) | Avoid calls to Runtime.exit() as this prevents runtime cleanup and makes code harder to reuse. |
| ExampleRuntimeExit.java:25:17:25:44 | exit(...) | Avoid calls to Runtime.exit() as this prevents runtime cleanup and makes code harder to reuse. |
| ExampleRuntimeHalt.java:18:17:18:44 | halt(...) | Avoid calls to Runtime.halt() as this prevents runtime cleanup and makes code harder to reuse. |
| ExampleRuntimeHalt.java:21:17:21:44 | halt(...) | Avoid calls to Runtime.halt() as this prevents runtime cleanup and makes code harder to reuse. |
| ExampleSystemExit.java:22:17:22:30 | exit(...) | Avoid calls to System.exit() as this prevents runtime cleanup and makes code harder to reuse. |
| ExampleSystemExit.java:25:17:25:30 | exit(...) | Avoid calls to System.exit() as this prevents runtime cleanup and makes code harder to reuse. |

2
java/ql/test/query-tests/CallsToSystemExit/CallsToSystemExit.qlref Normal file
View File

@@ -0,0 +1,2 @@
query: Violations of Best Practice/Undesirable Calls/CallsToSystemExit.ql
postprocess: utils/test/InlineExpectationsTestQuery.ql

37
java/ql/test/query-tests/CallsToSystemExit/ExampleRuntimeExit.java Normal file
View File

@@ -0,0 +1,37 @@
import java.io.FileOutputStream;
import java.io.IOException;
public class ExampleRuntimeExit {
public static void main(String[] args) {
Action action = new Action();
try {
action.run();
} catch (Exception e) {
printUsageAndExit(e.getMessage(), 1);
}
Runtime.getRuntime().exit(0); // COMPLIANT
}
static class Action {
public void run() {
try {
FileOutputStream fos = new FileOutputStream("output.txt");
fos.write("Hello, World!".getBytes());
fos.close();
Runtime.getRuntime().exit(0); // $ Alert
} catch (IOException e) {
e.printStackTrace();
Runtime.getRuntime().exit(1); // $ Alert
} catch (Exception e) {
// re-throw the exception
throw e;
}
}
}
protected static void printUsageAndExit(final String message, final int exitCode) {
System.err.println("Usage: <example_cmd> <example_args> : " + message);
Runtime.getRuntime().exit(exitCode); // COMPLIANT
}
}

25
java/ql/test/query-tests/CallsToSystemExit/ExampleRuntimeHalt.java Normal file
View File

@@ -0,0 +1,25 @@
import java.io.FileOutputStream;
import java.io.IOException;
public class ExampleRuntimeHalt {
public static void main(String[] args) {
Action action = new Action();
action.run();
Runtime.getRuntime().halt(0); // COMPLIANT
}
static class Action {
public void run() {
try {
FileOutputStream fos = new FileOutputStream("output.txt");
fos.write("Hello, World!".getBytes());
fos.close();
Runtime.getRuntime().halt(0); // $ Alert
} catch (IOException e) {
e.printStackTrace();
Runtime.getRuntime().halt(1); // $ Alert
}
}
}
}

37
java/ql/test/query-tests/CallsToSystemExit/ExampleSystemExit.java Normal file
View File

@@ -0,0 +1,37 @@
import java.io.FileOutputStream;
import java.io.IOException;
public class ExampleSystemExit {
public static void main(String[] args) {
Action action = new Action();
try {
action.run();
} catch (Exception e) {
printUsageAndExit(e.getMessage(), 1);
}
System.exit(0); // COMPLIANT
}
static class Action {
public void run() {
try {
FileOutputStream fos = new FileOutputStream("output.txt");
fos.write("Hello, World!".getBytes());
fos.close();
System.exit(0); // $ Alert
} catch (IOException e) {
e.printStackTrace();
System.exit(1); // $ Alert
} catch (Exception e) {
// re-throw the exception
throw e;
}
}
}
protected static void printUsageAndExit(final String message, final int exitCode) {
System.err.println("Usage: <example_cmd> <example_args> : " + message);
System.exit(exitCode); // COMPLIANT
}
}

26
java/ql/test/query-tests/CallsToSystemExit/LocalClassInTestMethod.java Normal file
View File

@@ -0,0 +1,26 @@
public class LocalClassInTestMethod {
public void testNestedCase() {
class OuterLocalClass {
void func() {
class NestedLocalClass {
void nestedMethod() {
System.exit(4);
Runtime.getRuntime().halt(5);
}
}
}
}
OuterLocalClass outer = new OuterLocalClass();
outer.func();
}
public void testNestedCase2() {
class OuterLocalClass {
class NestedLocalClass {
void nestedMethod() {
System.exit(4);
Runtime.getRuntime().halt(5);
}
}
}
}
}