From cc72d79fc2e3ebcd1b3f73ac2e68d17a26add779 Mon Sep 17 00:00:00 2001 From: Simon Abbott Date: Wed, 17 Aug 2022 11:10:15 -0500 Subject: [PATCH] fix: don't use resolved paths if they don't actually exist Previously `toFilePath` would treat _any_ absolute path that doesn't start with the `root` (usually the current working directory) as a relative path, which resulted in some seriously mangled paths. Now we actually check if that resolved path exists before committing to it. This fixes #1864. --- packages/vite-node/src/utils.ts | 20 +++++---- pnpm-lock.yaml | 41 ++++++++++--------- test/core/test/file-path.test.ts | 27 ++++++++++++ test/path-res-target/.eslintignore | 1 + test/path-res-target/.gitignore | 1 + test/path-res-target/README.md | 2 + test/path-res-target/dist/index.d.ts | 1 + test/path-res-target/dist/index.js | 3 ++ test/path-res-target/package.json | 6 +++ test/path-resolution/package.json | 15 +++++++ .../test/sister-package.test.ts | 17 ++++++++ test/path-resolution/vitest.config.ts | 7 ++++ 12 files changed, 115 insertions(+), 26 deletions(-) create mode 100644 test/path-res-target/.eslintignore create mode 100644 test/path-res-target/.gitignore create mode 100644 test/path-res-target/README.md create mode 100644 test/path-res-target/dist/index.d.ts create mode 100644 test/path-res-target/dist/index.js create mode 100644 test/path-res-target/package.json create mode 100644 test/path-resolution/package.json create mode 100644 test/path-resolution/test/sister-package.test.ts create mode 100644 test/path-resolution/vitest.config.ts diff --git a/packages/vite-node/src/utils.ts b/packages/vite-node/src/utils.ts index b5c2d933d59e..e24a36fde9ed 100644 --- a/packages/vite-node/src/utils.ts +++ b/packages/vite-node/src/utils.ts @@ -1,4 +1,5 @@ import { fileURLToPath, pathToFileURL } from 'url' +import { existsSync } from 'fs' import { resolve } from 'pathe' import type { TransformResult } from 'vite' import type { Arrayable, Nullable } from './types' @@ -46,13 +47,18 @@ export function isPrimitive(v: any) { } export function toFilePath(id: string, root: string): string { - let absolute = id.startsWith('/@fs/') - ? id.slice(4) - : id.startsWith(root) - ? id - : id.startsWith('/') - ? resolve(root, id.slice(1)) - : id + let absolute = (() => { + if (id.startsWith('/@fs/')) + return id.slice(4) + if (!id.startsWith(root) && id.startsWith('/')) { + const resolved = resolve(root, id.slice(1)) + // The resolved path can have query values. Remove them before checking + // the file path. + if (existsSync(resolved.replace(/\?.*$/, ''))) + return resolved + } + return id + })() if (absolute.startsWith('//')) absolute = absolute.slice(1) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 009afe5c4bb8..8cebc95f0f46 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -216,7 +216,7 @@ importers: react-dom: 18.0.0_react@18.0.0 devDependencies: '@testing-library/react': 13.2.0_zpnidt7m3osuk7shl3s4oenomq - '@types/node': 18.7.5 + '@types/node': 18.7.6 '@types/react': 18.0.17 '@vitejs/plugin-react': 2.0.1 jsdom: 20.0.0 @@ -976,6 +976,21 @@ importers: devDependencies: vitest: link:../../packages/vitest + test/path-res-target: + specifiers: {} + + test/path-resolution: + specifiers: + '@edge-runtime/vm': 1.1.0-beta.26 + '@vitest/test-path-res-target': workspace:* + '@vitest/web-worker': workspace:* + vitest: workspace:* + devDependencies: + '@edge-runtime/vm': 1.1.0-beta.26 + '@vitest/test-path-res-target': link:../path-res-target + '@vitest/web-worker': link:../../packages/web-worker + vitest: link:../../packages/vitest + test/related: specifiers: execa: ^6.1.0 @@ -1869,7 +1884,7 @@ packages: resolution: {integrity: sha512-+il1gTy0oHwUsBQZyJvukbB4vPMdcYBrFHa0Uc4AizLxbq6BOYC51Rv4tWocX9BLBDLZ4kc6qUFpQ6HRgL+3zw==} engines: {node: '>=6.9.0'} dependencies: - '@babel/types': 7.18.9 + '@babel/types': 7.18.10 dev: true /@babel/helper-split-export-declaration/7.16.7: @@ -4028,14 +4043,6 @@ packages: '@babel/helper-validator-identifier': 7.16.7 to-fast-properties: 2.0.0 - /@babel/types/7.18.9: - resolution: {integrity: sha512-WwMLAg2MvJmt/rKEVQBBhIVffMmnilX4oe0sRe7iPOHIGsqpruFHHdrfj4O1CMMtgMtCU4oPafZjDPCRgO57Wg==} - engines: {node: '>=6.9.0'} - dependencies: - '@babel/helper-validator-identifier': 7.18.6 - to-fast-properties: 2.0.0 - dev: true - /@base2/pretty-print-object/1.0.1: resolution: {integrity: sha512-4iri8i1AqYHJE2DstZYkyEprg6Pq6sKx3xn5FpySk9sNhH7qN2LLlHJCfDTZRILNwQNPD7mATWM0TBui7uC1pA==} dev: true @@ -6963,7 +6970,7 @@ packages: resolution: {integrity: sha512-9VHgfIatKNXQNaZTtLnalIy0jNZzY35a4S3oi08YAt9Hv1VsfZ/DfA45lM8D/UhtHBGJ4/lGwp0PZkVndRkoOQ==} engines: {node: '>=12'} dependencies: - '@babel/code-frame': 7.16.7 + '@babel/code-frame': 7.18.6 '@babel/runtime': 7.17.9 '@types/aria-query': 4.2.2 aria-query: 5.0.0 @@ -7131,7 +7138,7 @@ packages: /@types/concat-stream/1.6.1: resolution: {integrity: sha512-eHE4cQPoj6ngxBZMvVf6Hw7Mh4jMW4U9lpGmS5GBPB9RYxlFg+CHaVN7ErNY4W9XfLIEn20b4VDYaIrbq0q4uA==} dependencies: - '@types/node': 18.7.4 + '@types/node': 18.7.6 dev: true /@types/cookie/0.4.1: @@ -7173,7 +7180,7 @@ packages: /@types/form-data/0.0.33: resolution: {integrity: sha512-8BSvG1kGm83cyJITQMZSulnl6QV8jqAGreJsc5tPu1Jq0vTSOiY/k24Wx82JRpWwZSqrala6sd5rWi6aNXvqcw==} dependencies: - '@types/node': 18.7.4 + '@types/node': 18.7.6 dev: true /@types/fs-extra/9.0.13: @@ -7325,12 +7332,8 @@ packages: resolution: {integrity: sha512-LJgzOEwWuMTBxHzgBR/fhhBOWrvBjvO+zPteUgbbuQi80rYIZHrk1mNbRUqPZqSLP2H7Rwt1EFLL/tNLD1Xx/w==} dev: true - /@types/node/18.7.4: - resolution: {integrity: sha512-RzRcw8c0B8LzryWOR4Wj7YOTFXvdYKwvrb6xQQyuDfnlTxwYXGCV5RZ/TEbq5L5kn+w3rliHAUyRcG1RtbmTFg==} - dev: true - - /@types/node/18.7.5: - resolution: {integrity: sha512-NcKK6Ts+9LqdHJaW6HQmgr7dT/i3GOHG+pt6BiWv++5SnjtRd4NXeiuN2kA153SjhXPR/AhHIPHPbrsbpUVOww==} + /@types/node/18.7.6: + resolution: {integrity: sha512-EdxgKRXgYsNITy5mjjXjVE/CS8YENSdhiagGrLqjG0pvA2owgJ6i4l7wy/PFZGC0B1/H20lWKN7ONVDNYDZm7A==} dev: true /@types/node/8.10.66: diff --git a/test/core/test/file-path.test.ts b/test/core/test/file-path.test.ts index 2a8f0b629e88..4f4271ce2a9e 100644 --- a/test/core/test/file-path.test.ts +++ b/test/core/test/file-path.test.ts @@ -1,6 +1,9 @@ +import { existsSync } from 'fs' import { describe, expect, it, vi } from 'vitest' import { isWindows, slash, toFilePath } from '../../../packages/vite-node/src/utils' +vi.mock('fs') + describe('toFilePath', () => { // the following tests will work incorrectly on unix systems if (isWindows) { @@ -37,8 +40,10 @@ describe('toFilePath', () => { const expected = '/path/to/project/node_modules/pkg/file.js' const processSpy = vi.spyOn(process, 'cwd').mockReturnValue(root) + const existsSpy = vi.mocked(existsSync).mockReturnValue(true) const filePath = toFilePath(id, root) processSpy.mockRestore() + existsSpy.mockRestore() expect(slash(filePath)).toEqual(expected) }) @@ -49,8 +54,11 @@ describe('toFilePath', () => { const expected = '/path/to/project/node_modules/pkg/file.js' const processSpy = vi.spyOn(process, 'cwd').mockReturnValue(root) + const existsSpy = vi.mocked(existsSync).mockReturnValue(true) const filePath = toFilePath(id, root) processSpy.mockRestore() + existsSpy.mockRestore() + expect(slash(filePath)).toEqual(expected) }) @@ -60,8 +68,10 @@ describe('toFilePath', () => { const expected = '/root/node_modules/pkg/file.js' const processSpy = vi.spyOn(process, 'cwd').mockReturnValue(root) + const existsSpy = vi.mocked(existsSync).mockReturnValue(true) const filePath = toFilePath(id, root) processSpy.mockRestore() + existsSpy.mockRestore() expect(slash(filePath)).toEqual(expected) }) @@ -72,8 +82,10 @@ describe('toFilePath', () => { const expected = '/root/node_modules/pkg/file.js' const processSpy = vi.spyOn(process, 'cwd').mockReturnValue(root) + const existsSpy = vi.mocked(existsSync).mockReturnValue(true) const filePath = toFilePath(id, root) processSpy.mockRestore() + existsSpy.mockRestore() expect(slash(filePath)).toEqual(expected) }) @@ -84,10 +96,25 @@ describe('toFilePath', () => { const expected = '/root/path/to/file.js' const processSpy = vi.spyOn(process, 'cwd').mockReturnValue(root) + const existsSpy = vi.mocked(existsSync).mockReturnValue(true) const filePath = toFilePath(id, root) processSpy.mockRestore() + existsSpy.mockRestore() expect(slash(filePath)).toEqual(expected) }) + + it('unix with sibling path', () => { + const root = '/path/to/first/package' + const id = '/path/to/second/package/file.js' + + const processSpy = vi.spyOn(process, 'cwd').mockReturnValue(root) + const existsSpy = vi.mocked(existsSync).mockReturnValue(false) + const filePath = toFilePath(id, root) + processSpy.mockRestore() + existsSpy.mockRestore() + + expect(slash(filePath)).toEqual(id) + }) } }) diff --git a/test/path-res-target/.eslintignore b/test/path-res-target/.eslintignore new file mode 100644 index 000000000000..af18ca9559d2 --- /dev/null +++ b/test/path-res-target/.eslintignore @@ -0,0 +1 @@ +!dist/ diff --git a/test/path-res-target/.gitignore b/test/path-res-target/.gitignore new file mode 100644 index 000000000000..af18ca9559d2 --- /dev/null +++ b/test/path-res-target/.gitignore @@ -0,0 +1 @@ +!dist/ diff --git a/test/path-res-target/README.md b/test/path-res-target/README.md new file mode 100644 index 000000000000..79839556fa38 --- /dev/null +++ b/test/path-res-target/README.md @@ -0,0 +1,2 @@ +This package is here just to be a sibling package to `path-resolution`. It +contains no tests of its own. diff --git a/test/path-res-target/dist/index.d.ts b/test/path-res-target/dist/index.d.ts new file mode 100644 index 000000000000..3b9981e05940 --- /dev/null +++ b/test/path-res-target/dist/index.d.ts @@ -0,0 +1 @@ +export function sub(a: number, b: number): number diff --git a/test/path-res-target/dist/index.js b/test/path-res-target/dist/index.js new file mode 100644 index 000000000000..cb92ed2e794b --- /dev/null +++ b/test/path-res-target/dist/index.js @@ -0,0 +1,3 @@ +export function sub(a, b) { + return a - b; +} diff --git a/test/path-res-target/package.json b/test/path-res-target/package.json new file mode 100644 index 000000000000..30190d8a37b1 --- /dev/null +++ b/test/path-res-target/package.json @@ -0,0 +1,6 @@ +{ + "name": "@vitest/test-path-res-target", + "type": "module", + "private": true, + "main": "./dist/index.js" +} diff --git a/test/path-resolution/package.json b/test/path-resolution/package.json new file mode 100644 index 000000000000..db64a812ab50 --- /dev/null +++ b/test/path-resolution/package.json @@ -0,0 +1,15 @@ +{ + "name": "@vitest/test-path-resolution", + "type": "module", + "private": true, + "scripts": { + "test": "vitest", + "coverage": "vitest run --coverage" + }, + "devDependencies": { + "@edge-runtime/vm": "1.1.0-beta.26", + "@vitest/test-path-res-target": "workspace:*", + "@vitest/web-worker": "workspace:*", + "vitest": "workspace:*" + } +} diff --git a/test/path-resolution/test/sister-package.test.ts b/test/path-resolution/test/sister-package.test.ts new file mode 100644 index 000000000000..394cfd1771ad --- /dev/null +++ b/test/path-resolution/test/sister-package.test.ts @@ -0,0 +1,17 @@ +import { expect, it, vitest } from 'vitest' + +import { sub } from '@vitest/test-path-res-target' + +vitest.mock('@vitest/test-path-res-target') + +it('should be mocked', () => { + expect(sub).toHaveProperty('mock') + expect(sub(5, 3)).toBeUndefined() +}) + +it('should import actual', async () => { + const { sub } = await vitest.importActual('@vitest/test-path-res-target') + + expect(sub).not.toHaveProperty('mock') + expect(sub(5, 3)).toBe(2) +}) diff --git a/test/path-resolution/vitest.config.ts b/test/path-resolution/vitest.config.ts new file mode 100644 index 000000000000..41bc8c0ff39f --- /dev/null +++ b/test/path-resolution/vitest.config.ts @@ -0,0 +1,7 @@ +import { defineConfig } from 'vite' + +export default defineConfig({ + test: { + include: ['test/*.test.ts'], + }, +})