Skip to content
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

Jump to bottom

fix: improve windows path handling and improve coverage #36

Merged
merged 1 commit into from Aug 10, 2022
Merged

fix: improve windows path handling and improve coverage #36

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 15 additions & 11 deletions src/path.ts
View file
Open in desktop
Expand Up @@ -10,9 +10,9 @@ import type path from 'path'

import { normalizeWindowsPath } from './utils'

const _UNC_REGEX = /^[/][/]/
const _UNC_DRIVE_REGEX = /^[/][/]([.]{1,2}[/])?([a-zA-Z]):[/]/
const _IS_ABSOLUTE_RE = /^\/|^\\|^[a-zA-Z]:[/\\]/
const _UNC_REGEX = /^[\\/]{2}/
const _IS_ABSOLUTE_RE = /^[\\/](?![\\/])|^[\\/]{2}(?!\.)|^[a-zA-Z]:[\\/]/
const _DRIVE_LETTER_RE = /^[a-zA-Z]:$/

// Force POSIX contants
export const sep = '/'
Expand All @@ -26,7 +26,6 @@ export const normalize: typeof path.normalize = function (path: string) {
path = normalizeWindowsPath(path)

const isUNCPath = path.match(_UNC_REGEX)
const hasUNCDrive = isUNCPath && path.match(_UNC_DRIVE_REGEX)
const isPathAbsolute = isAbsolute(path)
const trailingSeparator = path[path.length - 1] === '/'

Expand All @@ -38,9 +37,10 @@ export const normalize: typeof path.normalize = function (path: string) {
return trailingSeparator ? './' : '.'
}
if (trailingSeparator) { path += '/' }
if (_DRIVE_LETTER_RE.test(path)) { path += '/' }

if (isUNCPath) {
if (hasUNCDrive) {
if (!isPathAbsolute) {
return `//./${path}`
}
return `//${path}`
Expand All @@ -58,7 +58,7 @@ export const join: typeof path.join = function (...args) {
let joined: string
for (let i = 0; i < args.length; ++i) {
const arg = args[i]
if (arg.length > 0) {
if (arg && arg.length > 0) {
if (joined === undefined) {
joined = arg
} else {
Expand All @@ -85,7 +85,7 @@ export const resolve: typeof path.resolve = function (...args) {
const path = i >= 0 ? args[i] : process.cwd().replace(/\\/g, '/')

// Skip empty entries
if (path.length === 0) {
if (!path || path.length === 0) {
continue
}

Expand All @@ -112,7 +112,7 @@ export function normalizeString (path: string, allowAboveRoot: boolean) {
let lastSegmentLength = 0
let lastSlash = -1
let dots = 0
let char = null
let char: string | null = null
for (let i = 0; i <= path.length; ++i) {
if (i < path.length) {
char = path[i]
Expand All @@ -126,8 +126,8 @@ export function normalizeString (path: string, allowAboveRoot: boolean) {
// NOOP
} else if (dots === 2) {
if (res.length < 2 || lastSegmentLength !== 2 ||
res[res.length - 1] !== '.' ||
res[res.length - 2] !== '.') {
res[res.length - 1] !== '.' ||
res[res.length - 2] !== '.') {
if (res.length > 2) {
const lastSlashIndex = res.lastIndexOf('/')
if (lastSlashIndex === -1) {
Expand Down Expand Up @@ -202,7 +202,11 @@ export const relative: typeof path.relative = function (from, to) {

// dirname
export const dirname: typeof path.dirname = function (p) {
return normalizeWindowsPath(p).replace(/\/$/, '').split('/').slice(0, -1).join('/') || (isAbsolute(p) ? '/' : '.')
const segments = normalizeWindowsPath(p).replace(/\/$/, '').split('/').slice(0, -1)
if (segments.length === 1 && _DRIVE_LETTER_RE.test(segments[0])) {
segments[0] += '/'
}
return segments.join('/') || (isAbsolute(p) ? '/' : '.')
}

// format
Expand Down
2 changes: 1 addition & 1 deletion src/utils.ts
View file
Open in desktop
@@ -1,6 +1,6 @@
// Util to normalize windows paths to posix
export function normalizeWindowsPath (input: string = '') {
if (!input.includes('\\')) {
if (!input || !input.includes('\\')) {
return input
}
return input.replace(/\\/g, '/')
Expand Down
79 changes: 69 additions & 10 deletions test/index.spec.ts
View file
Open in desktop
@@ -1,6 +1,6 @@
import { describe, expect, it, vi } from 'vitest'

import { basename, dirname, extname, format, parse, relative, delimiter, isAbsolute, join, normalize, resolve, sep, toNamespacedPath } from '../src'
import { basename, dirname, extname, format, parse, relative, delimiter, isAbsolute, join, normalize, resolve, sep, toNamespacedPath, normalizeString } from '../src'

import { normalizeWindowsPath } from '../src/utils'

Expand All @@ -22,24 +22,52 @@ runTest('isAbsolute', isAbsolute, {
'.': false,

// Windows
'C:': false,
'C:.': false,
'C:/': true,
'C:.\\temp\\': false,
'//server': true,
'\\\\server': true,
'C:/foo/..': true,
'bar\\baz': false,
'bar/baz': false
})

runTest('basename', basename, [
runTest('normalizeString', normalizeString, {
// POSIX
['C:\\temp\\myfile.html', 'myfile.html'],
['\\temp\\myfile.html', 'myfile.html'],
['.\\myfile.html', 'myfile.html'],
['.\\myfile.html', '.html', 'myfile'],
'/foo/bar': 'foo/bar',
'/foo/bar/.././baz': 'foo/baz',
'/foo/bar/../.well-known/baz': 'foo/.well-known/baz',
'/foo/bar/../..well-known/baz': 'foo/..well-known/baz',
'/a/../': '',
'/a/./': 'a',
'./foobar/../a': 'a',
'./foo/be/bar/../ab/test': 'foo/be/ab/test',
// './foobar./../a/./': 'a',

// Windows
[normalizeWindowsPath('C:\\temp\\..')]: 'C:',
[normalizeWindowsPath('C:\\temp\\..\\.\\Users')]: 'C:/Users',
[normalizeWindowsPath('C:\\temp\\..\\.well-known\\Users')]: 'C:/.well-known/Users',
[normalizeWindowsPath('C:\\temp\\..\\..well-known\\Users')]: 'C:/..well-known/Users',
[normalizeWindowsPath('C:\\a\\..\\')]: 'C:',
[normalizeWindowsPath('C:\\temp\\myfile.html')]: 'C:/temp/myfile.html',
[normalizeWindowsPath('\\temp\\myfile.html')]: 'temp/myfile.html',
[normalizeWindowsPath('.\\myfile.html')]: 'myfile.html'
})

runTest('basename', basename, [

// POSIX
['/temp/myfile.html', 'myfile.html'],
['./myfile.html', 'myfile.html'],
['./myfile.html', '.html', 'myfile']
['./myfile.html', '.html', 'myfile'],

// Windows
['C:\\temp\\myfile.html', 'myfile.html'],
['\\temp\\myfile.html', 'myfile.html'],
['.\\myfile.html', 'myfile.html'],
['.\\myfile.html', '.html', 'myfile']
])

runTest('dirname', dirname, {
Expand All @@ -50,7 +78,9 @@ runTest('dirname', dirname, {
'./myfile.html': '.',

// Windows
'C:\\temp\\': 'C:',
'C:\\temp\\': 'C:/',
'C:.\\temp\\': 'C:.',
'C:.\\temp\\bar\\': 'C:./temp',
'C:\\temp\\myfile.html': 'C:/temp',
'\\temp\\myfile.html': '/temp',
'.\\myfile.html': '.'
Expand Down Expand Up @@ -88,6 +118,8 @@ runTest('format', format, [
])

runTest('join', join, [
['.'],
[undefined, '.'],
['/', '/path', '/path'],
['/test//', '//path', '/test/path'],
['some/nodejs/deep', '../path', 'some/nodejs/path'],
Expand All @@ -108,13 +140,22 @@ runTest('join', join, [

runTest('normalize', normalize, {
// POSIX
'': '.',
'/': '/',
'/a/..': '/',
'./a/../': './',
'./a/..': '.',
'./': './',
'./../': '../',
'happiness/ab/../': 'happiness/',
'happiness/a./../': 'happiness/',
'./../dep/': '../dep/',
'path//dep\\': 'path/dep/',
'/foo/bar//baz/asdf/quux/..': '/foo/bar/baz/asdf',

// Windows
'C:\\': 'C:/',
'C:\\temp\\..': 'C:/',
'C:\\temp\\\\foo\\bar\\..\\': 'C:/temp/foo/',
'C:////temp\\\\/\\/\\/foo/bar': 'C:/temp/foo/bar',
'c:/windows/nodejs/path': 'c:/windows/nodejs/path',
Expand All @@ -131,7 +172,9 @@ runTest('normalize', normalize, {
// UNC
'\\\\server\\share\\file\\..\\path': '//server/share/path',
'\\\\.\\c:\\temp\\file\\..\\path': '//./c:/temp/path',
'\\\\server/share/file/../path': '//server/share/path'
'\\\\server/share/file/../path': '//server/share/path',
'\\\\C:\\foo\\bar': '//C:/foo/bar',
'\\\\.\\foo\\bar': '//./foo/bar'
})

it('parse', () => {
Expand Down Expand Up @@ -182,19 +225,35 @@ runTest('relative', relative, [
runTest('resolve', resolve, [
// POSIX
['/', '/path', '/path'],
['/', '', undefined, null, '', '/path', '/path'],
['/foo/bar', './baz', '/foo/bar/baz'],
['/foo/bar', './baz', undefined, null, '', '/foo/bar/baz'],
['/foo/bar', '..', '.', './baz', '/foo/baz'],
['/foo/bar', '/tmp/file/', '/tmp/file'],
['wwwroot', 'static_files/png/', '../gif/image.gif', () => `${process.cwd().replace(/\\/g, '/')}/wwwroot/static_files/gif/image.gif`],

// Windows
['C:\\foo\\bar', '.\\baz', 'C:/foo/bar/baz'],
['\\foo\\bar', '.\\baz', '/foo/bar/baz'],
['\\foo\\bar', '..', '.', '.\\baz', '/foo/baz'],
['\\foo\\bar', '\\tmp\\file\\', '/tmp/file'],
['\\foo\\bar', undefined, null, '', '\\tmp\\file\\', '/tmp/file'],
['\\foo\\bar', undefined, null, '', '\\tmp\\file\\', undefined, null, '', '/tmp/file'],
['wwwroot', 'static_files\\png\\', '..\\gif\\image.gif', () => `${process.cwd().replace(/\\/g, '/')}/wwwroot/static_files/gif/image.gif`],
['C:\\Windows\\path\\only', '../../reports', 'C:/Windows/reports'],
['C:\\Windows\\long\\path\\mixed/with/unix', '../..', '..\\../reports', 'C:/Windows/long/reports']
])

describe('resolve with catastrophic process.cwd() failure', () => {
it('still works', () => {
const originalCwd = process.cwd
process.cwd = () => ''
expect(resolve('.', './')).to.equal('.')
expect(resolve('..', '..')).to.equal('../..')
process.cwd = originalCwd
})
})

runTest('toNamespacedPath', toNamespacedPath, {
// POSIX
'/foo/bar': '/foo/bar',
Expand All @@ -215,7 +274,7 @@ describe('constants', () => {
})

function _s (item) {
return JSON.stringify(_r(item)).replace(/"/g, '\'')
return (JSON.stringify(_r(item)) || 'undefined').replace(/"/g, '\'')
}

function _r (item) {
Expand Down