fix: Avoid POSIX localtime_r in Scala Native log formatter on Windows#519
Conversation
… Windows MSVC's CRT does not provide `localtime_r`, so any downstream Scala Native consumer of `uni-core` that builds for Windows hits a linker error (`unresolved external symbol localtime_r`). The wvlet project caught this when producing `wvlet.dll`: https://github.com/wvlet/wvlet/actions/runs/25133797459/job/73666762135 Gate the existing `localtime_r` + `strftime` path on `LinktimeInfo.isWindows` and fall back to a pure-Scala UTC formatter (Howard Hinnant's civil-from-days) on Windows. Since `LinktimeInfo.isWindows` resolves at link time, POSIX builds keep their existing behavior verbatim — only the Windows path drops the system timezone offset (now emits `Z`), which matches what the Scala.js path already does. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request refactors LogTimestampFormatter to support Windows in Scala Native by introducing a pure-Scala UTC formatter based on Howard Hinnant's algorithm. The reviewer identified critical buffer overflow vulnerabilities and potential issues with negative timestamps in the POSIX implementation. Additionally, it was suggested to improve the maintainability of the new UTC formatter by replacing magic numbers with named constants and providing a reference link to the algorithm.
| else | ||
| formatPosix(c"%Y-%m-%dT%H:%M:%S.", timeMillis) | ||
|
|
||
| private def formatPosix(pattern: CString, timeMillis: Long): String = Zone { |
There was a problem hiding this comment.
The implementation of formatPosix has a few critical issues that seem to be pre-existing but are important to address, as this function is being refactored:
- Potential for negative milliseconds:
timeMillis % 1000on line 53 can result in a negative value iftimeMillisis negative (for dates before the epoch). This will lead to incorrect formatting. You should useMath.floorMod(timeMillis, 1000L)to ensure the milliseconds part is always non-negative, similar to how it's done informatUtc. - Buffer overflow in
msBuf:msBufon line 55 is allocated with 3 bytes, butsprintfwithc"%03d"will write 3 digits plus a null terminator, requiring 4 bytes. This is a buffer overflow. The buffer size should be 4. - Buffer overflow in
tzBuf:tzBufon line 59 is allocated with 5 bytes.strftimewithc"%z"can produce a string like+hhmmor-hhmm, which is 5 characters. With the null terminator, this requires 6 bytes. This is another buffer overflow. The buffer size should be 6.
These buffer overflows are serious security vulnerabilities and can lead to memory corruption or crashes.
| // Pure-Scala UTC formatter used on Windows. Howard Hinnant's | ||
| // civil-from-days algorithm gives correct year/month/day for any epoch | ||
| // millis without leaning on POSIX-only C symbols. | ||
| private def formatUtc(timeMillis: Long, withSpace: Boolean): String = |
There was a problem hiding this comment.
The formatUtc method contains several magic numbers related to date and time calculations (e.g., 86400L, 719468L). To improve readability and maintainability, consider extracting these numbers into named constants, perhaps in a private helper object. This will make the algorithm's implementation clearer.
Additionally, adding a comment with a direct link to Howard Hinnant's algorithm would be very helpful for future readers. For example:
// Based on Howard Hinnant's civil-from-days algorithm:
// http://howardhinnant.github.io/date_algorithms.html#civil_from_days## Summary - The `Native` workflow's `Build native on Windows (x64)` job was failing with `unresolved external symbol localtime_r` when linking `wvc-lib/wvlet.dll` (run [25133797459](https://github.com/wvlet/wvlet/actions/runs/25133797459/job/73666762135)). - Root cause: `uni 2026.1.6`'s `LogTimestampFormatter` (Scala Native) called POSIX `localtime_r`, which MSVC's CRT doesn't expose. - [wvlet/uni#519](wvlet/uni#519) gates the `localtime_r` + `strftime` path on `LinktimeInfo.isWindows` and falls back to a pure-Scala UTC formatter on Windows. Released as `uni 2026.1.7` — this PR bumps `UNI_VERSION` to pick it up. ## Test plan - [x] Locally re-ran `./sbt wvcLib/nativeLink` against the new uni — POSIX path still links cleanly (Boehm GC, pthread/dl/m/z/crypto). - [ ] CI: `Build native on Windows (x64)` no longer hits the `localtime_r` unresolved-symbol error. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
localtime_r, so downstream ScalaNative consumers of
uni-corethat build for Windows hit a linkererror (
unresolved external symbol localtime_r). wvlet caught thiswhile producing
wvlet.dllonwindows-2022:https://github.com/wvlet/wvlet/actions/runs/25133797459/job/73666762135
localtime_r+strftimepath onscala.scalanative.meta.LinktimeInfo.isWindows, and fall back to apure-Scala UTC formatter on Windows (Howard Hinnant's civil-from-days
algorithm).
LinktimeInfo.isWindowsis a link-time constant, soPOSIX targets keep their existing behavior unchanged via DCE.
Z), whichmatches what the Scala.js path already does (
Date.toISOString()).Test plan
coreNative/testpasses locally on macOS (./sbt uniNative/test— 1330 passed)scalafmtCheckAllUNI_VERSIONinwvlet/wvletafter this lands and is published, and re-running the
Build native on Windows (x64)job that originally caught it.Follow-up
Add a
windows-latestScala Native job touni'stest.ymlso thisclass of regression is caught at the source rather than by downstream
projects.
🤖 Generated with Claude Code