[v92-audit] increx: BYFLOAT reply is a RESP3 Double, so out-of-range results reach clients as inf while the stored value is finite

[madolson]

INCREX ... BYFLOAT computes in long double but replies with addReplyHumanLongDouble(), which emits a RESP3 Double. Results outside IEEE-754 double range are reachable (long double holds ~1.1e4932), so a RESP3 client that maps Doubles to float64 reads inf for a value the server stored as a finite 309-digit string, and the same command over RESP2 returns that string exactly. This also contradicts the command's own contract, which explicitly refuses to store an infinite result at src/t_string.c:887. INCRBYFLOAT avoids this by replying with a bulk string in both protocols (src/t_string.c:792).

Details

Repro

Clean server at 3f9062e, no config beyond defaults.

SET big 1e308
INCREX big BYFLOAT 1e308
GET big

Raw bytes, same two commands on two connections:

=== RESP2 ===
SET      -> b'+OK\r\n'
INCREX   -> b'*2\r\n$309\r\n1999999999999999999933717593116912913211201996948311344155940959898434' ... len=637
GET      -> b'$309\r\n199999999999999999993371759311691291321120199694831134' ... len=317
  stored digits: b'1999999999999999999933717593116912913211' len 309

=== RESP3 ===
SET      -> b'+OK\r\n'
INCREX   -> b'*2\r\n,199999999999999999993371759311691291321120199694831134415594095989843469737' ... len=627
  first element token=b',1999999999999999999' (310 bytes)
  float64(payload) = inf
GET      -> b'$309\r\n199999999999999999993371759311691291321120199694831134' ... len=317
  stored digits: b'1999999999999999999933717593116912913211' len 309

RESP2 gives $309 followed by the exact value. RESP3 gives , followed by 309 digits, which is the RESP3 Double type. float64 of that payload is inf, so the reply says the counter is infinite while GET says it is finite.

Ordinary inputs lose precision by the same mechanism, without going out of range. long double carries 18-19 significant digits and float64 carries 15-17:

SET k 1e10
INCREX k BYFLOAT 0.1   ->  ,10000000000.09999999962747097
GET k                  ->  10000000000.09999999962747097

A RESP3 client reading that Double gets 10000000000.1. Feeding the reply value back into the key corrupts it. Under RESP2 the client gets the 28-character string and round-trips cleanly.

Why

increxCommand() replies through addReplyHumanLongDouble() at src/t_string.c:959-960 (and at :850, :864-865, :889-890, :915-916). In RESP3 that function emits the value as a , Double; in RESP2 it emits createStringObjectFromLongDouble(ld, 1) as a bulk string. The two encodings do not have the same range or precision, and the value being encoded is a long double.

The command already treats an infinite result as unrepresentable and refuses it:

/* src/t_string.c:887 */
if (isinf(value_ld)) {
    addReplyArrayLen(c, 2);
    addReplyHumanLongDouble(c, oldvalue_ld);
    addReplyHumanLongDouble(c, 0);
    return;
}

So the RESP3 encoding manufactures exactly the value the command guarantees it will never produce.

INCRBYFLOAT does not have this problem. It replies with the stored object as a bulk string, identically in both protocols:

/* src/t_string.c:792 */
addReplyBulk(c, new);

Before INCREX, addReplyHumanLongDouble() had only two core callers, GEOPOS and GEORADIUS ... WITHCOORD (src/geo.c:817-818, src/geo.c:981-982). Both encode coordinates that originate as double, so they always fit. INCREX is the first core command to put an arbitrary long double on the wire as a RESP3 Double.

Introduced by

6ae7eb4, "Introduce INCREX command (#3253)".

Decisions for a reviewer

1. Reply with the stored string instead, matching INCRBYFLOAT: addReplyBulk(c, new) for the value and a bulk string for the delta. Lossless in both protocols, and it removes the second copy of the formatting logic, since the reply then comes from the same object that was stored. Proposed.
2. Keep the Double and clamp the arithmetic to IEEE-754 double range, rejecting anything larger. Changes accepted inputs and still loses the 18th and 19th significant digits, so it does not fix the ordinary-input case above.
3. Document RESP3 as lossy for this command. Leaves the same command returning different values on different protocols.

reply_schema in src/commands/increx.json declares both elements as "type": "number", which matches the RESP3 Double but not the RESP2 bulk string, so it needs updating under option 1.

Not a defect

Ruled out as pre-existing rather than INCREX-specific: long double arithmetic producing 300-plus digit stored values, %.17Lf formatting, and SET k inf being accepted by string2ld. INCRBYFLOAT behaves the same way on all three.

Related

#34 also concerns INCREX BYFLOAT long-double behavior, but is about test flakiness under valgrind, not the reply encoding.

Testing

The RESP2/RESP3 asymmetry is deliberate and already asserted. tests/unit/type/incr.tcl:479 checks
,2.5 under RESP3 and tests/unit/type/incr.tcl:507 checks $3 / 2.5 under RESP2, with the comment
"BYFLOAT degrades to bulk strings under RESP2". So this is a design objection, not something the
authors missed.

What is untested is the range where the two encodings stop agreeing. Every BYFLOAT assertion uses
2.5, 1.5 or 0, all of which round-trip through float64 exactly. No test uses a value needing
more than 17 significant digits, and none uses a value outside IEEE-754 double range, which is where
the RESP3 reply becomes inf.

This was generated by AI but verified, with love, by a human.