[
https://issues.apache.org/jira/browse/TINKERPOP-3276?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18098306#comment-18098306
]
ASF GitHub Bot commented on TINKERPOP-3276:
-------------------------------------------
xiazcy opened a new pull request, #3549:
URL: https://github.com/apache/tinkerpop/pull/3549
### Summary
The `gremlin-javascript` OffsetDateTime deserializers constructed a native
`Date` without validating the result. JavaScript's `Date` has a much narrower
range than Gremlin/JVM date-time types, so extreme boundary values (e.g. years
near ±999999999, or `OffsetDateTime.MAX`/`MIN`) produced `NaN`. Because `new
Date(NaN)` does not throw, deserialization *appeared* to succeed but returned
an unusable `Date`:
```js
value instanceof Date // true
Number.isNaN(value.getTime()) // true
value.toISOString() // throws RangeError
```
Both readers now detect that condition and fail deserialization with a clear
error, so unsupported boundary values are rejected up front rather than
silently becoming invalid `Date` instances — consistent with how other
unsupported values are handled.
### Changes
- **GraphBinary** (`internals/OffsetDateTimeSerializer.js`): after building
the `Date` from the wire fields, throw if `Number.isNaN(v.getTime())`.
- **GraphSON** (`type-serializers.js`): the same guard in
`OffsetDateTimeSerializer.deserialize`, for parity.
- Added `OffsetDateTimeSerializer-test.js` (GraphBinary) covering normal
round-trips and boundary rejection for ±999999999.
- Added GraphSON reader tests: a normal `gx:OffsetDateTime` round-trip and
boundary rejection for expanded-year ISO strings.
---
VOTE +1
> gremlin-javascript offsetdatetime silently creates incorrect values
> -------------------------------------------------------------------
>
> Key: TINKERPOP-3276
> URL: https://issues.apache.org/jira/browse/TINKERPOP-3276
> Project: TinkerPop
> Issue Type: Bug
> Components: javascript
> Affects Versions: 3.8.1
> Reporter: Ken Hu
> Priority: Minor
>
> Caveat: I haven't fully verified this, just something AI told me when I was
> updating the "model" tests for gremlin-javascript. Just putting here so it
> can be checked later.
> JavaScript Date has a much smaller valid range than Gremlin/JVM date-time
> types. GraphBinary can encode extreme values like OffsetDateTime.MAX,
> OffsetDateTime.MIN, or the GraphBinary v4 DateTime equivalents with years
> around +999999999 / -999999999.
> The JavaScript GraphBinary reader currently reads the wire fields
> successfully:
> year, month, day, nanoseconds-since-midnight, utc-offset
> Then it tries to convert them into a native JS Date:
> new Date(Date.UTC(year, month, day, hour, minute, second, millisecond))
> For those extreme years, Date.UTC(...) returns NaN because the value is
> outside the supported JS Date range. new Date(NaN) does not throw. It creates
> an
> invalid Date object. So deserialization appears to succeed, but the returned
> object is unusable:
> value instanceof Date // true
> Number.isNaN(value.getTime()) // true
> value.toISOString() // throws RangeError
> What should happen instead:
> The reader should detect that the decoded date-time cannot be represented as
> a valid JavaScript Date and fail deserialization with a clear error. For
> example, after constructing the Date, it should validate:
> if (Number.isNaN(v.getTime())) {
> throw new Error('DateTime value is outside the supported JavaScript Date
> range');
> }
> The important behavior change is: unsupported date-time boundary values
> should not silently become invalid Date instances. They should be rejected
> during
> deserialization, just like other unsupported GraphBinary values
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
