Handle malformed XDR in the address conversion example - #2849
Open
devtechedge wants to merge 2 commits into
Open
Handle malformed XDR in the address conversion example#2849devtechedge wants to merge 2 commits into
devtechedge wants to merge 2 commits into
Conversation
Address::from_xdr is fallible, so the smart-contract example now returns Result<Address, ConversionError> instead of unwrapping, and the prose notes that untrusted bytes should be handled at the trust boundary. Fixes stellar#2768
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the address-conversion guide to promote fallible XDR decoding at trust boundaries.
Changes:
- Returns
Result<Address, ConversionError>. - Adds malformed-input guidance.
- However,
FromXdritself still panics on malformed XDR, so the issue remains unresolved.
Recommendation: NEEDS-CHANGES — provide a genuinely non-panicking decoding path or accurately document the limitation.
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…xample FromXdr::from_xdr returns Err only when the bytes deserialize to a valid ScVal of a different type. Bytes that are not valid ScVal XDR at all panic before a Result is produced, and the SDK offers no in-contract recovery for that case. The prose now documents both failure modes and recommends validating untrusted input before it reaches the contract, or preferring typed arguments such as Address.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (1)
docs/build/guides/conversions/address-conversions.mdx:125
- This still does not handle the malformed-XDR case named in the PR and issue. In the current SDK,
from_xdrcallsdeserialize_from_bytes(...).unwrap_infallible(), so malformed bytes panic before thisResultcan be returned; only validScValXDR of the wrong type producesConversionError. Client-side validation also cannot protect a public contract boundary because another caller can bypass it. Please either restrict this helper and its guidance to trusted bytes (requiring a typedAddressfor caller-controlled input), or show a complete in-contract validation strategy before decoding.
pub fn address_from_xdr_bytes(env: Env, bytes: Bytes) -> Result<Address, ConversionError> {
Address::from_xdr(&env, &bytes)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2768.
Summary
The smart-contract example on the address conversions guide called
Address::from_xdr(&env, &bytes).unwrap(), which panics when the bytes do not contain a validAddressvalue.This PR changes the example to return the fallible result directly (
Result<Address, ConversionError>) and adds a short note in the surrounding prose: contracts that receive XDR bytes from untrusted sources, such as a custom authentication scheme, should handle the error case instead of unwrapping, since malformed input is a normal condition at a trust boundary.Testing
Docs-only change to one guide page. The Rust snippet mirrors the
FromXdrtrait signature insoroban-sdk(fn from_xdr(env: &Env, b: &Bytes) -> Result<Self, Self::Error>, withAddress's conversion error beingConversionError).