BotMaker collection plus same defect class in Last()
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 33.3k
- Forks
- 5.4k
- Avg merge
- 1d 17h
- Merged PRs (30d)
- 1
Description
This is a follow-up to #58, which reported three unvalidated collection helpers in the BotMaker runtime. I re checked current main while verifying them I found a fourth instance of the same defect class in Last(), which that issue did not cover.
-
Affected:
botmaker/src/java/com/twitter/botmaker/function/collection/First.java, Last.java, FirstN.java, Slice.java -
New finding: Last()
// Last.java (~L49) return list.get(list.size() - 1); -
On an empty collection this resolves to get(-1) and throws IndexOutOfBoundsException.
-
Same root cause as First(), one function later in the file, not mentioned in #58.
-
FunctionNode1.toExtractor() wraps the exception in a FunctionFailure so that the exception does not halt the worker; however, the enclosing rule does not complete, meaning within the scarecrow flow that the intended label is never applied.
-
Based on the code, I could not determine whether the calling component treats this as fail-open (label ignored for that event) or fail-closed (event reprocessed), as the scarecrow dispatch layer and part of the provided rule set are not included in this repository.
-
This is the specific point I would ask the maintainers to clarify, as it determines whether the practical impact is a robustness issue or a vector for bypassing labeling.
-
I compiled the four files from main against minimal stubs of the internal dependencies (com.twitter.util.Future, the annotation processor types, etc.) and drove each crash condition directly:
-
The harness downloads the four files from raw.githubusercontent.com, compiles them unmodified, and exits non zero when any trigger fails to crash
-
Suggested fix
Validate at the function boundary and surface a typed FunctionFailure with a descriptive message, rather than letting the JDK throw unadorned:
if (list == null || list.isEmpty()) {
throw new FunctionFailure(this, context.getStackFrames(),
new IllegalArgumentException("First() called on empty list"));
}
long count = Math.max(0L, n);
return list.subList(0, (int) Math.min(count, list.size()));
int max = input instanceof String ? ((String) input).length() : ((List<?>) input).size();
if (beginIndex < 0 || endIndex < 0 || beginIndex > endIndex || endIndex > max) {
throw new IllegalArgumentException(
String.format("Invalid slice indices: begin=%d, end=%d, length=%d", beginIndex, endIndex, max));
}
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Read First.java, Last.java, FirstN.java, and Slice.java, then trace FunctionNode1.toExtractor() to understand how failures are surfaced. Reproduce the empty-collection and invalid-range cases with the described harness; done means the maintainers’ chosen boundary validation is applied consistently and failures have descriptive handling without leaving the rule incomplete.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100