common: json_to_s64 accept only valid JSON numbers

[guibsa]

bool str_to_s64 parses a decimal s64 from exactly buflen bytes; false on bad chars, overflow, leading +, hex like '0x...' . Similar behavior of json_to_u64 and str_to_u64. Tests added. Partially fixes #9377

Important

26.09 FREEZE August 5th: Non-bugfix PRs not ready by this date will wait for 26.12.

RC1 is scheduled on August 17th

The final release is scheduled for September 7th.

Checklist

Before submitting the PR, ensure the following tasks are completed. If an item is not applicable to your PR, please mark it as checked:

• The changelog has been updated in the relevant commit(s) according to the guidelines.
• Tests have been added or modified to reflect the changes.
• Documentation has been reviewed and updated as needed.
• Related issues have been listed and linked, including any that this PR closes.
• Important All PRs must consider how to reverse any persistent changes for tools/lightning-downgrade

[Andezion]

run-utils-str_to_s64.c has no direct test like assert(!str_to_s64("+123", 3, &val)) or assert(!str_to_s64("0x1A", 4, &val)). it only tests --123 and +-123 (mixed/double signs), which is a different case, maybe worth adding something like +123 and a hex string

[Andezion]

Nice!

[Andezion]

common/test/run-utils-str_to_s64.c has #include <assert.h> and then #include <assert.h> and #include <stdint.h> again right after /* AUTOGENERATED MOCKS END */. <stdint.h> also looks unused? The test uses s64 and LL/ULL literals, not int64_t/INT64_MIN directly

[Markadrian6399]

I'm reading through this to understand the change better, so please bear with the questions.

If I'm following correctly, the old json_to_s64 copied the token into a temp buffer and handed it to strtoll with base 0, which meant hex, octal, leading whitespace and a leading + all got through. This PR replaces that with a hand written str_to_s64 that walks the buffer digit by digit and rejects anything that isn't a plain decimal, matching how str_to_u64 already behaves. Is that a fair summary of the intent?

A few places I'd appreciate more context on:

1. The negative branch uses val < (INT64_MIN + digit) / 10 and builds the result as val * 10 - digit. My understanding is that accumulating negatively is what lets INT64_MIN be reached, since negating a positive magnitude at the end would overflow on that one value. Is that the reasoning, or is there something else it's protecting against?

2. Leading zeros are accepted ("000123" and the 21 zero case in the tests). JSON itself doesn't allow those, so I assume this is a deliberate choice to stay consistent with str_to_u64 rather than an oversight. Could you confirm?

3. Since the old parser was more permissive, is there a risk of breaking existing callers that were passing values like +5 or 0x10 to an s64 RPC field? I wasn't sure whether those paths exist in tree or whether they'd already have been rejected earlier by jsmn.

4. The issue mentions json_to_double as well. Is that intentionally out of scope here because floats need different handling, or is it queued for a follow up?

5. Minor process question: the checklist marks the changelog as done, but I couldn't find a Changelog- line in the commit message. Is one expected for a change that tightens accepted input, or is that only for user visiblI'm reading through this to understand the change better, so please bear with the questions.

If I'm following correctly, the old json_to_s64 copied the token into a temp buffer and handed it to strtoll with base 0, which meant hex, octal, leading whitespace and a leading + all got through. This PR replaces that with a hand written str_to_s64 that walks the buffer digit by digit and rejects anything that isn't a plain decimal, matching how str_to_u64 already behaves. Is that a fair summary of the intent?

A few places I'd appreciate more context on:

1. The negative branch uses val < (INT64_MIN + digit) / 10 and builds the result as val * 10 - digit. My understanding is that accumulating negatively is what lets INT64_MIN be reached, since negating a positive magnitude at the end would overflow on that one value. Is that the reasoning, or is there something else it's protecting against?

2. Leading zeros are accepted ("000123" and the 21 zero case in the tests). JSON itself doesn't allow those, so I assume this is a deliberate choice to stay consistent with str_to_u64 rather than an oversight. Could you confirm?

3. Since the old parser was more permissive, is there a risk of breaking existing callers that were passing values like +5 or 0x10 to an s64 RPC field? I wasn't sure whether those paths exist in tree or whether they'd already have been rejected earlier by jsmn.

4. The issue mentions json_to_double as well. Is that intentionally out of scope here because floats need different handling, or is it queued for a follow up?

5. Minor process question: the checklist marks the changelog as done, but I couldn't find a Changelog- line in the commit message. Is one expected for a change that tightens accepted input, or is that only for user visible behaviour?