Skip to content

fix: fromDecimal Auto delimiter now correctly parses multiple numbers#2270

Open
min23asdw wants to merge 9 commits intogchq:masterfrom
min23asdw:fix/from-decimal-auto-delim
Open

fix: fromDecimal Auto delimiter now correctly parses multiple numbers#2270
min23asdw wants to merge 9 commits intogchq:masterfrom
min23asdw:fix/from-decimal-auto-delim

Conversation

@min23asdw
Copy link
Copy Markdown

@min23asdw min23asdw commented Mar 21, 2026

Issue

Fixes #2217
Ref: #2221 (closed due to unsigned CLA)

Description

fromDecimal() did not correctly handle the delim parameter when set to "Auto".

Root cause: Utils.charRep("Auto") returns undefined (no "Auto" entry in the mapping), so data.split(undefined) returns the entire string as a single array element — only the first number gets parsed.

Fix: When delim === "Auto", split on regex /[^\d-]+/ (any non-digit, non-minus characters), consistent with how fromHex() handles its Auto delimiter. Existing delimiter behavior is unchanged.

Changes

  • src/core/lib/Decimal.mjs — Auto delimiter uses regex split instead of Utils.charRep
  • tests/operations/tests/FromDecimal.mjs — Added 5 test cases for Auto delimiter (space, comma, mixed, newline, signed values)

Before / After

Input Before After
"72 101 108 108 111" (Auto) [72]"H" [72,101,108,108,111]"Hello"
"72,101,108,108,111" (Auto) [72]"H" [72,101,108,108,111]"Hello"
"72, 101 : 108; 108\t111" (Auto) [72]"H" [72,101,108,108,111]"Hello"

Verification

  • npx grunt lint — all tasks pass
  • All existing and new test cases pass

Note on edge cases

Two edge cases with malformed input were tested — neither is a security concern:

Input Output Reason
"--1" [NaN] Double minus is not a valid number — parseInt("--1") returns NaN. This is consistent with garbage-in, garbage-out behavior.
"72;alert(1);101" [72, 1, 101] The regex treats alert( as a delimiter, so the 1 inside the parentheses is parsed as a number. No code execution occurs — fromDecimal only produces an integer array via parseInt. This matches fromHex Auto behavior.

▎ This pull request was created with the assistance of Claude and Antigravity, to suggest ideas on where it might be broken and to help write the tests. then searched online to verify whether those suggestions were correct.
I have reviewed and tested all code changes. All changes were manually verified.

min23asdw and others added 3 commits March 22, 2026 02:00
When delim is "Auto", Utils.charRep("Auto") returns undefined,
causing data.split(undefined) to return the entire string as a
single element. This meant only the first number was parsed.

Fix by splitting on a regex /[^\d-]+/ for Auto mode, consistent
with how fromHex handles its Auto delimiter. Existing delimiter
behavior is unchanged.

Adds test cases for Auto delimiter with space, comma, and mixed
separators.

Fixes gchq#2217
Ref: gchq#2221 (closed due to unsigned CLA)
Covers additional edge cases for the Auto delimiter fix:
- newline-separated input
- signed (negative) values with Auto delimiter

Ref: gchq#2217
Copy link
Copy Markdown
Contributor

@GCHQDeveloper581 GCHQDeveloper581 left a comment

Choose a reason for hiding this comment

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

We're currently having some internal discussions on how/whether we accept AI created/assisted submissions, so there may be a delay in responding further if/once my other comment is addressed.

Comment thread src/core/lib/Decimal.mjs Outdated
@min23asdw
Copy link
Copy Markdown
Author

min23asdw commented Mar 23, 2026

I add "Auto" (not default) mode in dropdown's From Decimal. Without this, the Auto regex in fromDecimal() is unreachable
since the UI never passes "Auto" as a delimiter value.

2026-03-23.16-12-29.mp4

@min23asdw
Copy link
Copy Markdown
Author

@GCHQDeveloper581 would you mind if it shold have 'FROM_DECIMAL_DELIM_OPTIONS'
https://github.com/gchq/CyberChef/pull/2270/changes#diff-758aa870501726a364e22480816d2b39af0509502254f69a8258c25aca760b17R44

becase if it don't have #2270 (comment) 'auto' mode it will can't not access by UI,

cc #2308

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug report: fromDecimal(...) function does not correctly handle delim parameter passed as default ("Auto")

2 participants