Skip to content

BotMaker collection plus same defect class in Last() #207

Description

@Alb4don

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 BotMaker collection functions crash on edge cases: missing validation for empty lists and invalid indices #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));
      }

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions