xai-org / xai-org/x-algorithm

BotMaker collection plus same defect class in Last()

Open
#207 0 comments 0 reactions 0 assignees View on GitHub

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:

Image
  • 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

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.