Repository navigation
Conversation
…ot Coll[Byte] getNftContentLink converted R9 (or the first element of its pair form) with an unchecked cast to Coll and an unchecked byte conversion, and getNftCoverImageLink did the same for the pair's second element. R9 of a minting box is arbitrary chain data: a Coll[Long] R9 crashed the content getter with a ClassCastException from collToByteArray, and a pair whose link element is not a Coll (or not Coll[Byte]) crashed both getters the same way, instead of returning null as the @nullable contract promises for unavailable links. Both getters now route through collByteValueToStringOrNull, which applies the same element-type check getR7ByteArrayOrNull already uses for R7 (Coll whose item type is Byte) and returns null for any other shape. The check is per element, so a pair with one valid link element still returns that link. Valid Coll[Byte] links return byte-identical strings.
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 #268.
Problem
Eip4Token.getNftContentLink()converted R9 (or the first element of its pair form) with an unchecked cast toColland an unchecked byte conversion, andgetNftCoverImageLink()did the same for the pair's second element. R9 of a minting box is untrusted chain data, so an NFT-typed token with a malformed R9 crashed both getters withClassCastExceptioninstead of returningnullas their@Nullablecontract promises — e.g. aColl[Long]R9 fails insidecollToByteArray([J cannot be cast to [B), and a pair whose link element is aLongfails at the cast itself, even when the pair's other element is a perfectly valid link.Fail-first, on pristine
developplus only the new test:nftLinkWithMalformedR9Testfails withClassCastException: class [J cannot be cast to class [BatEip4Token.java:182on theColl[Long]R9 case; the valid-shape pins in the same test (directColl[Byte]link, pair ofColl[Byte]links) pass, as do the other 4 tests in the class.Fix
Both getters now route through
collByteValueToStringOrNull, which applies the same element-type checkgetR7ByteArrayOrNullalready uses for R7 — the value must be aCollwhose item type isByte— and returnsnullfor any other shape. The check is per element, so a pair with one valid link element still returns that link. ValidColl[Byte]links return byte-identical strings, including an empty one.Tests
New
Eip4TokenBuilderTest.nftLinkWithMalformedR9Testpins the two valid shapes and four malformed ones (Coll[Long]R9;(Long, Coll[Byte]);(Coll[Byte], Long);(Coll[Long], Coll[Long])), assertingnullfor each malformed element and the surviving valid link for the mixed pairs.Verification:
Eip4TokenBuilderTest5/5 post-fix per itsTEST-*.xmlreport (tests=5, failures=0, including the live explorer test); fullsbt testgreen across all modules (common 18/18, lib-impl 4 passed / 1 skipped, appkit 96/96).Note: this PR, #265 and #267 all touch
Eip4Token.java, in disjoint areas (the R9 link getters here,getNftContentHashin #265, the constructor at the top of the file in #267), so they should apply in any order.Submitted by @Kshot9000 (https://x.com/kshot9000) · Tips: ERG 9fcM5RWnAjmP4vx5bnW6yohB6H9bLq8sJbaPLHtwZLtQPB32Pvy