Skip to content

fix(path): handle lowercasing that changes path length in windows relative() - #7337

Open
Punisheroot wants to merge 3 commits into
denoland:mainfrom
Punisheroot:fix/js/windows-relative-lowercase-index
Open

Punisheroot wants to merge 3 commits into
denoland:mainfrom
Punisheroot:fix/js/windows-relative-lowercase-index

Conversation

@Punisheroot

Copy link
Copy Markdown

Summary

  • Fix relative() from @std/path/windows dropping leading characters when a
    path contains a character that changes length when lowercased, such as İ
    (U+0130).
  • Fall back to a segment-by-segment comparison whenever lowercasing changes the
    length of either resolved path.

Context

relative() lowercases both paths to compare them case-insensitively, then
slices the original to path using indexes found in the lowercased strings.
"İ".toLowerCase() is "i̇", which is two code units, so each such character in
the common prefix shifts those indexes and the result loses one leading
character per occurrence:

relative("C:\\İPTV\\player", "C:\\İPTV\\player\\src\\main.ts");
// "rc\\main.ts" instead of "src\\main.ts"

This also affects ESLint on Windows: @eslint/config-array uses this function
to match ignore patterns against paths relative to the config folder, so
projects under a directory containing İ stop matching **/node_modules/ and
end up linting all of node_modules.

The fallback mirrors the fix merged upstream for path.win32.relative() in
nodejs/node#53991.

Fixes #7336

Changes

Validation

  • deno test -A path/: PASS (91 passed, 0 failed)
  • deno test path/relative_test.ts: PASS (fails before the fix with
    ode_modules\\zod\\index.ts)
  • deno check path/windows/relative.ts path/relative_test.ts: PASS
  • deno lint path/windows/relative.ts path/relative_test.ts: PASS
  • deno fmt --check: PASS
  • deno doc --lint path/windows/relative.ts: PASS
  • deno check --config browser-compat.tsconfig.json path/windows/relative.ts:
    PASS
  • Full deno task test run locally on Windows: 7424 passed, 156 ignored; the
    35 failures are all fs/* symlink tests failing with os error 1314 (symlink
    privileges unavailable on this machine), unrelated to this change.
  • deno task lint:docs and deno task test:browser can't run as-is on this
    Windows machine (command-line length limit and missing grep/xargs); the
    targeted equivalents above pass and CI covers the full tasks.

…ative()

`relative()` lowercases both paths to compare them case-insensitively, then slices the original `to` path using indexes found in the lowercased strings. Some characters change length when lowercased ("İ" becomes "i̇", which is two code units), shifting those indexes and dropping leading characters from the result.

Fall back to comparing path segments when lowercasing changes the length of either path, mirroring the fix merged for nodejs/node#53991. Regression cases are added to path/relative_test.ts.

Fixes denoland#7336
@CLAassistant

CLAassistant commented Sep 26, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions Bot added the path label Sep 26, 2026
@codecov

codecov Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.04%. Comparing base (f834d02) to head (c286abb).

Additional details and impacted files
@@           Coverage Diff            @@
##             main    #7337    +/-   ##
========================================
  Coverage   95.04%   95.04%            
========================================
  Files         619      619            
  Lines       52012    51788   -224     
  Branches     9450     9414    -36     
========================================
- Hits        49434    49223   -211     
+ Misses       2031     2022     -9     
+ Partials      547      543     -4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 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.

@tomas-zijdemans

Copy link
Copy Markdown
Contributor

This looks good. I noticed that Patch coverage is 67.74%

Could you investigate raising it? (ideally over 95% if practically possible)

@Punisheroot

Copy link
Copy Markdown
Author

@tomas-zijdemans done!

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

path: relative() on Windows drops leading characters when a path contains "İ"

3 participants