From e09eed0fc29709ddafb0aef64ab903270c454d5a Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Wed, 28 Jun 2023 11:15:20 -0700 Subject: [PATCH] Honor extension-provided uriBase (#508) * Part 1: move uriBase logic into uriRebaser * Step 2: Add translations * Disable more schemes * Also add translations for workspace prefixes * Add support for uriBase * Remove overrideUriBase * Correct decorations URI checking * Fix URI rebasing * Relax strategy restrictions * Clean up an any * Remove now-unused Array.commonLength * Allow file system strategy to work with filepaths * Update localUri to be a real Uri * Check uriBase before other strategies * uriBase must be an absolute URI * Remove bad comment * Remove unused url-join * Fix compile * Can't use Uris as map keys --- package-lock.json | 24 --- package.json | 2 - src/extension/index.activateDecorations.ts | 13 +- src/extension/index.activateFixes.ts | 12 +- src/extension/index.ts | 6 +- src/extension/panel.ts | 14 +- src/extension/platformUriNormalize.ts | 6 +- src/extension/uriExists.ts | 4 +- src/extension/uriRebaser.spec.ts | 8 - src/extension/uriRebaser.ts | 199 +++++++++++---------- src/panel/details.tsx | 2 +- src/panel/indexStore.ts | 8 +- src/shared/extension.spec.ts | 15 -- src/shared/extension.ts | 13 -- src/shared/index.ts | 31 +--- 15 files changed, 146 insertions(+), 211 deletions(-) diff --git a/package-lock.json b/package-lock.json index 226768b..7e5b8f1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -22,7 +22,6 @@ "react-markdown": "^5.0.3", "semver": "7.5.2", "tmp": "0.1.0", - "url-join": "4.0.1", "vscode-codicons": "0.0.2", "vscode-extension-telemetry": "0.1.6", "vscode-uri": "2.1.2" @@ -42,7 +41,6 @@ "@types/semver": "7.1.0", "@types/sinon": "9.0.4", "@types/tmp": "0.1.0", - "@types/url-join": "4.0.0", "@types/vscode": "1.57", "@typescript-eslint/eslint-plugin": "3.1.0", "@typescript-eslint/parser": "3.1.0", @@ -1044,12 +1042,6 @@ "resolved": "https://registry.npmjs.org/@types/unist/-/unist-2.0.6.tgz", "integrity": "sha512-PBjIUxZHOuj0R15/xuwJYjFi+KZdNFrehocChv4g5hu6aFroHue8m0lBP0POdK2nKzbw0cgV1mws8+V/JAcEkQ==" }, - "node_modules/@types/url-join": { - "version": "4.0.0", - "resolved": "https://registry.npmjs.org/@types/url-join/-/url-join-4.0.0.tgz", - "integrity": "sha512-awrJu8yML4E/xTwr2EMatC+HBnHGoDxc2+ImA9QyeUELI1S7dOCIZcyjki1rkwoA8P2D2NVgLAJLjnclkdLtAw==", - "dev": true - }, "node_modules/@types/vscode": { "version": "1.57.1", "resolved": "https://registry.npmjs.org/@types/vscode/-/vscode-1.57.1.tgz", @@ -10740,11 +10732,6 @@ "querystring": "0.2.0" } }, - "node_modules/url-join": { - "version": "4.0.1", - "resolved": "https://registry.npmjs.org/url-join/-/url-join-4.0.1.tgz", - "integrity": "sha512-jk1+QP6ZJqyOiuEI9AEWQfju/nB2Pw466kbA0LEZljHwKeMgd9WrAEgEGxjPDD2+TNbbb37rTyhEfrCXfuKXnA==" - }, "node_modules/url/node_modules/punycode": { "version": "1.3.2", "resolved": "https://registry.npmjs.org/punycode/-/punycode-1.3.2.tgz", @@ -12648,12 +12635,6 @@ "resolved": "https://registry.npmjs.org/@types/unist/-/unist-2.0.6.tgz", "integrity": "sha512-PBjIUxZHOuj0R15/xuwJYjFi+KZdNFrehocChv4g5hu6aFroHue8m0lBP0POdK2nKzbw0cgV1mws8+V/JAcEkQ==" }, - "@types/url-join": { - "version": "4.0.0", - "resolved": "https://registry.npmjs.org/@types/url-join/-/url-join-4.0.0.tgz", - "integrity": "sha512-awrJu8yML4E/xTwr2EMatC+HBnHGoDxc2+ImA9QyeUELI1S7dOCIZcyjki1rkwoA8P2D2NVgLAJLjnclkdLtAw==", - "dev": true - }, "@types/vscode": { "version": "1.57.1", "resolved": "https://registry.npmjs.org/@types/vscode/-/vscode-1.57.1.tgz", @@ -20248,11 +20229,6 @@ } } }, - "url-join": { - "version": "4.0.1", - "resolved": "https://registry.npmjs.org/url-join/-/url-join-4.0.1.tgz", - "integrity": "sha512-jk1+QP6ZJqyOiuEI9AEWQfju/nB2Pw466kbA0LEZljHwKeMgd9WrAEgEGxjPDD2+TNbbb37rTyhEfrCXfuKXnA==" - }, "util-deprecate": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/util-deprecate/-/util-deprecate-1.0.2.tgz", diff --git a/package.json b/package.json index 2952a86..db8fd07 100644 --- a/package.json +++ b/package.json @@ -161,7 +161,6 @@ "@types/semver": "7.1.0", "@types/sinon": "9.0.4", "@types/tmp": "0.1.0", - "@types/url-join": "4.0.0", "@types/vscode": "1.57", "@typescript-eslint/eslint-plugin": "3.1.0", "@typescript-eslint/parser": "3.1.0", @@ -202,7 +201,6 @@ "react-markdown": "^5.0.3", "semver": "7.5.2", "tmp": "0.1.0", - "url-join": "4.0.1", "vscode-codicons": "0.0.2", "vscode-extension-telemetry": "0.1.6", "vscode-uri": "2.1.2" diff --git a/src/extension/index.activateDecorations.ts b/src/extension/index.activateDecorations.ts index 0bfd462..810e985 100644 --- a/src/extension/index.activateDecorations.ts +++ b/src/extension/index.activateDecorations.ts @@ -5,16 +5,17 @@ import { diffChars } from 'diff'; import { IArraySplice, observable, observe } from 'mobx'; import { Log } from 'sarif'; -import { Disposable, languages, Range, ThemeColor, window, workspace } from 'vscode'; +import { Disposable, languages, Range, ThemeColor, window } from 'vscode'; import { findResult, parseArtifactLocation, ResultId } from '../shared'; import '../shared/extension'; import { getOriginalDoc } from './getOriginalDoc'; import { driftedRegionToSelection } from './regionToSelection'; import { ResultDiagnostic } from './resultDiagnostic'; import { Store } from './store'; +import { UriRebaser } from './uriRebaser'; // Decorations are for Analysis Steps. -export function activateDecorations(disposables: Disposable[], store: Store) { +export function activateDecorations(disposables: Disposable[], store: Store, baser: UriRebaser) { // Navigating away from a diagnostic/result will not clear the `activeResultId`. // This keeps the decorations "pinned" while users navigate the thread flow steps. const activeResultId = observable.box(); @@ -71,11 +72,9 @@ export function activateDecorations(disposables: Disposable[], store: Store) { const currentDoc = editor.document; const locations = result.codeFlows?.[0]?.threadFlows?.[0]?.locations ?? []; - const docUriString = currentDoc.uri.toString(); - const locationsInDoc = locations.filter(tfl => { - const workspaceUri = workspace.workspaceFolders?.[0]?.uri.toString(); // TODO: Handle multiple workspaces. - const [artifactUriString] = parseArtifactLocation(result, tfl.location?.physicalLocation?.artifactLocation, workspaceUri); - return docUriString === artifactUriString; + const locationsInDoc = locations.filter(async tfl => { + const [artifactUriString] = parseArtifactLocation(result, tfl.location?.physicalLocation?.artifactLocation); + return await baser.translateLocalToArtifact(currentDoc.uri) === artifactUriString; }); const originalDoc = await getOriginalDoc(store.analysisInfo?.commit_sha, currentDoc); diff --git a/src/extension/index.activateFixes.ts b/src/extension/index.activateFixes.ts index 9d94e81..f0fede8 100644 --- a/src/extension/index.activateFixes.ts +++ b/src/extension/index.activateFixes.ts @@ -4,7 +4,7 @@ import { diffChars } from 'diff'; import { Fix, Result } from 'sarif'; -import { CodeAction, CodeActionKind, Diagnostic, Disposable, languages, Uri, workspace, WorkspaceEdit } from 'vscode'; +import { CodeAction, CodeActionKind, Diagnostic, Disposable, languages, workspace, WorkspaceEdit } from 'vscode'; import { parseArtifactLocation } from '../shared'; import { getOriginalDoc } from './getOriginalDoc'; import { driftedRegionToSelection } from './regionToSelection'; @@ -46,18 +46,20 @@ export function activateFixes(disposables: Disposable[], store: Pick { if (doc.uri.scheme === 'sarif') { return doc.uri.toString(); } - return await baser.translateLocalToArtifact(doc.uri.toString()); + return baser.translateLocalToArtifact(doc.uri); })(); const severities = { error: DiagnosticSeverity.Error, diff --git a/src/extension/panel.ts b/src/extension/panel.ts index 0d596ed..236c588 100644 --- a/src/extension/panel.ts +++ b/src/extension/panel.ts @@ -123,14 +123,14 @@ export class Panel { break; } case 'select': { - const {logUri, uri, region} = message as { logUri: string, uri: string, region: Region}; - const [_, runIndex, resultIndex] = message.id as ResultId; + const {logUri, uri, uriBase, region} = message as { logUri: string, uri: string, uriBase: string | undefined, region: Region}; + const [_, runIndex] = message.id as ResultId; const log = store.logs.find(log => log._uri === logUri); if (!log) return; const versionControlProvenance = log.runs[runIndex].versionControlProvenance; - const validatedUri = await basing.translateArtifactToLocal(uri, versionControlProvenance); + const validatedUri = await basing.translateArtifactToLocal(uri, uriBase, versionControlProvenance); if (!validatedUri) return; await this.selectLocal(logUri, validatedUri, region); break; @@ -140,9 +140,9 @@ export class Panel { const log = store.logs.find(log => log._uri === logUri); if (!log) return; - const logUriUpgraded = log._uriUpgraded ?? log._uri; + const logUriUpgraded = Uri.parse(log._uriUpgraded ?? log._uri, true); if (!log._jsonMap) { - const file = fs.readFileSync(Uri.parse(logUriUpgraded).fsPath, 'utf8') // Assume scheme file. + const file = fs.readFileSync(logUriUpgraded.fsPath, 'utf8') // Assume scheme file. .replace(/^\uFEFF/, ''); // Trim BOM. log._jsonMap = (jsonMap.parse(file) as { pointers: JsonMap }).pointers; } @@ -178,7 +178,7 @@ export class Panel { }, undefined, context.subscriptions); } - public async selectLocal(logUri: string, localUri: string, region: Region | undefined) { + public async selectLocal(logUri: string, localUri: Uri, region: Region | undefined) { // Keep/pin active Log as needed for (const editor of window.visibleTextEditors.slice()) { if (editor.document.uri.toString() !== logUri) continue; @@ -186,7 +186,7 @@ export class Panel { await commands.executeCommand('workbench.action.keepEditor'); } - const currentDoc = await workspace.openTextDocument(Uri.parse(localUri, true)); + const currentDoc = await workspace.openTextDocument(localUri); // `disableSelectionSync` prevents a selection sync feedback loop in cases where: // 1) `showTextDocument` creates a new editor (where no editor was already open). diff --git a/src/extension/platformUriNormalize.ts b/src/extension/platformUriNormalize.ts index 9be0da9..81c4237 100644 --- a/src/extension/platformUriNormalize.ts +++ b/src/extension/platformUriNormalize.ts @@ -4,9 +4,9 @@ import { Uri } from 'vscode'; import platform from './platform'; -export default function(uri: string): string { - if (platform === 'win32' && Uri.parse(uri).scheme === 'file') { - return uri.toLowerCase(); +export default function(uri: Uri): Uri { + if (platform === 'win32' && uri.scheme === 'file') { + return Uri.parse(uri.toString().toLowerCase(), true); } return uri; } diff --git a/src/extension/uriExists.ts b/src/extension/uriExists.ts index fbb1981..7025504 100644 --- a/src/extension/uriExists.ts +++ b/src/extension/uriExists.ts @@ -5,9 +5,9 @@ import { Uri, workspace } from 'vscode'; // Hacky: We are using `fs.stat` to test the existence of documents as VS Code does not provide a dedicated existence API. // The similar Node `fs` API does not resolve custom URI schemes in the same way that VS Code does otherwise we would use that. -export default async function uriExists(absoluteUri: string) { +export default async function uriExists(absoluteUri: Uri) { try { - await workspace.fs.stat(Uri.parse(absoluteUri, true)); + await workspace.fs.stat(absoluteUri); } catch (error) { return false; } diff --git a/src/extension/uriRebaser.spec.ts b/src/extension/uriRebaser.spec.ts index fb2c23d..f23ad51 100644 --- a/src/extension/uriRebaser.spec.ts +++ b/src/extension/uriRebaser.spec.ts @@ -15,14 +15,6 @@ describe('baser', () => { './platform': 'darwin', }); - it('Array.commonLength', () => { - const commonLength = Array.commonLength( - ['a', 'b', 'c'], - ['a', 'b', 'd'] - ); - assert.strictEqual(commonLength, 2); - }); - it('translates uris - local -> artifact - case-insensitive file system', async () => { // Spaces inserted to emphasize common segments. const artifactUri = 'file:// /a/b'.replace(/ /g, ''); diff --git a/src/extension/uriRebaser.ts b/src/extension/uriRebaser.ts index e4bc887..c11e81b 100644 --- a/src/extension/uriRebaser.ts +++ b/src/extension/uriRebaser.ts @@ -12,28 +12,21 @@ import * as fs from 'fs'; import * as os from 'os'; import fetch from 'node-fetch'; -/** - * Splits a URI into path segments. Scheme+authority considered a "segment" for practical purposes. - * Query and fragment are current ignored until we have a concrete use case. - * @param uri - An absolute URI. - */ -function splitUri(uri: string | undefined) { - if (uri === undefined) return []; - const { scheme, authority, path } = Uri.parse(uri, true); - return [`${scheme}://${authority}`, ...path.slice(1).split('/')]; // By spec first '/' always exists, thus safe to slice(1). -} +const workspaceDistinctFilenameCache: Map = new Map(); -const workspaceDistinctFilenameCache: Map = new Map(); - -async function workspaceHasDistinctFilename(filename: string): Promise { - if (workspaceDistinctFilenameCache.has(filename)) { - return workspaceDistinctFilenameCache.get(filename); +async function workspaceHasDistinctFilename(filename: string): Promise { + const distinctFileName = workspaceDistinctFilenameCache.get(filename); + if (distinctFileName !== undefined) { + return distinctFileName; } - const matches = await workspace.findFiles(`**/${filename}`); // Is `.git` folder excluded? - const result = matches.length === 1 ? matches[0].toString() : undefined; - workspaceDistinctFilenameCache.set(filename, result); - return result; + const matches = await workspace.findFiles(`**/${filename}`); // Is `.git` folder excluded? + if (matches.length === 1) { + workspaceDistinctFilenameCache.set(filename, matches[0]); + return matches[0]; + } + + return undefined; } workspace.onDidCreateFiles(async (event) => { @@ -66,18 +59,25 @@ export class UriRebaser { } private basesArtifactToLocal = new Map() // - private updateBases(artifact: string[], local: string[]) { - const i = Array.commonLength(artifact.slice().reverse(), local.slice().reverse()); - this.basesArtifactToLocal.set( - artifact.slice(0, -i).join('/'), - local.slice(0, -i).join('/')); + private updateBases(artifact: string, local: Uri) { + const localPath = local.toString(); + let commonLength = 0; + while ( + commonLength < artifact.length && + commonLength < localPath.length && + artifact[artifact.length - commonLength - 1] === localPath[localPath.length - commonLength - 1]) { + commonLength++; + } + this.basesArtifactToLocal.set(artifact.slice(0, -commonLength), localPath.slice(0, -commonLength)); } - private validatedUrisArtifactToLocal = new Map() + private validatedUrisArtifactToLocal = new Map() private validatedUrisLocalToArtifact = new Map() - private updateValidatedUris(artifact: string, local: string) { + private updateValidatedUris(artifact: string, local: Uri) { this.validatedUrisArtifactToLocal.set(artifact, local); - this.validatedUrisLocalToArtifact.set(local, artifact); + + // Maps use reference equality so we can't use Uri objects as keys. + this.validatedUrisLocalToArtifact.set(local.toString(), artifact); } // Other possibilities: @@ -88,10 +88,10 @@ export class UriRebaser { // Notes: // If 2 logs have the same uri, then likely the same (unless the uri is super short) // If 2 logs don't have the same uri, they can still potentially be the same match - public async translateLocalToArtifact(localUri: string): Promise { // Future: Ret undefined when certain. + public async translateLocalToArtifact(localUri: Uri): Promise { // Need to refresh on uri map update. - if (!this.validatedUrisLocalToArtifact.has(localUri)) { - const { file } = platformUriNormalize(localUri); + if (!this.validatedUrisLocalToArtifact.has(localUri.toString())) { + const { file } = platformUriNormalize(localUri).toString(); // If no workspace then we choose to over-assume the localUri in-question is unique. It usually is, // but obviously can't always be true. @@ -102,48 +102,81 @@ export class UriRebaser { const artifactUri = this.store.distinctArtifactNames.get(file)!; // Not undefined due to surrounding if. this.updateValidatedUris(artifactUri, localUri); - this.updateBases(splitUri(artifactUri), splitUri(localUri)); + this.updateBases(artifactUri, localUri); } } - return this.validatedUrisLocalToArtifact.get(localUri) ?? localUri; + return this.validatedUrisLocalToArtifact.get(localUri.toString()); } private extensionName = 'sarif-viewer' private trustedSourceSitesConfigSection = 'trustedSourceSites'; private trustedSites = workspace.getConfiguration(this.extensionName).get(this.trustedSourceSitesConfigSection, []); private activeInfoMessages = new Set() // Prevent repeat message animations when arrowing through many results with the same uri. - public async translateArtifactToLocal(artifactUri: string, versionControlProvenance?: VersionControlDetails[]) { // Retval is validated. - if (Uri.parse(artifactUri, true).scheme === 'sarif') return artifactUri; // Sarif-scheme URIs are owned/created by us, so we know they exist. - const validateUri = async () => { + public async translateArtifactToLocal(artifactUri: string, uriBase: string | undefined, versionControlProvenance?: VersionControlDetails[]): Promise { // Retval is validated. + // Sarif-scheme URIs are owned/created by us, so we know they exist. + if (artifactUri.startsWith('sarif://')) return Uri.parse(artifactUri, true); + + const validateUri = async (): Promise => { // Cache - if (this.validatedUrisArtifactToLocal.has(artifactUri)) - return this.validatedUrisArtifactToLocal.get(artifactUri)!; + const artifact = this.validatedUrisArtifactToLocal.get(artifactUri); + if (artifact) + return artifact; - // File System Exist - if (await uriExists(artifactUri)) - return artifactUri; + const rxUriScheme = /^([^:/?#]+?):/; + const isRelative = !rxUriScheme.test(artifactUri); + if (isRelative) { + // §3.4.4: + // If the end user has configured the SARIF consumer with a value for the uriBaseId... + // then the consumer SHALL use the configured value + for (const uriBase of this.uriBases) { + const localUri = Uri.joinPath(Uri.parse(uriBase, true), artifactUri); + if (await uriExists(localUri)) { + this.updateValidatedUris(artifactUri, localUri); + return localUri; + } + } - // File System Exist with Workspace prefixed - const workspaceUri = workspace.workspaceFolders?.[0]?.uri.toString(); // TODO: Handle multiple workspaces. - if (workspaceUri) { - const workspaceArtifactUri = `${workspaceUri}/${artifactUri.replace('file:///', '')}`; - if (await uriExists(workspaceArtifactUri)) - return workspaceArtifactUri; - } + // If uriBaseId is not yet resolved and theRun.originalUriBaseIds (§3.14.14) is present, + // the consumer SHALL attempt to resolve the uriBaseId from the information in originalUriBaseIds + if (uriBase) { + const localUri = Uri.joinPath(Uri.parse(uriBase, true), artifactUri); + if (await uriExists(localUri)) { + this.updateValidatedUris(artifactUri, localUri); + return localUri; + } + } - // Known Bases - for (const [artifactBase, localBase] of this.basesArtifactToLocal) { - if (!artifactUri.startsWith(artifactBase)) continue; // Just let it fall through? - const localUri = artifactUri.replace(artifactBase, localBase); + // If uriBaseId is not yet resolved, + // the consumer MAY use other information or heuristics to locate the artifact. + + // File System Exist with Workspace prefixed + const workspaceUri = workspace.workspaceFolders?.[0]?.uri; // TODO: Handle multiple workspaces. + if (workspaceUri) { + const localUri = Uri.joinPath(workspaceUri, artifactUri); + if (await uriExists(localUri)) { + this.updateValidatedUris(artifactUri, localUri); + return localUri; + } + } + } else { + // File System Exist + const localUri = Uri.parse(artifactUri); if (await uriExists(localUri)) { this.updateValidatedUris(artifactUri, localUri); return localUri; } } - { // API-injected baseUris - const localUri = await this.tryUriBases(artifactUri); - if (localUri) return localUri; + // These strategies make sense regardless if the URI is relative or absolute + + // Known Bases + for (const [artifactBase, localBase] of this.basesArtifactToLocal) { + if (!artifactUri.startsWith(artifactBase)) continue; // Just let it fall through? + const localUri = Uri.parse(artifactUri.replace(artifactBase, localBase), true); + if (await uriExists(localUri)) { + this.updateValidatedUris(artifactUri, localUri); + return localUri; + } } // Distinct Project Items @@ -152,20 +185,20 @@ export class UriRebaser { if (distinctFilename && this.store.distinctArtifactNames.has(file)) { const localUri = distinctFilename; this.updateValidatedUris(artifactUri, localUri); - this.updateBases(splitUri(artifactUri), splitUri(localUri)); + this.updateBases(artifactUri, localUri); return localUri; } // Open Docs for (const doc of workspace.textDocuments) { - const localUri = doc.uri.toString(); - if (localUri.file !== artifactUri.file) continue; + const localUri = doc.uri; + if (localUri.toString().file !== artifactUri.file) continue; this.updateValidatedUris(artifactUri, localUri); - this.updateBases(splitUri(artifactUri), splitUri(localUri)); + this.updateBases(artifactUri, localUri); return localUri; } - return ''; // Signals inability to rebase. + return undefined; // Signals inability to rebase. }; let validatedUri = await validateUri(); @@ -181,9 +214,9 @@ export class UriRebaser { const root = os.tmpdir().endsWith(path.sep) ? os.tmpdir() : `${os.tmpdir()}${path.sep}`; const fileName = path.join(root, url.pathname).normalize(); if (!fileName.startsWith(root)) - return ''; + return undefined; - const fileUrl = `file:///${fileName.replace(/\\/g, '/')}`; + const fileUrl = Uri.file(fileName); // check if the file was already downloaded if (await uriExists(fileUrl)) return fileUrl; @@ -212,7 +245,7 @@ export class UriRebaser { if (choice === 'Yes' || choice === alwaysMsg) { const mkdirRecursive = async (dir: string) => { return new Promise((resolve, reject) => { - fs.mkdir(dir, { recursive: true }, (error: any) => { + fs.mkdir(dir, { recursive: true }, (error) => { if (error) { reject(error); } else { @@ -229,9 +262,7 @@ export class UriRebaser { await mkdirRecursive(dir); await fs.promises.writeFile(fileName, buffer); - const partsOld = splitUri(artifactUri); - const partsNew = splitUri(`file://${fileName.replace(/\\/g, '/')}`); - this.updateBases(partsOld, partsNew); + this.updateBases(artifactUri, fileUrl); return fileUrl; } catch (error: any) { @@ -252,43 +283,21 @@ export class UriRebaser { filters: { 'Matching file' : [extension] }, // Consider allowing folders. }); - if (!files?.length) return ''; // User cancelled. + if (!files?.length) return undefined; // User cancelled. - const partsOld = splitUri(artifactUri); - const partsNew = splitUri(files[0].toString()); - if (partsOld.last !== partsNew.last) { - void window.showErrorMessage(`File names must match: "${partsOld.last}" and "${partsNew.last}"`); - return ''; + this.updateBases(artifactUri, files[0]); + + const artifactFile = artifactUri.file; + const localFile = files[0].toString().file; + if (artifactFile !== localFile) { + void window.showErrorMessage(`File names must match: "${artifactFile}" and "${localFile}"`); + return undefined; } - this.updateBases(partsOld, partsNew); } validatedUri = await validateUri(); // Try again } return validatedUri; } - public static *commonIndices(a: T[], b: T[]) { // Add comparator? - for (const [aIndex, aPart] of a.entries()) { - for (const [bIndex, bPart] of b.entries()) { - if (aPart === bPart) yield [aIndex, bIndex]; - } - } - } - public uriBases = [] as string[] - private async tryUriBases(artifactUri: string) { - const artifactParts = splitUri(artifactUri); - for (const localUriBase of this.uriBases) { - const localParts = splitUri(localUriBase); - for (const [artifactIndex, localIndex] of UriRebaser.commonIndices(artifactParts, localParts)) { - const rebased = [...localParts.slice(0, localIndex), ...artifactParts.slice(artifactIndex)].join('/'); - if (await uriExists(rebased)) { - this.updateValidatedUris(artifactUri, localUriBase); - this.updateBases(artifactParts, localParts); - return rebased; - } - } - } - return undefined; - } } diff --git a/src/panel/details.tsx b/src/panel/details.tsx index 4b7bc3d..3f9221f 100644 --- a/src/panel/details.tsx +++ b/src/panel/details.tsx @@ -78,7 +78,7 @@ interface DetailsProps { result: Result, resultsFixed: string[], height: IObserv Locations {result.locations?.map((loc, i) => { const ploc = loc.physicalLocation; - const [uri, _] = parseArtifactLocation(result, ploc?.artifactLocation); + const [uri] = parseArtifactLocation(result, ploc?.artifactLocation); return { e.preventDefault(); // Cancel # nav. diff --git a/src/panel/indexStore.ts b/src/panel/indexStore.ts index 7fae97e..8a5d653 100644 --- a/src/panel/indexStore.ts +++ b/src/panel/indexStore.ts @@ -56,7 +56,7 @@ export class IndexStore { const selectedRow = this.selection.get(); const result = selectedRow instanceof RowItem && selectedRow.item; if (!result?._uri) return; // Bail on no result or location-less result. - postSelectArtifact(result, result.locations?.[0]?.physicalLocation, workspaceUri); + postSelectArtifact(result, result.locations?.[0]?.physicalLocation); }); } @@ -149,7 +149,7 @@ export async function postLoad() { await vscode.postMessage({ command: 'load' }); } -export async function postSelectArtifact(result: Result, ploc?: PhysicalLocation, overrideUriBase?: string) { +export async function postSelectArtifact(result: Result, ploc?: PhysicalLocation) { // If this panel is not active, then any selection change did not originate from (a user's action) here. // It must have originated from (a user's action in) the editor, which then sent a message here. // If that is the case, don't send another 'select' message back. This would cause selection unstability. @@ -160,9 +160,9 @@ export async function postSelectArtifact(result: Result, ploc?: PhysicalLocation if (!ploc) return; const log = result._log; const logUri = log._uri; - const [uri, uriContent] = parseArtifactLocation(result, ploc?.artifactLocation, overrideUriBase); + const [uri, uriBase, uriContent] = parseArtifactLocation(result, ploc?.artifactLocation); const region = ploc?.region; - await vscode.postMessage({ command: 'select', logUri, uri: uriContent ?? uri, region, id: result._id }); + await vscode.postMessage({ command: 'select', logUri, uri: uriContent ?? uri, uriBase, region, id: result._id }); } export async function postSelectLog(result: Result) { diff --git a/src/shared/extension.spec.ts b/src/shared/extension.spec.ts index 27b2f03..de37c6e 100644 --- a/src/shared/extension.spec.ts +++ b/src/shared/extension.spec.ts @@ -5,21 +5,6 @@ import assert from 'assert'; describe('Extension', () => { - describe('Array.commonLength', () => { - it('finds the length of common consecutive elements when common elements start from index 0', () => { - assert.strictEqual(Array.commonLength(['a', 'b', 'c'], ['a', 'b', 'd', 'e']), 2); - }); - it('returns zero if common consecutive elements do not start from 0 index', () => { - assert.strictEqual(Array.commonLength(['a', 'b', 'c'], ['b', 'c', 'd']), 0); - }); - it('returns zero if no common consecutive elements', () => { - assert.strictEqual(Array.commonLength(['a', 'b', 'c'], ['x', 'y', 'z']), 0); - }); - it('returns zero if one array is empty', () => { - assert.strictEqual(Array.commonLength(['a', 'b', 'c'], []), 0); - assert.strictEqual(Array.commonLength([], ['a', 'b']), 0); - }); - }); describe('Array.prototype.last', () => { it('finds the last element when more than 1 elements are present', () => { assert.strictEqual(['a', 'b', 'c'].last, 'c'); diff --git a/src/shared/extension.ts b/src/shared/extension.ts index 3698b0e..f35a0e9 100644 --- a/src/shared/extension.ts +++ b/src/shared/extension.ts @@ -11,9 +11,6 @@ export {}; type Selector = (_: T) => number | string declare global { - interface ArrayConstructor { - commonLength(a: any[], b: any[]): number; - } interface Array { last: T; replace(items: T[]): void; // From Mobx, but not showing up. @@ -27,16 +24,6 @@ declare global { } } -!Array.hasOwnProperty('commonLength') && -Object.defineProperty(Array, 'commonLength', { - value: function(a: any[], b: any[]): number { - let i = 0; - // eslint-disable-next-line no-empty - for (; a[i] === b[i] && i < a.length && i < b.length; i++) {} - return i; - } -}); - !Array.prototype.hasOwnProperty('last') && Object.defineProperty(Array.prototype, 'last', { get: function() { diff --git a/src/shared/index.ts b/src/shared/index.ts index b69bb38..20cd857 100644 --- a/src/shared/index.ts +++ b/src/shared/index.ts @@ -2,7 +2,6 @@ // Licensed under the MIT License. import { ArtifactLocation, Location, Log, ReportingDescriptor, Result } from 'sarif'; -import urlJoin from 'url-join'; import { URI } from 'vscode-uri'; type JsonLocation = { line: number, column: number } // Unused: pos @@ -98,7 +97,7 @@ export function augmentLog(log: Log, rules?: Map, w result._id = [log._uri, runIndex, resultIndex]; const ploc = result.locations?.[0]?.physicalLocation; - const [uri, uriContents] = parseArtifactLocation(result, ploc?.artifactLocation, workspaceUri); + const [uri, _, uriContents] = parseArtifactLocation(result, ploc?.artifactLocation); result._uri = uri; result._uriContents = uriContents; result._relativeUri = result._uri?.replace(workspaceUri ?? '' , '') ?? ''; // For grouping, Empty works more predictably than undefined @@ -170,40 +169,26 @@ Run.artifacts: Art[] location: ArtLoc contents: ArtCon */ -export function parseLocation(result: Result, loc?: Location, overrideUriBase?: string) { +export function parseLocation(result: Result, loc?: Location) { const message = loc?.message?.text; - const [uri, uriContent] = parseArtifactLocation(result, loc?.physicalLocation?.artifactLocation, overrideUriBase); + const [uri, _, uriContent] = parseArtifactLocation(result, loc?.physicalLocation?.artifactLocation); const region = loc?.physicalLocation?.region; return { message, uri, uriContent, region }; } // Improve: `result` purely used for `_run.artifacts`. -export function parseArtifactLocation(result: Pick, anyArtLoc: ArtifactLocation | undefined, overrideUriBase?: string) { - if (!anyArtLoc) return [undefined, undefined]; +export function parseArtifactLocation(result: Pick, anyArtLoc: ArtifactLocation | undefined) { + if (!anyArtLoc) return [undefined, undefined, undefined]; const runArt = result._run.artifacts?.[anyArtLoc.index ?? -1]; const runArtLoc = runArt?.location; const runArtCon = runArt?.contents; - let uri = anyArtLoc.uri ?? runArtLoc?.uri ?? ''; // If index (§3.4.5) is absent, uri SHALL be present. + const uri = anyArtLoc.uri ?? runArtLoc?.uri ?? ''; // If index (§3.4.5) is absent, uri SHALL be present. // Currently not supported: recursive resolution of uriBaseId. // Note: While an uriBase often results in an absolute URI, there is no guarantee. // Note: While an uriBase often represents the project root, there is no guarantee. const uriBaseId = anyArtLoc.uriBaseId ?? runArtLoc?.uriBaseId; - if (uriBaseId) { - const uriBase - = overrideUriBase // Typically the workspaceUri, which takes precedence. - ?? result._run.originalUriBaseIds?.[uriBaseId]?.uri - ?? ''; - uri = urlJoin(uriBase, uri); - } - - // Determine if `uri` absolute or relative. Using scheme as an approximation. - const rxUriScheme = /^([^:/?#]+?):/; - const isRelative = !rxUriScheme.test(uri); - if (isRelative) { - uri = urlJoin(overrideUriBase ?? 'file://', uri); - // After this point, the URI must be absolute. - } + const uriBase = uriBaseId ? result._run.originalUriBaseIds?.[uriBaseId]?.uri : undefined; // A shorter more transparent URI format would be: // `sarif://${encodeURIComponent(result._log._uri)}/${result._run._index}/${anyArtLoc.index}/${uri?.file ?? 'Untitled'}` @@ -212,7 +197,7 @@ export function parseArtifactLocation(result: Pick, any const uriContents = runArtCon?.text || runArtCon?.binary ? encodeURI(`sarif:${encodeURIComponent(result._log._uri)}/${result._run._index}/${anyArtLoc.index}/${uri?.file ?? 'Untitled'}`) : undefined; - return [uri, uriContents]; + return [uri, uriBase, uriContents]; } export function decodeFileUri(uriString: string) {