esm: avoid super-linear data URL MIME regex by skdas20 · Pull Request #61951 · nodejs/node

skdas20 · 2026-02-23T08:41:25Z

Fixes: #61904

This updates ESM data URL MIME extraction regexes to remove overlapping quantifiers that allow super-linear backtracking.

Replaced (?:[^,]*?)(;base64)?, with (?:;[^,]*)?, in:
- lib/internal/modules/esm/get_format.js
- lib/internal/modules/esm/load.js

Behavior is preserved for existing valid/invalid data: URL shapes while avoiding pathological backtracking on crafted inputs.

nodejs-github-bot · 2026-02-23T08:41:31Z

Review requested:

@nodejs/loaders

jsumners-nr

Where are tests to assert the undesired behavior is resolved by the changes?

skdas20 · 2026-02-23T15:10:46Z

Added regression coverage in test/parallel/test-esm-data-url-format-regex-backtracking.js (commit 740d1320).

The test exercises defaultGetFormat() with a long malformed data: URL (data:a/aaaa...B) that used to trigger super-linear backtracking, and asserts the path now completes within a platform-adjusted bound.

aduh95 · 2026-02-24T14:34:36Z

+// Flags: --expose-internals
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const { defaultGetFormat } = require('internal/modules/esm/get_format');
+
+// Regression test for #61904. This malformed data URL path is crafted to
+// stress the MIME regex without triggering URL parsing failures.
+const longPath = `data:a/${'a'.repeat(2 ** 17)}B`;
+const start = process.hrtime.bigint();
+const format = defaultGetFormat(new URL(longPath), { parentURL: undefined });
+const elapsedMs = Number(process.hrtime.bigint() - start) / 1e6;
+
+assert.strictEqual(format, null);
+assert.ok(
+ elapsedMs < common.platformTimeout(1000),
+ `Expected format detection to complete quickly, took ${elapsedMs}ms`,
+);


This is going to be inherently flaky, could we instead write a benchmark?

skdas20 · 2026-02-26T18:50:18Z

CI has not started yet on this PR (currently no check runs, and needs-ci is present).

Could a collaborator please add the request-ci label to trigger Jenkins?

JakobJingleheimer · 2026-02-26T19:12:57Z

There are no official approvals on this PR, so CI cannot be started (it will refuse).

skdas20 · 2026-07-25T19:31:03Z

Replaced the timing-based regression test with a benchmark (esm/get-data-protocol-format) parameterized over path length, per @aduh95's suggestion — a backtracking regression will now show up as an ops/sec cliff instead of relying on wall-clock assertions in CI.

Could a collaborator add request-ci to kick off Jenkins?

nodejs-github-bot · 2026-07-25T19:57:24Z

CI: https://ci.nodejs.org/job/node-test-pull-request/75139/

skdas20 · 2026-07-29T14:17:17Z

Squashed into a single signed commit and force-pushed — the diff is unchanged (regex fix in get_format.js/load.js plus the esm/get-data-protocol-format benchmark).

Doing this pre-emptively since @trivikr flagged the same missing Signed-off-by on #62010 and #62009, and this one is already approved — should be one less thing blocking the commit queue.

mcollina · 2026-07-29T14:55:54Z

check the commit lint

codecov · 2026-07-29T16:20:23Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.14%. Comparing base (da5efc4) to head (cf0eb3b).
⚠️ Report is 1468 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #61951 +/- ##
==========================================
+ Coverage 88.84% 90.14% +1.30% 
==========================================
Files 674 746 +72 Lines 204957 242818 +37861 Branches 39309 45766 +6457 ==========================================
+ Hits 182087 218891 +36804 - Misses 15088 15420 +332 - Partials 7782 8507 +725

Files with missing lines	Coverage Δ
lib/internal/modules/esm/get_format.js	`95.20% <100.00%> (+1.10%)`	⬆️
lib/internal/modules/esm/load.js	`91.40% <100.00%> (-0.08%)`	⬇️

... and 520 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

The MIME regex used for data: URLs could backtrack super-linearly on a malformed URL lacking a ',' separator. Make the optional parameter group anchored on ';' so it cannot overlap with the media-type group. Adds a benchmark (esm/get-data-protocol-format) parameterized over path length, so a backtracking regression shows up as an ops/sec cliff instead of a wall-clock assertion in a test, per review. Fixes: nodejs#61904 Signed-off-by: skdas20 <skdas5405@gmail.com>

skdas20 · 2026-07-29T16:26:00Z

Thanks @mcollina — line-length was failing: one body line was 74 columns. Rewrapped the message so every line is within 72; the diff is unchanged.

skdas20 · 2026-08-01T06:27:42Z

Status summary for whoever picks this up: the PR is approved, and the lint-commit-message failure @mcollina flagged is fixed (one body line was 74 columns; rewrapped to 72). The force-push that fixed it was message-only — the tree is identical to the one that passed CI on 25 July (https://ci.nodejs.org/job/node-test-pull-request/75139/).

Could a collaborator add request-ci so Jenkins can re-run against the current commit? That looks like the only thing left before this can be queued.

skdas20 · 2026-08-10T18:27:15Z

@avivkeller @jsumners-nr — sorry to ping directly, but this has been sitting since 1 Aug and I think it only needs a label.

State: two approvals, GitHub Actions fully green on cf0eb3b7, mergeable_state is blocked solely on the outstanding needs-ci. The last force-push was message-only (rewrapping a 74-column line for lint-commit-message), so the tree is byte-identical to the one that passed Jenkins on 25 July: https://ci.nodejs.org/job/node-test-pull-request/75139/

Could one of you add request-ci? Happy to rebase first if you'd rather it run against a fresher base.

nodejs-github-bot added esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run. labels Feb 23, 2026

jsumners-nr reviewed Feb 23, 2026
View reviewed changes

jsumners-nr approved these changes Feb 24, 2026
View reviewed changes

aduh95 reviewed Feb 24, 2026
View reviewed changes

avivkeller approved these changes Jul 25, 2026
View reviewed changes

avivkeller added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 25, 2026

github-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 25, 2026

skdas20 force-pushed the fix-61904-data-url-regex-backtracking branch from bd04ddd to 3487338Compare July 29, 2026 14:17

skdas20 force-pushed the fix-61904-data-url-regex-backtracking branch from 3487338 to cf0eb3bCompare July 29, 2026 16:26

Uh oh!

Uh oh!

Conversation

skdas20 commented Feb 23, 2026

Uh oh!

nodejs-github-bot commented Feb 23, 2026

Uh oh!

jsumners-nr left a comment

Choose a reason for hiding this comment

Uh oh!

skdas20 commented Feb 23, 2026

Uh oh!

aduh95Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

skdas20 commented Feb 26, 2026

Uh oh!

JakobJingleheimer commented Feb 26, 2026

Uh oh!

skdas20 commented Jul 25, 2026

Uh oh!

nodejs-github-bot commented Jul 25, 2026

Uh oh!

skdas20 commented Jul 29, 2026

Uh oh!

mcollina commented Jul 29, 2026

Uh oh!

codecovBot commented Jul 29, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

skdas20 commented Jul 29, 2026

Uh oh!

skdas20 commented Aug 1, 2026

Uh oh!

skdas20 commented Aug 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

codecovBot commented Jul 29, 2026•
edited
Loading