fix: add RFC 1035 length limits to domain regex - #5970
Conversation
|
TL;DR — Aligns Key changes
Summary ｜ 2 files ｜ 1 commit ｜ base: RFC 1035 length limits on
|
There was a problem hiding this comment.
Important
The regex change itself is correct and well-motivated — RFC 1035 interpretations are right, no ReDoS risk, no incidental narrowing beyond the documented limits. But tightening z.regexes.domain leaves two spots in the tree that embed the old regex literal and will now be inaccurate: the recommended-schema callout in packages/docs/content/api.mdx and an inline comment next to the consumer in this test file. Please update both before merge.
Note that z.regexes.domain is a public export and this is technically a breaking change, though the newly-rejected inputs (TLD >63 chars, total domain >253 chars) are all DNS-invalid by RFC 1035 — the same limits z.regexes.hostname already enforces — so the framing as a fix is reasonable. Worth a line in the eventual release notes.
Note: 1 inline comment(s) dropped because they did not anchor to lines inside the PR diff:
packages/zod/src/v4/classic/tests/string.test.ts:433(RIGHT) — line 433 (RIGHT) is not inside a diff hunk
Claude Opus ｜ 𝕏
| /^(?=.{1,253}\.?$)[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[-0-9a-zA-Z]{0,61}[0-9a-zA-Z])?)*\.?$/; | ||
|
|
||
| export const domain: RegExp = /^([a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?\.)+[a-zA-Z]{2,}$/; | ||
| export const domain: RegExp = /^(?=.{1,253}\.?$)([a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?\.)+[a-zA-Z]{2,63}$/; |
There was a problem hiding this comment.
Consider a one-line annotation so the magic numbers are self-documenting, matching the style of the hostname comment above:
/** RFC 1035: labels ≤63 chars, total ≤253 chars, TLD 2–63 chars. */
export const domain: RegExp = /^(?=.{1,253}\.?$)([a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?\.)+[a-zA-Z]{2,63}$/;Not blocking — but would save a reader from deriving intent from the literals.
There was a problem hiding this comment.
Good suggestion — added the RFC 1035 annotation in 3df7fb6.
The domain regex (used by z.httpUrl()) was missing length anchors that
the hostname regex already enforces:
- No 253-character total length limit
- No upper bound on TLD label length
Added:
- (?=.{1,253}\.?$) lookahead for overall domain length
- {2,63} TLD label cap (was unbounded {2,})
Fixes colinhacks#5965
d54ecd6 to
1072064
Compare
There was a problem hiding this comment.
Reviewed changes — added two test cases exercising the new RFC 1035 length limits on z.regexes.domain.
- TLD >63 chars rejected —
http://example.+ 64-char label triggers the{2,63}TLD cap. - Domain >253 chars rejected — 259-char hostname triggers the
(?=.{1,253}\.?$)lookahead.
The two items flagged in the prior review (packages/docs/content/api.mdx:382 embedded regex literal and packages/zod/src/v4/classic/tests/string.test.ts:433 stale regex comment) remain unaddressed — both still show the pre-change pattern.
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
|
Hi! Is there anything else needed from my side on this PR? |
Sync the api.mdx documentation and test comment with the actual z.regexes.domain pattern (RFC 1035 length limits: total ≤ 253, TLD ≤ 63).
|
Updated the two stale regex references:\n- |
wahajahmed010
left a comment
There was a problem hiding this comment.
Review
Nice catch — the inconsistency between regexes.hostname (already RFC 1035-compliant) and regexes.domain was a latent footgun in z.httpUrl()'s default. Aligning them is the right move.
Substantive notes:
-
(?=.{1,253}\.?$)is correct for the total-length constraint..matches everything except newline by default in JS (nosflag needed here, nomflag set), so embedded newlines correctly fail the lookahead and the whole regex. Good. -
{2,63}on the TLD matches RFC 1035 §2.3.4 (label length 1–63 octets). With the leading label constraint{0,61}from the rest of the regex, all labels are also capped — consistent withhostname. -
Behavioural change worth flagging: any caller passing a string longer than 253 chars or with a >63-char label that was previously accepted by
z.string().regex(z.regexes.domain)will now fail validation. That's the correct behaviour, but it's still a change — worth a CHANGELOG entry underzod/v4. If the project does a minor bump soon, an[next]note is appropriate. -
Test additions cover the two failure modes (TLD >63 chars, total >253 chars). Suggest two more boundary cases to lock the contract:
- exactly 63-char TLD → should pass (
{2,63}inclusive upper bound) - exactly 253-char total length → should pass (
{1,253}inclusive)
These guard against off-by-one if anyone ever changes the quantifiers.
- exactly 63-char TLD → should pass (
-
One small nit on the diff: the JSDoc comment
/** RFC 1035: labels ≤63 chars, total ≤253 chars, TLD 2–63 chars. */slightly undersells what the regex enforces — the leading label is also bounded to[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(≤63 chars), same ashostname. Consider matching that wording for consistency, or just point readers atregexes.hostname's comment.
Otherwise solid. Approving once the boundary-case tests land or in a follow-up.
Problem
The
regexes.domainregex (used internally byz.httpUrl()for hostname validation) was missing RFC 1035 length limits that theregexes.hostnameregex already enforces:{2,}vs{2,63})This meant
z.httpUrl()accepted domains exceeding the 253-character DNS limit and TLDs longer than 63 characters, whilez.hostname()correctly rejected them.Fix
Added RFC 1035 length constraints to
regexes.domain:(?=.{1,253}\.?$)lookahead — enforces the 253-character total domain length limit{2,63}on the TLD label — caps the last label at 63 characters per RFC 1035Testing
Fixes #5965