Skip to content

refactor: parseHttpDate#4421

Merged
Uzlopak merged 13 commits into
mainfrom
date
Sep 6, 2025
Merged

refactor: parseHttpDate#4421
Uzlopak merged 13 commits into
mainfrom
date

Conversation

@Uzlopak
Copy link
Copy Markdown
Contributor

@Uzlopak Uzlopak commented Aug 17, 2025

This PR modifies the parseHttpDate function by adding jsdoc, and doing two major changes:

  • the weekday is checked
  • rfc850 dates process the year according to RFC 6265

At one point i took the unit tests of the npm package http-dates, which tests for the correct weekday, but it doesnt convince me personally. We can of course remove the weekday check.

The rfc850 year logic is imho more important.

Also it increases the performance significantly.

before:

parseHttpDate
┌─────────┬───────────┬──────────────────┬──────────────────┬────────────────────────┬────────────────────────┬─────────┐
│ (index) │ Task name │ Latency avg (ns) │ Latency med (ns) │ Throughput avg (ops/s) │ Throughput med (ops/s) │ Samples │
├─────────┼───────────┼──────────────────┼──────────────────┼────────────────────────┼────────────────────────┼─────────┤
│ 0       │ 'asctime''711.54 ± 0.25%''662.00 ± 22.00''1485599 ± 0.01%''1510574 ± 49488'      │ 1405401 │
│ 1       │ 'rfc850''873.87 ± 2.17%''814.00 ± 9.00''1206639 ± 0.01%''1228501 ± 13434'      │ 1144333 │
│ 2       │ 'imf''767.61 ± 0.30%''705.00 ± 24.00''1410214 ± 0.02%''1418440 ± 49989'      │ 1302746 │
└─────────┴───────────┴──────────────────┴──────────────────┴────────────────────────┴────────────────────────┴─────────┘

after:

parseHttpDate
┌─────────┬───────────┬──────────────────┬──────────────────┬────────────────────────┬────────────────────────┬─────────┐
│ (index) │ Task name │ Latency avg (ns) │ Latency med (ns) │ Throughput avg (ops/s) │ Throughput med (ops/s) │ Samples │
├─────────┼───────────┼──────────────────┼──────────────────┼────────────────────────┼────────────────────────┼─────────┤
│ 0       │ 'asctime''297.08 ± 0.21%''263.00 ± 4.00''3676255 ± 0.01%''3802281 ± 58722'      │ 3366096 │
│ 1       │ 'rfc850''332.98 ± 0.25%''306.00 ± 11.00''3278293 ± 0.01%''3267974 ± 121857'     │ 3003188 │
│ 2       │ 'imf''326.61 ± 5.11%''284.00 ± 4.00''3438115 ± 0.01%''3521127 ± 50302'      │ 3061736 │
└─────────┴───────────┴──────────────────┴──────────────────┴────────────────────────┴────────────────────────┴─────────┘

This relates to...

Rationale

Changes

Features

Bug Fixes

Breaking Changes and Deprecations

Status

@Uzlopak Uzlopak marked this pull request as ready for review August 21, 2025 07:03
@Uzlopak Uzlopak changed the title chore: improve date parsing refactor: parseHttpDate Aug 21, 2025
@Uzlopak Uzlopak requested a review from Copilot September 6, 2025 09:55
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the parseHttpDate function to improve performance and correctness by adding JSDoc documentation, implementing weekday validation, and updating RFC850 year parsing to comply with RFC 6265.

  • Replaces string-based parsing with character code validation for better performance
  • Adds weekday validation to ensure dates match their specified day of the week
  • Updates RFC850 two-digit year handling according to RFC 6265 (70-99 → 1970-1999, 00-69 → 2000-2069)

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
lib/util/date.js Complete rewrite of date parsing functions with character-level validation and weekday checking
test/utils/date.js Extensive test additions including fuzzing tests and corrected weekday values in existing tests
benchmarks/utils/date.mjs New benchmark file for performance testing the parseHttpDate function
benchmarks/package.json Added tinybench dependency for benchmarking

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread lib/util/date.js
Comment thread lib/util/date.js
Comment thread lib/util/date.js
Comment thread test/utils/date.js
Copy link
Copy Markdown
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@Uzlopak Uzlopak merged commit 82ea8fc into main Sep 6, 2025
36 checks passed
@Uzlopak Uzlopak deleted the date branch September 6, 2025 16:33
@github-actions github-actions Bot mentioned this pull request Sep 9, 2025
slagiewka pushed a commit to slagiewka/undici that referenced this pull request Feb 14, 2026
* chore: improve util/date

* add benchmark

* case sensitive

* improve mainly asctime

* take care of other cases

* remove  one check

* add fuzzing

* remove redundant line

* add test from http-dates

* add weekday check, adapt rfc 6265 for interpretation of rfc850

* fix ascTime

* remove unused parameter
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.

3 participants