From 2c1c691b06ce90b9517bd3c5e5f518a130c18939 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Fri, 18 Oct 2019 12:37:25 +0200 Subject: [PATCH] uri - remove strict mode, use file when scheme is missing --- src/vs/base/common/uri.ts | 48 +++++-------------- src/vs/monaco.d.ts | 3 +- .../api/common/extHostTypeConverters.ts | 10 ++-- .../extensions/common/extensionHostMain.ts | 6 +-- .../api/extHostTypeConverter.test.ts | 6 ++- 5 files changed, 24 insertions(+), 49 deletions(-) diff --git a/src/vs/base/common/uri.ts b/src/vs/base/common/uri.ts index 966555e04d0..7df9153b440 100644 --- a/src/vs/base/common/uri.ts +++ b/src/vs/base/common/uri.ts @@ -10,26 +10,11 @@ const _schemePattern = /^\w[\w\d+.-]*$/; const _singleSlashStart = /^\//; const _doubleSlashStart = /^\/\//; -let _throwOnMissingSchema: boolean = true; - -/** - * @internal - */ -export function setUriThrowOnMissingScheme(value: boolean): boolean { - const old = _throwOnMissingSchema; - _throwOnMissingSchema = value; - return old; -} - -function _validateUri(ret: URI, _strict?: boolean): void { +function _validateUri(ret: URI): void { // scheme, must be set if (!ret.scheme) { - if (_strict || _throwOnMissingSchema) { - throw new Error(`[UriError]: Scheme is missing: {scheme: "", authority: "${ret.authority}", path: "${ret.path}", query: "${ret.query}", fragment: "${ret.fragment}"}`); - } else { - console.warn(`[UriError]: Scheme is missing: {scheme: "", authority: "${ret.authority}", path: "${ret.path}", query: "${ret.query}", fragment: "${ret.fragment}"}`); - } + throw new Error(`[UriError]: Scheme is missing: {scheme: "", authority: "${ret.authority}", path: "${ret.path}", query: "${ret.query}", fragment: "${ret.fragment}"}`); } // scheme, https://tools.ietf.org/html/rfc3986#section-3.1 @@ -56,14 +41,8 @@ function _validateUri(ret: URI, _strict?: boolean): void { } } -// for a while we allowed uris *without* schemes and this is the migration -// for them, e.g. an uri without scheme and without strict-mode warns and falls -// back to the file-scheme. that should cause the least carnage and still be a -// clear warning -function _schemeFix(scheme: string, _strict: boolean): string { - if (_strict || _throwOnMissingSchema) { - return scheme || _empty; - } +// graceful behaviour when scheme is missing: fallback to using 'file'-scheme +function _schemeFix(scheme: string): string { if (!scheme) { console.trace('BAD uri lacks scheme, falling back to file-scheme.'); scheme = 'file'; @@ -159,7 +138,7 @@ export class URI implements UriComponents { /** * @internal */ - protected constructor(scheme: string, authority?: string, path?: string, query?: string, fragment?: string, _strict?: boolean); + protected constructor(scheme: string, authority?: string, path?: string, query?: string, fragment?: string); /** * @internal @@ -169,7 +148,7 @@ export class URI implements UriComponents { /** * @internal */ - protected constructor(schemeOrData: string | UriComponents, authority?: string, path?: string, query?: string, fragment?: string, _strict: boolean = false) { + protected constructor(schemeOrData: string | UriComponents, authority?: string, path?: string, query?: string, fragment?: string) { if (typeof schemeOrData === 'object') { this.scheme = schemeOrData.scheme || _empty; @@ -181,13 +160,13 @@ export class URI implements UriComponents { // that creates uri components. // _validateUri(this); } else { - this.scheme = _schemeFix(schemeOrData, _strict); + this.scheme = _schemeFix(schemeOrData); this.authority = authority || _empty; this.path = _referenceResolution(this.scheme, path || _empty); this.query = query || _empty; this.fragment = fragment || _empty; - _validateUri(this, _strict); + _validateUri(this); } } @@ -226,7 +205,7 @@ export class URI implements UriComponents { // ---- modify to new ------------------------- - with(change: { scheme?: string; authority?: string | null; path?: string | null; query?: string | null; fragment?: string | null }): URI { + with(change: { scheme?: string; authority?: string | null; path?: string | null; query?: string | null; fragment?: string | null; }): URI { if (!change) { return this; @@ -279,7 +258,7 @@ export class URI implements UriComponents { * * @param value A string which represents an URI (see `URI#toString`). */ - static parse(value: string, _strict: boolean = false): URI { + static parse(value: string): URI { const match = _regexp.exec(value); if (!match) { return new _URI(_empty, _empty, _empty, _empty, _empty); @@ -289,8 +268,7 @@ export class URI implements UriComponents { decodeURIComponent(match[4] || _empty), decodeURIComponent(match[5] || _empty), decodeURIComponent(match[7] || _empty), - decodeURIComponent(match[9] || _empty), - _strict + decodeURIComponent(match[9] || _empty) ); } @@ -342,7 +320,7 @@ export class URI implements UriComponents { return new _URI('file', authority, path, _empty, _empty); } - static from(components: { scheme: string; authority?: string; path?: string; query?: string; fragment?: string }): URI { + static from(components: { scheme: string; authority?: string; path?: string; query?: string; fragment?: string; }): URI { return new _URI( components.scheme, components.authority, @@ -466,7 +444,7 @@ class _URI extends URI { } // reserved characters: https://tools.ietf.org/html/rfc3986#section-2.2 -const encodeTable: { [ch: number]: string } = { +const encodeTable: { [ch: number]: string; } = { [CharCode.Colon]: '%3A', // gen-delims [CharCode.Slash]: '%2F', [CharCode.QuestionMark]: '%3F', diff --git a/src/vs/monaco.d.ts b/src/vs/monaco.d.ts index 9abd28bc461..f0e63042d1e 100644 --- a/src/vs/monaco.d.ts +++ b/src/vs/monaco.d.ts @@ -55,7 +55,6 @@ declare namespace monaco { */ readonly onCancellationRequested: IEvent; } - /** * Uniform Resource Identifier (Uri) http://tools.ietf.org/html/rfc3986. * This class is a simple parser which creates the basic component parts @@ -132,7 +131,7 @@ declare namespace monaco { * * @param value A string which represents an Uri (see `Uri#toString`). */ - static parse(value: string, _strict?: boolean): Uri; + static parse(value: string): Uri; /** * Creates a new Uri from a file system path, e.g. `c:\my\files`, * `/usr/home`, or `\\server\share\some\path`. diff --git a/src/vs/workbench/api/common/extHostTypeConverters.ts b/src/vs/workbench/api/common/extHostTypeConverters.ts index 24dd0936044..c3f84a27286 100644 --- a/src/vs/workbench/api/common/extHostTypeConverters.ts +++ b/src/vs/workbench/api/common/extHostTypeConverters.ts @@ -255,12 +255,12 @@ export namespace MarkdownString { } // extract uris into a separate object - const resUris: { [href: string]: UriComponents } = Object.create(null); + const resUris: { [href: string]: UriComponents; } = Object.create(null); res.uris = resUris; const collectUri = (href: string): string => { try { - let uri = URI.parse(href, true); + let uri = URI.parse(href); uri = uri.with({ query: _uriMassage(uri.query, resUris) }); resUris[href] = uri; } catch (e) { @@ -277,7 +277,7 @@ export namespace MarkdownString { return res; } - function _uriMassage(part: string, bucket: { [n: string]: UriComponents }): string { + function _uriMassage(part: string, bucket: { [n: string]: UriComponents; }): string { if (!part) { return part; } @@ -513,7 +513,7 @@ export namespace WorkspaceEdit { export namespace SymbolKind { - const _fromMapping: { [kind: number]: modes.SymbolKind } = Object.create(null); + const _fromMapping: { [kind: number]: modes.SymbolKind; } = Object.create(null); _fromMapping[types.SymbolKind.File] = modes.SymbolKind.File; _fromMapping[types.SymbolKind.Module] = modes.SymbolKind.Module; _fromMapping[types.SymbolKind.Namespace] = modes.SymbolKind.Namespace; @@ -903,7 +903,7 @@ export namespace DocumentLink { let target: URI | undefined = undefined; if (link.url) { try { - target = typeof link.url === 'string' ? URI.parse(link.url, true) : URI.revive(link.url); + target = typeof link.url === 'string' ? URI.parse(link.url) : URI.revive(link.url); } catch (err) { // ignore } diff --git a/src/vs/workbench/services/extensions/common/extensionHostMain.ts b/src/vs/workbench/services/extensions/common/extensionHostMain.ts index 16356a4a96c..da1dc8e9a9e 100644 --- a/src/vs/workbench/services/extensions/common/extensionHostMain.ts +++ b/src/vs/workbench/services/extensions/common/extensionHostMain.ts @@ -6,7 +6,7 @@ import { timeout } from 'vs/base/common/async'; import * as errors from 'vs/base/common/errors'; import { DisposableStore } from 'vs/base/common/lifecycle'; -import { URI, setUriThrowOnMissingScheme } from 'vs/base/common/uri'; +import { URI } from 'vs/base/common/uri'; import { IURITransformer } from 'vs/base/common/uriIpc'; import { IMessagePassingProtocol } from 'vs/base/parts/ipc/common/ipc'; import { IInitData, MainContext, MainThreadConsoleShape } from 'vs/workbench/api/common/extHost.protocol'; @@ -22,10 +22,6 @@ import { IExtHostRpcService, ExtHostRpcService } from 'vs/workbench/api/common/e import { IURITransformerService, URITransformerService } from 'vs/workbench/api/common/extHostUriTransformerService'; import { IExtHostExtensionService, IHostUtils } from 'vs/workbench/api/common/extHostExtensionService'; -// we don't (yet) throw when extensions parse -// uris that have no scheme -setUriThrowOnMissingScheme(false); - export interface IExitFn { (code?: number): any; } diff --git a/src/vs/workbench/test/electron-browser/api/extHostTypeConverter.test.ts b/src/vs/workbench/test/electron-browser/api/extHostTypeConverter.test.ts index aac33827faa..63f13266677 100644 --- a/src/vs/workbench/test/electron-browser/api/extHostTypeConverter.test.ts +++ b/src/vs/workbench/test/electron-browser/api/extHostTypeConverter.test.ts @@ -22,11 +22,13 @@ suite('ExtHostTypeConverter', function () { data = MarkdownString.from('Hello [link](foo)'); assert.equal(data.value, 'Hello [link](foo)'); - assert.equal(isEmptyObject(data.uris), true); // no scheme, no uri + assert.equal(size(data.uris!), 1); + assert.ok(!!data.uris!['foo']); data = MarkdownString.from('Hello [link](www.noscheme.bad)'); assert.equal(data.value, 'Hello [link](www.noscheme.bad)'); - assert.equal(isEmptyObject(data.uris), true); // no scheme, no uri + assert.equal(size(data.uris!), 1); + assert.ok(!!data.uris!['www.noscheme.bad']); data = MarkdownString.from('Hello [link](foo:path)'); assert.equal(data.value, 'Hello [link](foo:path)'); -- GitLab