Skip to content

feat: RFC 3339 duration Validator #1554

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hazrid93 wants to merge 1 commit into
base: main
Choose a base branch
from
+90 −0
Open

feat: RFC 3339 duration Validator #1554

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions library/src/actions/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export * from './ipv6/index.ts';
export * from './isbn/index.ts';
export * from './isrc/index.ts';
export * from './isoDate/index.ts';
export * from './isoDuration/index.ts';
export * from './isoDateTime/index.ts';
export * from './isoDateTimeSecond/index.ts';
export * from './isoTime/index.ts';
Expand Down
1 change: 1 addition & 0 deletions library/src/actions/isoDuration/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from './isoDuration.ts';
88 changes: 88 additions & 0 deletions library/src/actions/isoDuration/isoDuration.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import type {
BaseIssue,
BaseValidation,
ErrorMessage,
} from '../../types/index.ts';
import { _addIssue } from '../../utils/index.ts';

/**
* The ISO 8601 / RFC 3339 duration regex. Matches strings like `P1D`,
* `PT5H30M`, `P1Y2M3DT4H5M6S`, with optional decimal values. A bare `P` or `PT`
* (no components) is rejected. See issue #1497.
*/
const ISO_DURATION_REGEX: RegExp =
/^P(?!T?$)(?:(\d+(?:\.\d+)?)Y)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)W)?(?:(\d+(?:\.\d+)?)D)?(?:T(?:(\d+(?:\.\d+)?)H)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)S)?)?$/;
Comment on lines +13 to +14

@coderabbitai coderabbitai Bot Jul 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Prevent the T designator from matching when time components are absent.

In the current ISO_DURATION_REGEX, the time components (H, M, S) are fully optional inside the T group (?:T(?:...H)?(?:...M)?(?:...S)?)?. This allows invalid durations like P1YT to pass validation, as T can be matched with nothing following it. According to the ISO 8601 standard, if there are no time components, the T designator must be absent.

Adding a positive lookahead (?=\d) immediately after the T ensures it is only matched when followed by a valid time component (which must start with a digit).

💚 Proposed fix 
 /**
  * The ISO 8601 / RFC 3339 duration regex. Matches strings like `P1D`,
  * `PT5H30M`, `P1Y2M3DT4H5M6S`, with optional decimal values. A bare `P` or `PT`
  * (no components) is rejected. See issue `#1497`.
  */
 const ISO_DURATION_REGEX: RegExp =
-  /^P(?!T?$)(?:(\d+(?:\.\d+)?)Y)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)W)?(?:(\d+(?:\.\d+)?)D)?(?:T(?:(\d+(?:\.\d+)?)H)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)S)?)?$/;
+  /^P(?!T?$)(?:(\d+(?:\.\d+)?)Y)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)W)?(?:(\d+(?:\.\d+)?)D)?(?:T(?=\d)(?:(\d+(?:\.\d+)?)H)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)S)?)?$/;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const ISO_DURATION_REGEX: RegExp =
/^P(?!T?$)(?:(\d+(?:\.\d+)?)Y)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)W)?(?:(\d+(?:\.\d+)?)D)?(?:T(?:(\d+(?:\.\d+)?)H)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)S)?)?$/;
const ISO_DURATION_REGEX: RegExp =
/^P(?!T?$)(?:(\d+(?:\.\d+)?)Y)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)W)?(?:(\d+(?:\.\d+)?)D)?(?:T(?=\d)(?:(\d+(?:\.\d+)?)H)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)S)?)?$/;
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@library/src/actions/isoDuration/isoDuration.ts` around lines 13 - 14, Update
ISO_DURATION_REGEX so the optional time group’s T designator requires a
following digit via a positive lookahead, preventing values such as P1YT from
matching while preserving valid hour, minute, and second components.


Comment on lines +14 to +15

@cubic-dev-ai cubic-dev-ai Bot Jul 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: Regex accepts P1DT / P1Y1M1DT (T separator with no time components). Per ISO 8601, the T designator must be followed by at least one time component; a bare T should be rejected.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At library/src/actions/isoDuration/isoDuration.ts, line 14:

<comment>Regex accepts `P1DT` / `P1Y1M1DT` (T separator with no time components). Per ISO 8601, the T designator must be followed by at least one time component; a bare `T` should be rejected.</comment>

<file context>
@@ -0,0 +1,88 @@
+ * (no components) is rejected. See issue #1497.
+ */
+const ISO_DURATION_REGEX: RegExp =
+  /^P(?!T?$)(?:(\d+(?:\.\d+)?)Y)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)W)?(?:(\d+(?:\.\d+)?)D)?(?:T(?:(\d+(?:\.\d+)?)H)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)S)?)?$/;
+
+/**
</file context>
Suggested change
/^P(?!T?$)(?:(\d+(?:\.\d+)?)Y)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)W)?(?:(\d+(?:\.\d+)?)D)?(?:T(?:(\d+(?:\.\d+)?)H)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)S)?)?$/;
/^P(?!T?$)(?:(\d+(?:\.\d+)?)Y)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)W)?(?:(\d+(?:\.\d+)?)D)?(?:T(?=\d)(?:(\d+(?:\.\d+)?)H)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)S)?)?$/;

/**
* ISO duration issue interface.
*/
export interface IsoDurationIssue<TInput extends string>
extends BaseIssue<TInput> {
readonly kind: 'validation';
readonly type: 'iso_duration';
readonly expected: null;
readonly received: `"${string}"`;
readonly requirement: RegExp;
}

/**
* ISO duration action interface.
*/
export interface IsoDurationAction<
TInput extends string,
TMessage extends ErrorMessage<IsoDurationIssue<TInput>> | undefined,
> extends BaseValidation<TInput, TInput, IsoDurationIssue<TInput>> {
readonly type: 'iso_duration';
readonly reference: typeof isoDuration;
readonly expects: null;
readonly requirement: RegExp;
readonly message: TMessage;
}

/**
* Creates an [ISO 8601](https://en.wikipedia.org/wiki/ISO_8601#Durations) /
* [RFC 3339](https://www.rfc-editor.org/rfc/rfc3339) duration validation action.
*
* Format: `P[n]Y[n]M[n]W[n]DT[n]H[n]M[n]S` (e.g. `P1D`, `PT5H30M`,
* `P1Y2M3DT4H5M6S`). Decimal values are supported. A bare `P` or `PT` with no
* components is rejected. See issue #1497.
*
* @returns An ISO duration action.
*/
export function isoDuration<TInput extends string>(): IsoDurationAction<
TInput,
undefined
>;

/**
* Creates an ISO duration validation action.
*
* @param message The error message.
*
* @returns An ISO duration action.
*/
export function isoDuration<
TInput extends string,
const TMessage extends ErrorMessage<IsoDurationIssue<TInput>> | undefined,
>(message: TMessage): IsoDurationAction<TInput, TMessage>;

// @__NO_SIDE_EFFECTS__
export function isoDuration(
message?: ErrorMessage<IsoDurationIssue<string>>,
): IsoDurationAction<string, ErrorMessage<IsoDurationIssue<string>> | undefined> {
return {
kind: 'validation',
type: 'iso_duration',
reference: isoDuration,
async: false,
expects: null,
requirement: ISO_DURATION_REGEX,
message,
'~run'(dataset, config) {
if (dataset.typed && !this.requirement.test(dataset.value)) {
_addIssue(this, 'duration', dataset, config);
}
return dataset;
},
};
}