Skip to content

Fix possible out of bounds exception in switch pattern hint#7973

Merged
mbien merged 1 commit intoapache:masterfrom
mbien:fix-switch-hint-ioobe
Nov 19, 2024
Merged

Fix possible out of bounds exception in switch pattern hint#7973
mbien merged 1 commit intoapache:masterfrom
mbien:fix-switch-hint-ioobe

Conversation

@mbien
Copy link
Copy Markdown
Member

@mbien mbien commented Nov 19, 2024

index can be less than 0

found this issue while testing #7968

this (invalid) snippet:

    public static void main(String[] args) {
        switch (args) {
            case 0 -> {}
         }
    }

will cause:

java.lang.IndexOutOfBoundsException: -1
	at com.sun.tools.javac.util.List.get(List.java:468)
	at org.netbeans.modules.java.hints.jdk.ConvertToSwitchPatternInstanceOf.switchPatternMatchToSwitchNull(ConvertToSwitchPatternInstanceOf.java:281)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)

@mbien mbien added Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) hints labels Nov 19, 2024
@mbien mbien added this to the NB25 milestone Nov 19, 2024
@mbien mbien requested a review from lahodaj November 19, 2024 00:33
@lahodaj
Copy link
Copy Markdown
Contributor

lahodaj commented Nov 19, 2024

Nice catch. Overall, I agree with the solution, although having a test would be good. Also, we could consider making the code a little bit more understandable, e.g.:

diff --git a/java/java.hints/src/org/netbeans/modules/java/hints/jdk/ConvertToSwitchPatternInstanceOf.java b/java/java.hints/src/org/netbeans/modules/java/hints/jdk/ConvertToSwitchPatternInstanceOf.java
index a295ed153f..f52cd72e33 100644
--- a/java/java.hints/src/org/netbeans/modules/java/hints/jdk/ConvertToSwitchPatternInstanceOf.java
+++ b/java/java.hints/src/org/netbeans/modules/java/hints/jdk/ConvertToSwitchPatternInstanceOf.java
@@ -28,6 +28,7 @@ import com.sun.source.tree.StatementTree;
 import com.sun.source.tree.SwitchTree;
 import com.sun.source.tree.ThrowTree;
 import com.sun.source.tree.Tree;
+import com.sun.source.tree.Tree.Kind;
 import com.sun.source.tree.VariableTree;
 import com.sun.source.util.TreePath;
 import java.util.ArrayList;
@@ -272,18 +273,21 @@ public class ConvertToSwitchPatternInstanceOf {
         }
         Tree expression = ((ParenthesizedTree) switchTree.getExpression()).getExpression();
         Tree parent = (Tree) ctx.getPath().getParentPath().getLeaf();
-        int indexOf;
-        if (parent instanceof BlockTree) {
-            indexOf = ((BlockTree) parent).getStatements().indexOf(switchTree) - 1;
+        int indexOfSwitch;
+        if (parent.getKind() == Kind.BLOCK) {
+            indexOfSwitch = ((BlockTree) parent).getStatements().indexOf(switchTree);
+            if (indexOfSwitch < 1) {
+                return null;
+            }
         } else {
             return null;
         }
-        Tree ifTree = ((BlockTree) parent).getStatements().get(indexOf);
-        if ((!(ifTree instanceof IfTree) || !MatcherUtilities.matches(ctx, new TreePath(ctx.getPath(), ((IfTree) ifTree).getCondition()), "($expr0 == null)", true))
+        Tree ifTree = ((BlockTree) parent).getStatements().get(indexOfSwitch - 1);
+        if (ifTree.getKind() != Kind.IF || !MatcherUtilities.matches(ctx, new TreePath(ctx.getPath(), ((IfTree) ifTree).getCondition()), "($expr0 == null)", true)
                 || !(ctx.getVariables().get("$expr0").getLeaf().toString().equals(expression.toString()))) {
             return null;
         }
-        Fix fix = new FixSwitchPatternMatchToSwitchNull(ctx.getInfo(), ctx.getPath().getParentPath(), indexOf).toEditorFix();
+        Fix fix = new FixSwitchPatternMatchToSwitchNull(ctx.getInfo(), ctx.getPath().getParentPath(), indexOfSwitch).toEditorFix();
         return ErrorDescriptionFactory.forTree(ctx, ifTree, Bundle.ERR_ConvertToSwitchPatternInstanceOf(), fix);
 
     }
@@ -300,11 +304,11 @@ public class ConvertToSwitchPatternInstanceOf {
 
     private static final class FixSwitchPatternMatchToSwitchNull extends JavaFix {
 
-        private final int indexOf;
+        private final int indexOfSwitch;
 
-        public FixSwitchPatternMatchToSwitchNull(CompilationInfo info, TreePath path, int indexOf) {
+        public FixSwitchPatternMatchToSwitchNull(CompilationInfo info, TreePath path, int indexOfSwitch) {
             super(info, path);
-            this.indexOf = indexOf;
+            this.indexOfSwitch = indexOfSwitch;
         }
 
         @Override
@@ -318,9 +322,9 @@ public class ConvertToSwitchPatternInstanceOf {
             TreePath main = ctx.getPath();
             TreeMaker make = wc.getTreeMaker();
             List<Tree> caseNullLabel = new LinkedList<>();
-            SwitchTree switchTree = (SwitchTree) ((BlockTree) main.getLeaf()).getStatements().get(indexOf + 1);
+            SwitchTree switchTree = (SwitchTree) ((BlockTree) main.getLeaf()).getStatements().get(indexOfSwitch);
 
-            Tree ifTree = ((BlockTree) main.getLeaf()).getStatements().get(indexOf);
+            Tree ifTree = ((BlockTree) main.getLeaf()).getStatements().get(indexOfSwitch - 1);
             StatementTree thenStatement = ((IfTree) ifTree).getThenStatement();
             caseNullLabel.add(wc.getTreeMaker().Identifier("null"));
             BlockTree blockTree = (BlockTree)thenStatement;
@@ -328,7 +332,7 @@ public class ConvertToSwitchPatternInstanceOf {
             CaseTree caseMultipleSwitchPatterns = wc.getTreeMaker().CasePatterns(caseNullLabel, statementTree);
             SwitchTree insertSwitchCase = make.insertSwitchCase(switchTree, 0, caseMultipleSwitchPatterns);
             wc.rewrite(switchTree, insertSwitchCase);
-            BlockTree removeBlockStatement = make.removeBlockStatement((BlockTree) main.getLeaf(), indexOf);
+            BlockTree removeBlockStatement = make.removeBlockStatement((BlockTree) main.getLeaf(), indexOfSwitch - 1);
             wc.rewrite(main.getLeaf(), removeBlockStatement);
         }
     }
diff --git a/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/jdk/ConvertToSwitchPatternInstanceOfTest.java b/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/jdk/ConvertToSwitchPatternInstanceOfTest.java
index 4e25857d72..4d7f7a7a49 100644
--- a/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/jdk/ConvertToSwitchPatternInstanceOfTest.java
+++ b/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/jdk/ConvertToSwitchPatternInstanceOfTest.java
@@ -209,6 +209,24 @@ public class ConvertToSwitchPatternInstanceOfTest extends NbTestCase {
                         + "}\n");
     }
     
+    @Test
+    public void testSwitchWithNullExceptionSwitchIsFirst() throws Exception {
+        HintTest.create()
+                .input("""
+                       package test;
+                       public class Test {
+                           private static void test(Object o) {
+                               switch (o) {
+                                   case String s -> {}
+                               }
+                           }
+                       }
+                       """, false)
+                .sourceLevel("21")
+                .run(ConvertToSwitchPatternInstanceOf.class)
+                .assertWarnings();
+    }
+
     @Test
     public void testSimpleSwitchWithNullNoHint() throws Exception {
         HintTest.create()

 - index can be less than 0 in some situations
 - added test and code simplification suggestions from Jan Lahoda

Co-authored-by: Jan Lahoda <jan.lahoda@oracle.com>
@mbien mbien force-pushed the fix-switch-hint-ioobe branch from 1c9e957 to 0fa77ff Compare November 19, 2024 13:12
@mbien
Copy link
Copy Markdown
Member Author

mbien commented Nov 19, 2024

applied the patch and added @lahodaj as co-author

Copy link
Copy Markdown
Contributor

@lahodaj lahodaj left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@mbien mbien merged commit 7796670 into apache:master Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hints Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants