denoland / denoland/std

@std/toml: Inline array stringification mishandles edge cases

Open
#7,162 0 comments 0 reactions 0 assignees View on GitHub
bug needs triage
Dominant language
TypeScript
Stars
3.6k
Forks
681
PR merge metrics
No merged PRs in 30d

Description

Inline array stringification has multiple bugs in the current implementation. Running:
```ts
import * as toml from "@std/toml";

// Primitive array bugs
console.log(toml.stringify({x: [Infinity, -Infinity, NaN]}));
console.log(toml.stringify({x: [new Date(0)]}));

// Object-only array bug
try {
console.log(toml.stringify({x: [null]}));
} catch (e) {
console.log("got error", e);
}

// Mixed-type array bugs
console.log(toml.stringify({x: [Infinity, -Infinity, NaN, {}]}));
console.log(toml.stringify({x: [new Date(0), {}]}));
try {
console.log(toml.stringify({x: [1, null]}));
} catch (e) {
console.log("got error", e);
}
```
Will log:
```
x = [null,null,null] // expected: x = [inf, -inf, nan]

x = ["1970-01-01T00:00:00.000Z"] // expected: x = [1970-01-01T00:00:00.000Z]

got error TypeError: Cannot convert undefined or null to object // expected: ???
at Object.keys ()
at Dumper.#printObject (https://jsr.io/@std/toml/1.0.11/stringify.ts:51:26)
at Dumper.#printObject (https://jsr.io/@std/toml/1.0.11/stringify.ts:83:42)
at Dumper.dump (https://jsr.io/@std/toml/1.0.11/stringify.ts:45:36)
at Module.stringify (https://jsr.io/@std/toml/1.0.11/stringify.ts:294:26)

x = [Infinity,-Infinity,NaN,{}] // expected: x = [inf, -inf, nan, {}]

x = ["1970-01-01T00:00:00.000",{}] // expected: x = [1970-01-01T00:00:00.000Z, {}]

got error Error: Should never reach // expected: ???
at Dumper.#printAsInlineValue (https://jsr.io/@std/toml/1.0.11/stringify.ts:151:15)
at https://jsr.io/@std/toml/1.0.11/stringify.ts:87:64
at Array.map ()
at Dumper.#printObject (https://jsr.io/@std/toml/1.0.11/stringify.ts:87:29)
at Dumper.dump (https://jsr.io/@std/toml/1.0.11/stringify.ts:45:36)
at Module.stringify (https://jsr.io/@std/toml/1.0.11/stringify.ts:294:26)
```

Since TOML doesn't support `null` values, I'm not sure how the `{x: [null]}` and `{x: [1, null]}` cases should be handled, but I feel at the very least a better error message should be given. The quotation marks around the date is a somewhat minor bug as it causes the date to reparse as a string instead of a `Date`. The mishandling of `Infinity`/`NaN` is a more severe bug as both cases (stringifying to `null` and stringifying to `Infinity`/`NaN`) causes it to no longer parse.

I can attempt a PR to fix this, but I'm wondering if it's even necessary to stringify "primitive" arrays using `JSON.stringify`. This is causing the first 2 bugs here, and I'm not sure that it meaningfully improves runtime compared to just using the mixed-type array stringification.

**Environment**
- OS: Arch Linux
- deno version: 2.8.1
- std version: 1.0.11

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.