Skip to content

decoder: raise default max depth to 1500 and add DecodeUnlimitedDepth#32

Merged
urvisavla merged 1 commit into
masterfrom
decoder-unlimited-depth
May 29, 2026
Merged

decoder: raise default max depth to 1500 and add DecodeUnlimitedDepth#32
urvisavla merged 1 commit into
masterfrom
decoder-unlimited-depth

Conversation

@urvisavla
Copy link
Copy Markdown

What

  • Raise DecodeDefaultMaxDepth from 250 → 1500. This is the bound applied to untrusted, user-supplied XDR (anyone using NewDecoder/Unmarshal or MaxDepth: 0).
  • Add a new sentinel DecodeUnlimitedDepth = uint(math.MaxUint). Callers decoding trusted XDR (e.g. emitted by stellar-core) can set DecodeOptions{MaxDepth: xdr3.DecodeUnlimitedDepth} to disable the depth limit.

Why

The previous default of 250 was too shallow for some legitimately deep XDR. User-supplied input still needs a guard against unbounded recursion / stack growth, so the default is raised to 1500 rather than removed. Trusted core output, which is acyclic and finite but can nest deeply, gets an explicit opt-out.

Usage

// User-supplied / untrusted XDR — capped at 1500
xdr3.Unmarshal(r, &v)

// Trusted XDR emitted by stellar-core — no depth limit
xdr3.UnmarshalWithOptions(r, &v, xdr3.DecodeOptions{
    MaxDepth: xdr3.DecodeUnlimitedDepth,
})

Implementation notes

  • 0 already means "use the default", so a separate sentinel was needed for "unlimited". math.MaxUint reuses the existing countdown logic untouched — no hot-path change — and is effectively unbounded for any real nesting.
  • Trade-off: DecodeUnlimitedDepth is not truly unbounded; a genuinely cyclic/adversarial input would no longer be caught by the depth check. That's intended, since this knob is only for trusted input (documented on the constant).

Testing

  • Added TestDecodeUnlimitedDepth: builds a chain nested 100 levels past the default, confirms the default limit rejects it (ErrMaxDecodingDepth) and DecodeUnlimitedDepth decodes it cleanly.
  • Full suite green across xdr, xdr2, xdr3.

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings May 29, 2026 17:54
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Raises the default XDR decoding depth limit in xdr3 from 250 to 1500 and introduces a DecodeUnlimitedDepth sentinel (math.MaxUint) for callers decoding trusted XDR (e.g., stellar-core output) where deep nesting is expected.

Changes:

  • Raise DecodeDefaultMaxDepth from 250 to 1500 and expand its doc comment.
  • Add new DecodeUnlimitedDepth constant and document it in DecodeOptions.MaxDepth.
  • Add TestDecodeUnlimitedDepth covering both the default-limit rejection and the unlimited-depth success path via a self-referential linkedNode type.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
xdr3/decode.go Bumps default max depth to 1500 and adds DecodeUnlimitedDepth sentinel with docs on MaxDepth.
xdr3/decode_test.go Adds linkedNode helper and TestDecodeUnlimitedDepth exercising default-limit failure and unlimited-depth success.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

The previous default decoding depth of 250 was too shallow for some
legitimately deep XDR. Raise the default to 1500 for untrusted,
user-supplied input, and add a DecodeUnlimitedDepth sentinel
(math.MaxUint) so callers decoding trusted XDR emitted by stellar-core
can disable the limit entirely.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@urvisavla urvisavla force-pushed the decoder-unlimited-depth branch from baf18dc to a00ebd7 Compare May 29, 2026 17:57
@urvisavla urvisavla merged commit 0bf8f49 into master May 29, 2026
8 checks passed
@urvisavla urvisavla deleted the decoder-unlimited-depth branch May 29, 2026 21:08
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.

4 participants