Skip to content

fix: serialise OffsetDateTime as RFC 3339 - #609

Open
dudanogueira wants to merge 1 commit into
mainfrom
fix/rfc3339-dates
Open

fix: serialise OffsetDateTime as RFC 3339#609
dudanogueira wants to merge 1 commit into
mainfrom
fix/rfc3339-dates

Conversation

@dudanogueira

Copy link
Copy Markdown

Motivation

OffsetDateTime.toString() drops the seconds when the second and the nanosecond are both zero, so 2024-03-01T00:00:00Z goes on the wire as 2024-03-01T00:00Z. RFC 3339's partial-time requires hour ":" minute ":" second, and Weaviate rejects the short form:

invalid date property 'when' on class 'X': requires a string with a RFC3339 formatted date,
but the given value is '2024-03-01T00:00Z'

Every timestamp on an exact minute boundary is affected — which is most timestamps written by hand — and it breaks writes and filters alike. toString() is a display format, not a wire format.

Approach

DateUtil already owned the read side (fromISO8601) but had no write-side counterpart, which is exactly why the same toString() call got copy-pasted into every marshalling site. It now has toRFC3339(), and all six sites call it.

The formatter is built with DateTimeFormatterBuilder rather than ofPattern("yyyy-MM-dd'T'HH:mm:ssXXX"): the pattern form would fix the reported bug while silently truncating sub-second precision that works today. appendFraction(NANO_OF_SECOND, 0, 9, true) keeps the fraction variable-width, so it is neither invented for whole seconds nor truncated for nanos.

Scope: six call sites, not three

The issue named three. The list and array variants have the same bug:

Site Path In the report
Filter.DateOperand.appendTo gRPC search yes
Filter.DateArrayOperand.formatted containsAny/containsNone no
InsertManyRequest scalar gRPC batch yes
InsertManyRequest List<OffsetDateTime> gRPC batch no
InsertManyRequest OffsetDateTime[] gRPC batch no
DateUtil Gson TypeAdapter REST yes

Aggregate filters, boost filters and deleteMany all funnel through Filter, so they are fixed without separate changes. Filter.DateOperand.toString() is left alone — it feeds the human-readable Filter.toString(), not the wire.

Key areas for review

  • DateUtil.RFC3339 — the formatter itself; the fraction handling is the part worth a second look
  • The read path is deliberately untouched. All four parse sites use OffsetDateTime.parse, whose default formatter treats seconds and fraction as optional, so it already accepts both spellings and data written by older clients still loads. This is what makes the change safe to ship without a migration.

Testing

The reason no existing test caught this: every date test seeded from OffsetDateTime.now(), which practically never has second and nano zero. The new tests use literals.

  • Rfc3339DateTest — 26 cases: formatting across minute-boundary/millis/nanos/non-UTC/negative offsets, round-trip through the reader, the old truncated form still parsing, all eight filter comparison paths (including createdAt()/lastUpdatedAt(), which take OffsetDateTime only and so had no String workaround), the array operand, and the three InsertManyRequest shapes.
  • JSONTest — five OffsetDateTime rows; the file had none. Each row asserts both directions.
  • DataITest.testDataTypes and SearchITest.test_filterCreateUpdateTime now use minute-boundary values, so the existing round-trip and filter assertions guard the regression against a real server.

Verified the tests fail without the fix — with src/main reverted, the JSONTest rows fail with expected:<"2024-03-01T00:00[:00]Z"> but was:<"2024-03-01T00:00[]Z">.

Locally green: 418 unit tests, and DataITest + SearchITest + OrmITest against a container (48 run, 3 skipped by version gates).

Breaking changes

None. Public API is unchanged; DateUtil.toRFC3339 is additive, and the wire format only becomes more standards-compliant.

Closes #605

🤖 Generated with Claude Code

https://claude.ai/code/session_01WmY5dAGWCccWDoqkKNC2JU

OffsetDateTime.toString() omits the seconds when the second and the
nanosecond are both zero, so a timestamp on an exact minute boundary
went on the wire as "2024-03-01T00:00Z". RFC 3339's partial-time
requires hour:minute:second, and the server rejects the short form --
breaking writes and filters alike for most timestamps written by hand.

DateUtil owned the read side but had no write-side counterpart, which
is why the same toString() was copy-pasted into all six marshalling
sites. It now has toRFC3339(), which always writes the seconds and
keeps the fraction variable-width so sub-second precision is neither
invented nor truncated.

The array and list variants in Filter and InsertManyRequest were
affected too, not just the three scalar sites in the report.

Reading is unchanged and stays lenient: OffsetDateTime.parse accepts
both forms, so timestamps written by older clients still load.

Closes #605

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WmY5dAGWCccWDoqkKNC2JU

@orca-security-eu orca-security-eu Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Orca Security Scan Summary

Status Check Issues by priority
Passed Passed Infrastructure as Code high 0   medium 0   low 0   info 0 View in Orca
Passed Passed SAST high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Secrets high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Vulnerabilities high 0   medium 0   low 0   info 0 View in Orca

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

v6: OffsetDateTime is serialised with toString(), which drops the seconds and is not RFC 3339

1 participant