From 4f2a4085d3f17e96c5bb95aca807e31344999657 Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Mon, 3 Aug 2026 08:41:27 +0200 Subject: [PATCH 1/2] Implement new rule S9130 Detect when the return value of InputStream.read() or Reader.read() is cast to byte or char before being checked for -1, which can cause false end-of-stream detection or missed end-of-stream conditions. --- .../StreamReadResultCastCheckSample.java | 110 ++++++++++++++++++ .../checks/StreamReadResultCastCheck.java | 62 ++++++++++ .../checks/StreamReadResultCastCheckTest.java | 34 ++++++ .../org/sonar/l10n/java/rules/java/S9130.html | 28 +++++ .../org/sonar/l10n/java/rules/java/S9130.json | 21 ++++ .../resources/profiles/Sonar_agentic_AI/S9130 | 0 .../main/resources/profiles/Sonar_way/S9130 | 0 7 files changed, 255 insertions(+) create mode 100644 java-checks-test-sources/default/src/main/java/checks/StreamReadResultCastCheckSample.java create mode 100644 java-checks/src/main/java/org/sonar/java/checks/StreamReadResultCastCheck.java create mode 100644 java-checks/src/test/java/org/sonar/java/checks/StreamReadResultCastCheckTest.java create mode 100644 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9130.html create mode 100644 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9130.json create mode 100644 sonar-java-plugin/src/main/resources/profiles/Sonar_agentic_AI/S9130 create mode 100644 sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9130 diff --git a/java-checks-test-sources/default/src/main/java/checks/StreamReadResultCastCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/StreamReadResultCastCheckSample.java new file mode 100644 index 00000000000..f9057275be3 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/StreamReadResultCastCheckSample.java @@ -0,0 +1,110 @@ +package checks; + +import java.io.BufferedInputStream; +import java.io.FileInputStream; +import java.io.FileReader; +import java.io.IOException; +import java.io.InputStream; +import java.io.Reader; + +class StreamReadResultCastCheckSample { + + void byteCastInWhileLoop(FileInputStream fis) throws IOException { + byte b; + while ((b = (byte) fis.read()) != -1) { // Noncompliant {{Store the return value of "read()" in an "int" variable and check for -1 before casting.}} + process(b); + } + } + + void byteCastAssignment(InputStream is) throws IOException { + byte value = (byte) is.read(); // Noncompliant + } + + void charCastFromInputStream(InputStream is) throws IOException { + char c; + while ((c = (char) is.read()) != -1) { // Noncompliant + process(c); + } + } + + void byteCastInDoWhile(FileInputStream fis) throws IOException { + byte b; + do { + b = (byte) fis.read(); // Noncompliant + } while (b != -1); + } + + void charCastFromReader(FileReader reader) throws IOException { + char c; + while ((c = (char) reader.read()) != -1) { // Noncompliant + process(c); + } + } + + void charCastAssignmentFromReader(Reader reader) throws IOException { + char c = (char) reader.read(); // Noncompliant + } + + void byteCastWithParentheses(InputStream is) throws IOException { + byte b = (byte) (is.read()); // Noncompliant + } + + void byteCastFromSubtype(BufferedInputStream bis) throws IOException { + byte b = (byte) bis.read(); // Noncompliant + } + + // Compliant cases + + void correctPatternWithIntVariable(FileInputStream fis) throws IOException { + int data; + while ((data = fis.read()) != -1) { + byte b = (byte) data; + process(b); + } + } + + void multiArgReadByteArray(InputStream is) throws IOException { + byte[] buffer = new byte[1024]; + int bytesRead = is.read(buffer); + } + + void multiArgReadByteArrayWithOffset(InputStream is) throws IOException { + byte[] buffer = new byte[1024]; + int bytesRead = is.read(buffer, 0, 1024); + } + + void noCastAtAll(InputStream is) throws IOException { + int value = is.read(); + } + + void multiArgReadCharArray(Reader reader) throws IOException { + char[] buffer = new char[1024]; + int charsRead = reader.read(buffer); + } + + void correctPatternWithReader(Reader reader) throws IOException { + int data; + while ((data = reader.read()) != -1) { + char c = (char) data; + process(c); + } + } + + void wideningCast(InputStream is) throws IOException { + long value = (long) is.read(); + } + + void customReadMethod() { + CustomReader custom = new CustomReader(); + byte b = (byte) custom.read(); + } + + private void process(byte b) {} + private void process(char c) {} + + static class CustomReader { + int read() { + return 0; + } + } +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/StreamReadResultCastCheck.java b/java-checks/src/main/java/org/sonar/java/checks/StreamReadResultCastCheck.java new file mode 100644 index 00000000000..af6507562e0 --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/StreamReadResultCastCheck.java @@ -0,0 +1,62 @@ +/* + * SonarQube Java + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks; + +import java.util.Collections; +import java.util.List; +import org.sonar.check.Rule; +import org.sonar.java.model.ExpressionUtils; +import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.semantic.MethodMatchers; +import org.sonar.plugins.java.api.tree.ExpressionTree; +import org.sonar.plugins.java.api.tree.MethodInvocationTree; +import org.sonar.plugins.java.api.tree.Tree; +import org.sonar.plugins.java.api.tree.TypeCastTree; + +@Rule(key = "S9130") +public class StreamReadResultCastCheck extends IssuableSubscriptionVisitor { + + private static final MethodMatchers READ_MATCHERS = MethodMatchers.or( + MethodMatchers.create() + .ofSubTypes("java.io.InputStream") + .names("read") + .addWithoutParametersMatcher() + .build(), + MethodMatchers.create() + .ofSubTypes("java.io.Reader") + .names("read") + .addWithoutParametersMatcher() + .build()); + + @Override + public List nodesToVisit() { + return Collections.singletonList(Tree.Kind.TYPE_CAST); + } + + @Override + public void visitNode(Tree tree) { + TypeCastTree castTree = (TypeCastTree) tree; + var castToType = castTree.type().symbolType(); + if (castToType.is("byte") || castToType.is("char")) { + ExpressionTree expression = ExpressionUtils.skipParentheses(castTree.expression()); + if (expression.is(Tree.Kind.METHOD_INVOCATION) && READ_MATCHERS.matches((MethodInvocationTree) expression)) { + reportIssue(castTree, "Store the return value of \"read()\" in an \"int\" variable and check for -1 before casting."); + } + } + } + +} diff --git a/java-checks/src/test/java/org/sonar/java/checks/StreamReadResultCastCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/StreamReadResultCastCheckTest.java new file mode 100644 index 00000000000..b0fa38fb26b --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/StreamReadResultCastCheckTest.java @@ -0,0 +1,34 @@ +/* + * SonarQube Java + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks; + +import org.junit.jupiter.api.Test; +import org.sonar.java.checks.verifier.CheckVerifier; + +import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath; + +class StreamReadResultCastCheckTest { + + @Test + void test() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/StreamReadResultCastCheckSample.java")) + .withCheck(new StreamReadResultCastCheck()) + .verifyIssues(); + } + +} diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9130.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9130.html new file mode 100644 index 00000000000..9efd9efeff9 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9130.html @@ -0,0 +1,28 @@ +

Why is this an issue?

+

Stream reading methods like InputStream.read() and Reader.read() return an int that can represent all +possible data values (0-255 for byte-oriented streams) plus a special sentinel value (-1) to indicate end-of-stream.

+

When the return value is cast to a narrower type (byte or char) before checking for -1, the maximum +unsigned value (such as 255 or 0xFF) wraps around to -1 due to signed integer representation. This makes it impossible to distinguish +between a legitimate data value and end-of-stream, leading to premature termination or data corruption.

+

Noncompliant code example

+
+FileInputStream fis = new FileInputStream("data.bin");
+byte b;
+while ((b = (byte) fis.read()) != -1) {  // Noncompliant
+    process(b);
+}
+
+

Compliant solution

+
+FileInputStream fis = new FileInputStream("data.bin");
+int data;
+while ((data = fis.read()) != -1) {
+    byte b = (byte) data;
+    process(b);
+}
+
+

Resources

+ diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9130.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9130.json new file mode 100644 index 00000000000..1b5aae414c2 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9130.json @@ -0,0 +1,21 @@ +{ + "title": "Stream read results should be checked for -1 before casting", + "type": "BUG", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5 min" + }, + "tags": [], + "defaultSeverity": "Critical", + "ruleSpecification": "RSPEC-9130", + "sqKey": "S9130", + "scope": "All", + "quickfix": "unknown", + "code": { + "impacts": { + "RELIABILITY": "HIGH" + }, + "attribute": "LOGICAL" + } +} diff --git a/sonar-java-plugin/src/main/resources/profiles/Sonar_agentic_AI/S9130 b/sonar-java-plugin/src/main/resources/profiles/Sonar_agentic_AI/S9130 new file mode 100644 index 00000000000..e69de29bb2d diff --git a/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9130 b/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9130 new file mode 100644 index 00000000000..e69de29bb2d From c2262d325af2fd4a36fbbec80cbec03d1e0eaed3 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 3 Aug 2026 12:07:13 +0200 Subject: [PATCH 2/2] Update ruling results for PR #5855 (#5858) Co-authored-by: github-actions[bot] --- its/ruling/src/test/resources/diff_S9130.json | 6 ++++++ .../resources/eclipse-jetty/java-S9130.json | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 its/ruling/src/test/resources/diff_S9130.json create mode 100644 its/ruling/src/test/resources/eclipse-jetty/java-S9130.json diff --git a/its/ruling/src/test/resources/diff_S9130.json b/its/ruling/src/test/resources/diff_S9130.json new file mode 100644 index 00000000000..cc3c537c0a5 --- /dev/null +++ b/its/ruling/src/test/resources/diff_S9130.json @@ -0,0 +1,6 @@ +{ + "ruleKey": "S9130", + "hasTruePositives": true, + "falseNegatives": 0, + "falsePositives": 0 +} \ No newline at end of file diff --git a/its/ruling/src/test/resources/eclipse-jetty/java-S9130.json b/its/ruling/src/test/resources/eclipse-jetty/java-S9130.json new file mode 100644 index 00000000000..3c5d0a67184 --- /dev/null +++ b/its/ruling/src/test/resources/eclipse-jetty/java-S9130.json @@ -0,0 +1,19 @@ +{ +"org.eclipse.jetty:jetty-project:jetty-server/src/test/java/org/eclipse/jetty/server/DumpHandler.java": [ +84 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestBase.java": [ +1379, +1386, +1407, +1414, +1423 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SSLSelectChannelConnectorLoadTest.java": [ +303 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/UrlEncoded.java": [ +466, +467 +] +}