Skip to content

fix: skip null SLEs in directory item iteration - #7723

Open
BraedonKlock wants to merge 1 commit into
XRPLF:developfrom
BraedonKlock:fix-7512-null-sle-foreachitemafter
Open

fix: skip null SLEs in directory item iteration#7723
BraedonKlock wants to merge 1 commit into
XRPLF:developfrom
BraedonKlock:fix-7512-null-sle-foreachitemafter

Conversation

@BraedonKlock

Copy link
Copy Markdown

High Level Overview of Change

This PR adds defensive null checks when reading child ledger entries during directory item iteration.

Specifically, forEachItem and forEachItemAfter now verify that view.read(keylet::child(key)) returns a valid SLE before passing it to the callback.

Fixes #7512.

Context of Change

view.read(keylet::child(key)) can theoretically return nullptr if a directory contains a dangling reference to a missing ledger entry. Under normal ledger operation this should not occur, because directory entries and their referenced objects are maintained atomically.

However, if ledger corruption or an internal invariant violation ever produced a dangling directory entry, the previous implementation could pass a null SLE to callback code. Some callbacks dereference the received SLE, which could result in a crash.

This change adds defense-in-depth by skipping null child entries inside the shared directory iteration helpers before callbacks are invoked.

API Impact

  • Public API: New feature (new methods and/or new fields)
  • Public API: Breaking change (in general, breaking changes should only impact the next api_version)
  • libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
  • Peer protocol change (must be backward compatible or bump the peer protocol version)

Before / After

Before this change, forEachItem and forEachItemAfter could pass the result of view.read(keylet::child(key)) directly to the callback without checking whether the read succeeded.

After this change, null child SLE values are skipped .

@ximinez

ximinez commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Hi @BraedonKlock. Thank you for your contributions! We really appreciate them. At this point, we ask that you hold off on submitting any more until we've had a chance to review the current batch. There is a not insignificant engineer overhead for each review, and it may take some time to get caught up. Too many PRs can make that harder. When we've gotten down to 3 open or fewer, feel free to create more!

In the meantime, you can open "Draft" PRs so that changes will be ready to go once we're ready for more. At that time, you'll be able to decide which ones are the best, and convert them to "Ready to review". Keep in mind that you will be responsible for keeping your PRs up to date with develop. The more you open, the more chances you'll have for conflicts, so don't get too carried away. 😄

@BraedonKlock

Copy link
Copy Markdown
Author

Thank you @ximinez for the clarification! I really appreciate the feedback. I completely understand. I'll hold off on opening additional ready for review PRs until my open count is down to three or fewer. In the meantime, I'll continue working locally and use draft PRs if appropriate. Thanks again!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Null SLE Deref in forEachItemAfter

2 participants