False Negative: IterableIterator.ql misses `iterator() == this` implementations once the guard logic is hidden behind helpers or trivial control flow.

Bắt đầu với checker Language Abuse/IterableIterator.ql và so sánh logic matching của nó với PosCase4_Var2.java, PosCase4_Var3.java và PosCase4_Var4.java. Cập nhật checker và phạm vi kiểm thử hồi quy để cả ba triển khai tự lặp đều bị đánh dấu dù có các lệnh gọi helper hoặc luồng điều khiển tầm thường, đồng thời giữ nguyên hành vi guard dự kiến.

False Negative: IterableIterator.ql misses iterator() == this implementations once the guard logic is hidden behind helpers or trivial control flow.

Version
codeql 2.24.3

Checker

• Checker id: Language Abuse/IterableIterator.ql
• Checker description: This checker detects classes that implement Iterable by returning themselves as the Iterator but lack a guard to prevent multiple concurrent iterations.

Description of the false negative

All three samples still implement the same dangerous pattern: iterator() returns this, the object is also its own Iterator, and hasNext() does not provide a real guard against repeated or concurrent iteration. The only changes are that this is returned through a helper or ternary expression, and hasNext() is dressed up with extra control flow.

Affected test cases

PosCase4_Var2.java

The class still returns itself as the iterator and still lacks a real reuse guard. The helper only hides that.

// A class implements Iterable, declares an iterator() method that returns "this", and any declared method named "hasNext" has a body with exactly one statement, which is not a return statement returning the boolean literal false should be flagged as lacking a guard against multiple concurrent iterations.
package scensct.var.pos;

import java.util.Iterator;

public class PosCase4_Var2 implements Iterable<Object>, Iterator<Object> {
    // Keep iterator returning this
    public Iterator<Object> iterator() {
        Iterator<Object> it = this;
        return it;
    }

    // hasNext with a try-catch that doesn't affect the single return statement
    public boolean hasNext() {
        try {
            return 1 < 2;
        } catch (Exception e) {
            throw new RuntimeException(e);
        }
    }

    // next with a dummy operation
    public Object next() {
        System.gc();
        return null;
    }
}

PosCase4_Var3.java

This is the same unsafe self-iterable pattern with one extra layer of indirection.

// A class implements Iterable, declares an iterator() method that returns "this", and any declared method named "hasNext" has a body with exactly one statement, which is not a return statement returning the boolean literal false should be flagged as lacking a guard against multiple concurrent iterations.
package scensct.var.pos;

import java.util.Iterator;

public class PosCase4_Var3 implements Iterable<Object>, Iterator<Object> {
    // Inline a helper method call in iterator
    public Iterator<Object> iterator() {
        return getSelf();
    }

    private Iterator<Object> getSelf() {
        return this;
    }

    // hasNext with a single statement that computes true via method call
    public boolean hasNext() {
        return checkHasNext();
    }

    private boolean checkHasNext() {
        return true;
    }

    public Object next() {
        return "dummy";
    }
}

PosCase4_Var4.java

The control-flow refactoring does not change the fact that repeated iteration is still unsafe.

// A class implements Iterable, declares an iterator() method that returns "this", and any declared method named "hasNext" has a body with exactly one statement, which is not a return statement returning the boolean literal false should be flagged as lacking a guard against multiple concurrent iterations.
package scensct.var.pos;

import java.util.Iterator;

public class PosCase4_Var4 implements Iterable<Object>, Iterator<Object> {
    // iterator returns this via a ternary operator (trivial)
    public Iterator<Object> iterator() {
        return (System.currentTimeMillis() > 0) ? this : this;
    }

    // hasNext with a single statement that is not a simple return false
    public boolean hasNext() {
        for (int i = 0; i < 1; i++) {
            return i == 0;
        }
        return false; // This line is unreachable, but the method body still has one reachable return
    }

    public Object next() {
        return Integer.valueOf(42);
    }
}

Cause analysis

The query appears to rely on a very direct return this / return false style match. Once either method is wrapped in a helper, a ternary, or a small control-flow construct, it stops recognizing the same iterator misuse pattern.

That is narrower than it should be. These implementations are still self-iterating objects without a real reentrancy guard.

References

None known.