Reject amount strings with trailing or invalid characters - #44
Open
Bornoz wants to merge 1 commit into
Open
Conversation
parseAmount stripped formatting characters and then called parseFloat, which parses a leading numeric prefix and ignores the rest. So "10abc" parsed as 10 and "1.25xyz" as 1.25 — a malformed string was silently accepted as a smaller-looking but different value. "1e5" parsed as 100000. For monetary values used in deposit and refund flows this is the wrong failure mode. Require the cleaned string to be a plain decimal in its entirety before parsing, so anything with trailing or non-numeric characters is rejected instead. Valid formatted inputs like $1,000.50 still parse to 1000.5. Closes circlefin#38
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
1 participant
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.
parseAmountstripped formatting characters and then calledparseFloat, which parses a leading numeric prefix and ignores the rest. So malformed strings were silently accepted as different values:10abc101.25xyz1.251e5100000101010$1,000.501000.51000.5Since
parseAmountfeeds deposit and refund flows, silently recording a different amount is the wrong failure mode for monetary values. The cleaned string is now required to be a plain decimal in its entirety before parsing.Closes #38