lizhimins commented on PR #6490:
URL:
https://github.com/apache/rocketmq-dashboard/pull/6490#issuecomment-6190305972
Closing in favour of #6439, which is now merged into `rocketmq-studio` as
`eeaa90b91`. The bug is real and your description of it is accurate:
`restoreNumbers` at `rmqctl/internal/output/output.go:66-91` decoded every
`json.Number` through `Int64()`/`Float64()`, so `18446744073709551615` became
`1.8446744073709552e+19`, `0.1234567890123456789` lost its tail, and a literal
past float64 range never even reached the string fallback because `Float64()`
returns ±Inf with a nil error. Wrapping the literal in a tagged `yaml.Node` is
the right idea and matches what we merged elsewhere. We are closing this as a
duplicate of **#6439**, which fixes the same issue (#6437) on the same lines
and conflicts with this branch. Three reasons, in order of weight. (1) **This
branch fails CI as written**: `"regexp"` is inserted at
`rmqctl/internal/output/output.go:20`, ahead of `"encoding/json"`, and gofmt
sorts each import group — so the `Check formatting` step
(`.github/workflows/ci.yml:162-163`,
running `make fmt`, which is `gofmt -l .` with a non-zero exit at
`rmqctl/Makefile:57`) will list the file and fail. #6439 adds no imports. (2)
#6439's coverage is strictly wider: six literals through one `t.Run` table
including `1e400` and `1e-400` (the two values that trunk turns into `.inf` and
`0`), decoded through a real `json.Decoder` with `UseNumber()` — mirroring
`rmqctl/internal/studio/client.go:165` instead of hand-building `json.Number`
map values — then `yaml.Unmarshal`-ed back into a `yaml.Node` to assert both
the exact digits *and* that the tag is numeric, plus a check that the input
payload was not mutated. Your test asserts three `strings.Contains` on the
rendered text; it has `1e100` in the payload at `output_test.go:64` but neither
assertion list (`:77-81`, `:86-89`) mentions it, so exponent notation — which
your description says is covered — is in fact untested, and a text-substring
match cannot tell `!!int 3` from `"3"` in the general case. (3) #6439 sits
five commits closer to the current base. Two smaller things worth knowing for
a follow-up: inserting `var integerLiteral` at `:67` orphaned `restoreNumbers`'
doc comment (it now documents the regexp, and the function at `:79` has none),
and #6439 keeps a fallback for an empty or invalid `json.Number` that this
version drops — that case is not reachable from `UseNumber()` decoding, so we
count it as defensive rather than a defect. Salvage path: if you want to keep
contributing here, review #6439 and send the nested-structure case (your
`negative: []any{map[string]any{…}}` payload exercises the recursive branches,
which #6439's single-level `{"items":[{"value":…}]}` only partly covers) as a
follow-up against that shape.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
