False Positive: IterableIterator.ql reports classes whose `hasNext()` still reliably disables iteration.
- 主要言語
- CodeQL
- スター
- 10.1k
- フォーク
- 2.1k
- 平均マージ
- 2日 15時間
- マージ済み PR(30日)
- 141
説明
# False Positive: IterableIterator.ql reports classes whose `hasNext()` still reliably disables iteration.
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 positive
These classes do return `this` from `iterator()`, but `hasNext()` still deterministically returns `false`, which is exactly the built-in guard that keeps iteration from proceeding. The refactoring only changes how that `false` result is computed.
## Affected test cases
### `NegCase6_Var2.java`
`hasNext()` still disables reuse of the iterator instance in practice, so this should not be reported as an unsafe self-iterable.
```java
// A concrete class that implements Iterable, returns "this" in iterator(), and has hasNext() returning false should not be flagged.
package scensct.var.neg;
import java.util.Iterator;
public class NegCase6_Var2 implements Iterable, Iterator { // [REPORTED LINE]
public Iterator iterator() {
return this;
}
private boolean neverHasNext() {
return false;
}
public boolean hasNext() {
return neverHasNext();
}
public Double next() {
return 0.0;
}
}
```
### `NegCase6_Var4.java`
The iteration guard is still present even though the control flow is slightly different.
```java
// A concrete class that implements Iterable, returns "this" in iterator(), and has hasNext() returning false should not be flagged.
package scensct.var.neg;
import java.util.Iterator;
public class NegCase6_Var4 implements Iterable, Iterator { // [REPORTED LINE]
private final boolean NO_MORE = false;
public Iterator iterator() {
return this;
}
public boolean hasNext() {
for (int i = 0; i < 1; i++) {
// loop does nothing
}
return NO_MORE;
}
public Double next() {
return 0.0;
}
}
```
## Cause analysis
The query appears too literal about what counts as a valid guard. Once `hasNext()` returns `false` via a helper or a constant field instead of a bare literal, the class is still reported.
That is overly rigid. The safety property here is semantic: iteration is disabled, regardless of whether `false` is returned directly or indirectly.
## References
None known.
コントリビューションガイド
調査の方向性
Language Abuse/IterableIterator.ql checker と、影響を受ける NegCase6_Var2.java および NegCase6_Var4.java のテストケースから始めます。helper または定数を介して false を返す場合に hasNext() がどのように認識されるかを追跡し、そのうえで checker のテストを更新して、これらのケースが報告されないようにしつつ、安全でない self-iterable ケースは引き続きカバーされるようにします。
索引モデルが issue の本文から書いたものです。
評価
- 技術スタック
- java
- 領域
- security, testing-qa
- issue の種類
- バグ
- 難易度
- 3/5
- 見積もり時間
- 1〜2日
- 活発さ
- 静か
- 明瞭さ
- おおむね明確
- 初心者へのやさしさ
- 55/100