Skip to content

fix: prevent IEEE 754 precision loss for large acquirer BIN values #4939

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
itsaryanchauhan wants to merge 1 commit into
base: main
Choose a base branch
from
+15 −11
Open

fix: prevent IEEE 754 precision loss for large acquirer BIN values #4939

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 15 additions & 11 deletions ...eveloper/PaymentSettingsRevamped/MerchantAcquirerDetails/MerchantAcquirerDetailsUtils.res
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,16 @@ let binField = makeFieldInfo(
~label="Acquirer BIN",
~name="acquirer_bin",
~placeholder="e.g. 56688",
~customInput=InputFields.numericTextInput(~removeLeadingZeroes=true, ~maxLength=20, ~precision=0),
~customInput=(~input: ReactFinalForm.fieldRenderPropsInput, ~placeholder) => {
let handleChange = event => {
let value = ReactEvent.Form.target(event)["value"]
let filteredValue = value->String.replaceRegExp(%re("/[^0-9]/g"), "")
input.onChange(filteredValue->Identity.stringToFormReactEvent)

@XyneSpaces XyneSpaces Jun 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔍 Regex runs on every keystroke — consider optimization

While the %re("/[^0-9]/g") regex is compiled at build time, it still executes on every change event. For consistency with the codebase's functional style, consider:

let filteredValue = value->String.split("")->Array.filter(c => c >= "0" && c <= "9")->Array.join("")

Or if regex is preferred, lift it to module scope to avoid reallocation.

@XyneSpaces XyneSpaces Jun 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

⚠️ Empty string edge case after filtering allows invalid submission

If the user types only non-digit characters (e.g., "abc"), the regex filter strips them all, resulting in an empty string. This passes both validation checks:

  • requiredKeys check passes because "" is technically a present value in the dict
  • BIN length check is skipped because isNonEmptyString returns false

The form would submit with an empty BIN despite the field being marked as required.

Fix: Either validate the filtered result is non-empty before setting, or ensure empty string after filtering fails the required check:

let filteredValue = value->String.replaceRegExp(%re("/[^0-9]/g"), "")
if filteredValue->isNonEmptyString || value->isEmptyString {
  // Only update if we have digits OR the original was already empty
  input.onChange(filteredValue->Identity.stringToFormReactEvent)
}

Or move the BIN length validation outside the empty check in validateForm.

}
<TextInputAdapter
input={...input, onChange: handleChange} placeholder autoComplete="off" maxLength=20

@XyneSpaces XyneSpaces Jun 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

💡 Identity conversion obscures intent

The ->Identity.stringToFormReactEvent conversion wraps a string in a fake event structure just to satisfy the type system. If input.onChange accepts the raw string in other contexts, consider passing filteredValue directly for clarity.

If the type system strictly requires an event-shaped object, add a brief inline comment explaining why this conversion is necessary.

/>
},
~isRequired=true,
)

Expand Down Expand Up @@ -120,7 +129,7 @@ let stampProfileId = (body: Dict.t<JSON.t>, ~profileId: string) => {
}

let normalizeNumericStringFields = (body: Dict.t<JSON.t>) => {
[AcquirerBin, AcquirerIca]->Array.forEach(field => {
[AcquirerIca]->Array.forEach(field => {
let key = (field :> string)
let value = body->getFloat(key, 0.0)
if value > 0.0 {
Expand All @@ -137,23 +146,18 @@ let validateForm = (~requiredKeys: array<acquirerField>, values: JSON.t): JSON.t

requiredKeys->Array.forEach(field => {

@XyneSpaces XyneSpaces Jun 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

⚠️ Simplified validation logic should verify edge cases

The validation change from getOptionFloat to getString for AcquirerBin assumes the value will always be a string in the form. Verify that:

  1. Form initialization doesn't leave the field absent from the dict
  2. Backend submission still receives the value as a string, not a number

This change is likely safe given the text input, but confirm the form state initialization sets an empty string default for acquirer_bin.

let key = (field :> string)
let present = switch field {
| AcquirerBin => valuesDict->getOptionFloat(key)->Option.isSome
| _ => valuesDict->getString(key, "")->isNonEmptyString
}
let present = valuesDict->getString(key, "")->isNonEmptyString
if !present {
setErr(key, "This field is required")
}
})

valuesDict
->getOptionFloat((AcquirerBin :> string))
->mapOptionOrDefault((), binFloat => {
let binStr = binFloat->Float.toString
let binStr = valuesDict->getString((AcquirerBin :> string), "")
if binStr->isNonEmptyString {
if binStr->String.length < 4 || binStr->String.length > 20 {
setErr((AcquirerBin :> string), "Acquirer BIN must be between 4 and 20 digits")
}
})
}

valuesDict
->getOptionFloat((AcquirerFraudRate :> string))

@XyneSpaces XyneSpaces Jun 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔍 Validation only runs on non-empty strings

If the user types only non-digit characters, the filtered value becomes empty, and this length validation is skipped entirely. Combined with the requiredKeys check using getString (which returns "" for missing keys), this creates a gap where an all-non-digit input could slip through validation.

Consider consolidating the BIN presence and length validation into a single check:

let binStr = valuesDict->getString((AcquirerBin :> string), "")
switch binStr->String.length {
| 0 => setErr((AcquirerBin :> string), "This field is required")
| n if n < 4 || n > 20 => setErr((AcquirerBin :> string), "Acquirer BIN must be between 4 and 20 digits")
| _ => ()
}

Expand Down
Loading