ckaczor/azuredatastudio
1
0
Fork 0
mirror of https://github.com/ckaczor/azuredatastudio.git synced 2026-01-30 01:25:38 -05:00
Code Issues Packages Projects Releases Wiki Activity

Fixes Encoding / Decoding issues with Notebook Linking (#17304)

Browse Source 
* fixes encoding / decoding issues with %20 files

* fix windows test

* address PR comments and absolute path setting
This commit is contained in:
Vasu Bhog
2021-10-08 15:22:10 -07:00
committed by GitHub
parent 75d6847a65
commit c35cd3e48f
6 changed files with 50 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
src/sql/workbench/contrib/notebook/browser/cellViews/markdownToolbar.component.ts
View File

@@ -244,7 +244,9 @@ export class MarkdownToolbarComponent extends AngularDisposable {
let linkUrl = notebookLink.getLinkUrl();
// Otherwise, re-focus on the output element, and insert the link directly.
this.output?.nativeElement?.focus();
document.execCommand('insertHTML', false, `<a href="${escape(linkUrl)}" title="${escape(linkUrl)}" is-absolute=${notebookLink.isAbsolutePath}>${escape(linkCalloutResult?.insertUnescapedLinkLabel)}</a>`);
// Need to encode URI here in order for user to click the proper encoded link in WYSIWYG
let encodedLinkURL = encodeURI(linkUrl);
document.execCommand('insertHTML', false, `<a href="${encodedLinkURL}" title="${encodedLinkURL}" is-encoded="true" is-absolute=${notebookLink.isAbsolutePath}>${escape(linkCalloutResult?.insertUnescapedLinkLabel)}</a>`);
return;
}
} else if (type === MarkdownButtonType.IMAGE_PREVIEW) {

48
src/sql/workbench/contrib/notebook/browser/notebookLinkHandler.ts
View File

@@ -9,7 +9,7 @@ import { IConfigurationService } from 'vs/platform/configuration/common/configur
import { replaceInvalidLinkPath } from 'sql/workbench/contrib/notebook/common/utils';
import { isWindows } from 'vs/base/common/platform';
const keepAbsolutePathConfigName = 'notebook.keepAbsolutePath';
const useAbsolutePathConfigName = 'notebook.useAbsoluteFilePaths';
export class NotebookLinkHandler {
private _notebookUriLink: URI;
@@ -18,12 +18,26 @@ export class NotebookLinkHandler {
private _isAnchorLink: boolean;
private _isFile: boolean;
public readonly isAbsolutePath: boolean;
public readonly isMarkdown: boolean;
public readonly isEncoded: boolean;
constructor(
private _notebookURI: URI,
private _link: string | HTMLAnchorElement,
@IConfigurationService private _configurationService: IConfigurationService,
) {
/**
* If link is string
* - string link is passed in via onInsertButtonClick (markdownToolbar.component.ts)
* - string in form of 'https://','http://', or 'file://'
* If link is HTMLAnchorElement
* - the node element is passed in via getCurrentLinkUrl() in markdownToolbar.component.ts
* - the node element is passed in via anchor rule in htmlMarkdownConverter.ts
* The link / href we receive is not escaped initially so we need to encode the special characters
* such as space and %20 to return the proper path.
* The link that we return should be the encoded, as that will allow the linkHandler to then decode the
* link via uri.parse to open the correct file or web link.
*/
if (typeof this._link === 'string') {
this._notebookUriLink = URI.parse(this._link);
this._isFile = this._notebookUriLink.scheme === 'file';
@@ -33,15 +47,18 @@ export class NotebookLinkHandler {
// HTMLAnchorElement
// windows files need to use the link.href instead as it contains the file:// prefix
// which enables us to get the proper relative path
this.isAbsolutePath = this._link.attributes['is-absolute']?.nodeValue === 'true' ? true : false;
this.isMarkdown = this._link.attributes['is-markdown']?.nodeValue === 'true' ? true : false;
this.isEncoded = this._link.attributes['is-encoded']?.nodeValue === 'true' ? true : false;
this._isFile = this._link.protocol === 'file:';
// Given an anchor element for windows href link will need to use nodeValue instead as that does not encode the url
if (isWindows) {
this._href = this._link.href;
this._href = this.isMarkdown || this.isEncoded ? this._link.href?.replace(/%5C/g, '\\') : this._link.attributes['href']?.nodeValue;
} else {
this._href = this._link.attributes['href']?.nodeValue;
}
this._notebookUriLink = this._href ? URI.parse(this._href) : undefined;
this._isFile = this._link.protocol === 'file:';
this._notebookUriLink = this._href ? URI.parse(encodeURI(this._href)) : undefined;
this._isAnchorLink = this._notebookUriLink?.fragment ? true : false;
this.isAbsolutePath = this._link.attributes['is-absolute']?.nodeValue === 'true' ? true : false;
}
this._notebookDirectory = this._notebookURI ? path.dirname(this._notebookURI.fsPath) : '';
}
@@ -58,7 +75,7 @@ export class NotebookLinkHandler {
// cases where we only have the href link
if (typeof this._link === 'string') {
// Does not convert absolute path to relative path
if (this._isFile && this.isAbsolutePath && this._configurationService.getValue(keepAbsolutePathConfigName) === true) {
if (this._isFile && this.isAbsolutePath && this._configurationService.getValue(useAbsolutePathConfigName) === true) {
return this._link;
}
// sets the string to absolute path to be used to resolve
@@ -77,8 +94,8 @@ export class NotebookLinkHandler {
if (this._notebookUriLink && this._isFile) {
let targetUri: URI;
// Does not convert absolute path to relative path if keep Absolute Path setting is enabled
if (this.isAbsolutePath && this._configurationService.getValue(keepAbsolutePathConfigName) === true) {
return escape(this._href);
if (this.isAbsolutePath && this._configurationService.getValue(useAbsolutePathConfigName) === true) {
return this._href;
} else {
if (this._isAnchorLink) {
targetUri = this.getUriAnchorLink(this._link, this._notebookURI);
@@ -89,7 +106,7 @@ export class NotebookLinkHandler {
}
// returns relative path of target notebook to the current notebook directory
if (this._notebookUriLink.fsPath !== this._notebookURI.fsPath && !targetUri?.fragment) {
return findPathRelativeToContent(this._notebookDirectory, targetUri);
return findPathRelativeToContent(this._notebookDirectory, targetUri, this.isMarkdown, this.isEncoded);
} else {
// if the anchor link is to a section in the same notebook then just add the fragment
return targetUri.fragment;
@@ -125,15 +142,20 @@ export class NotebookLinkHandler {
* Finds the Relative Path from current notebook folder to target (linked) notebook
* @param notebookFolder is the current notebook directory
* @param contentPath is the URI path to the notebook we are linking to
*/
/**
* Finds the Relative Path from current notebook folder to target (linked) notebook
* @param notebookFolder is the current notebook directory
* @param contentPath is the URI path to the notebook we are linking to
* @param isMarkdown is checked to see if the link is already in markdown format
* @param isEncoded is checked to know if the link is already encoded
* @returns relative path from the current notebook to the target notebook
*/
export function findPathRelativeToContent(notebookFolder: string, contentPath: URI | undefined): string {
export function findPathRelativeToContent(notebookFolder: string, contentPath: URI | undefined, isMarkdown?: boolean, isEncoded?: boolean): string {
if (contentPath?.scheme === 'file') {
let relativePath = contentPath.fragment ? path.relative(notebookFolder, contentPath.fsPath).concat('#', contentPath.fragment) : path.relative(notebookFolder, contentPath.fsPath);
//if path contains whitespaces then it's not identified as a link
relativePath = relativePath.replace(/\s/g, '%20');
// if relativePath contains improper directory format due to marked js parsing returning an invalid path (ex. ....\) then we need to replace it to ensure the directories are formatted properly (ex. ..\..\)
relativePath = replaceInvalidLinkPath(relativePath);
relativePath = isMarkdown || isEncoded ? replaceInvalidLinkPath(relativePath) : encodeURI(replaceInvalidLinkPath(relativePath)).replace(/%5C/g, '\\');
if (relativePath.startsWith(path.join('..', path.sep)) || relativePath.startsWith(path.join('.', path.sep))) {
return relativePath;
} else {

3
src/sql/workbench/contrib/notebook/browser/outputs/notebookMarkdown.ts
View File

@@ -149,7 +149,8 @@ export class NotebookMarkdownRenderer {
.replace(/>/g, '&gt;')
.replace(/"/g, '&quot;')
.replace(/'/g, '&#39;');
return `<a href=${href} data-href="${href}" title="${title || href}" is-absolute=${hrefAbsolute}>${text}</a>`;
let isMarkdown = markdown.value ? true : false;
return `<a href=${href} data-href="${href}" title="${title || href}" is-markdown=${isMarkdown} is-absolute=${hrefAbsolute}>${text}</a>`;
}
};
renderer.paragraph = (text): string => {

4
src/sql/workbench/contrib/notebook/test/browser/htmlMarkdownConverter.test.ts
View File

@@ -129,12 +129,12 @@ suite('HTML Markdown Converter', function (): void {
});
test('Should transform <a> tag', () => {
configurationService.updateValue('notebook.keepAbsolutePath', false, ConfigurationTarget.USER);
configurationService.updateValue('notebook.useAbsoluteFilePaths', false, ConfigurationTarget.USER);
htmlString = '<a href="/tmp/stuff.png">stuff</a>';
assert.strictEqual(htmlMarkdownConverter.convert(htmlString), `[stuff](.${path.sep}stuff.png)`, 'Basic link test failed');
htmlString = '<a href="/tmp/stuff.png"</a>';
assert.strictEqual(htmlMarkdownConverter.convert(htmlString), `[](.${path.sep}stuff.png)`, 'Basic link test no label failed');
htmlString = '<a href="/tmp/my stuff.png"</a>';
htmlString = '<a href="/tmp/my stuff.png"></a>';
assert.strictEqual(htmlMarkdownConverter.convert(htmlString), `[](.${path.sep}my%20stuff.png)`, 'Basic link test no label space filename failed');
htmlString = '<a href="/stuff.png">stuff</a>';
assert.strictEqual(htmlMarkdownConverter.convert(htmlString), `[stuff](..${path.sep}stuff.png)`, 'Basic link test above folder failed');

5
src/sql/workbench/contrib/notebook/test/browser/notebookLinkHandler.test.ts
View File

@@ -75,8 +75,7 @@ suite('Noteboook Link Handler', function (): void {
});
test('Should return absolute links with keep absolute path setting', () => {
// let basePath = process.cwd();
configurationService.updateValue('notebook.keepAbsolutePath', true, ConfigurationTarget.USER);
configurationService.updateValue('notebook.useAbsoluteFilePaths', true, ConfigurationTarget.USER);
let result = new NotebookLinkHandler(notebookUri, 'https://www.microsoft.com/images/msft.png', configurationService);
assert.strictEqual(result.getLinkUrl(), `https://www.microsoft.com/images/msft.png`, 'HTTPS link failed to resolve');
@@ -123,6 +122,6 @@ suite('Noteboook Link Handler', function (): void {
assert.strictEqual(result.getLinkUrl(), `.${path.sep}my%20stuff.png`, 'Basic link test with space filename failed');
result = new NotebookLinkHandler(notebookUri, Object.assign(document.createElement('a'), { href: '/tmp/my%20stuff.png' }), configurationService);
assert.strictEqual(result.getLinkUrl(), `.${path.sep}my%20stuff.png`, 'Basic link test with space filename failed');
assert.strictEqual(result.getLinkUrl(), `.${path.sep}my%2520stuff.png`, 'Basic link test with %20 filename failed');
});
});

12
src/sql/workbench/contrib/notebook/test/browser/notebookMarkdown.test.ts
View File

@@ -50,16 +50,16 @@ suite('NotebookMarkdownRenderer', () => {
test('link from local file path', () => {
let result: HTMLElement = notebookMarkdownRenderer.renderMarkdown({ value: `[Link to File Path](someFileurl)`, isTrusted: true });
assert.strictEqual(result.innerHTML, `<p><a href="someFileurl" data-href="someFileurl" title="someFileurl" is-absolute="false">Link to File Path</a></p>`);
assert.strictEqual(result.innerHTML, `<p><a href="someFileurl" data-href="someFileurl" title="someFileurl" is-markdown="true" is-absolute="false">Link to File Path</a></p>`);
});
test('link from relative file path', () => {
notebookMarkdownRenderer.setNotebookURI(URI.parse(`foo/temp/file1.txt`));
let result: HTMLElement = notebookMarkdownRenderer.renderMarkdown({ value: `[Link to relative path](../test/build/someimageurl)`, isTrusted: true });
if (process.platform === 'win32') {
assert.strictEqual(result.innerHTML, `<p><a href="\\foo\\test\\build\\someimageurl" data-href="\\foo\\test\\build\\someimageurl" title="\\foo\\test\\build\\someimageurl" is-absolute="false">Link to relative path</a></p>`);
assert.strictEqual(result.innerHTML, `<p><a href="\\foo\\test\\build\\someimageurl" data-href="\\foo\\test\\build\\someimageurl" title="\\foo\\test\\build\\someimageurl" is-markdown="true" is-absolute="false">Link to relative path</a></p>`);
} else {
assert.strictEqual(result.innerHTML, `<p><a href="/foo/test/build/someimageurl" data-href="/foo/test/build/someimageurl" title="/foo/test/build/someimageurl" is-absolute="false">Link to relative path</a></p>`);
assert.strictEqual(result.innerHTML, `<p><a href="/foo/test/build/someimageurl" data-href="/foo/test/build/someimageurl" title="/foo/test/build/someimageurl" is-markdown="true" is-absolute="false">Link to relative path</a></p>`);
}
});
@@ -72,17 +72,17 @@ suite('NotebookMarkdownRenderer', () => {
test('email in markdown format renders properly', () => {
let result: HTMLElement = notebookMarkdownRenderer.renderMarkdown({ value: `[test@email.com](mailto:test@email.com)`, isTrusted: true });
assert.strictEqual(result.innerHTML, `<p><a href="mailto:test@email.com" data-href="mailto:test@email.com" title="mailto:test@email.com" is-absolute="false">test@email.com</a></p>`);
assert.strictEqual(result.innerHTML, `<p><a href="mailto:test@email.com" data-href="mailto:test@email.com" title="mailto:test@email.com" is-markdown="true" is-absolute="false">test@email.com</a></p>`);
});
test('email inserted directly renders properly', () => {
let result: HTMLElement = notebookMarkdownRenderer.renderMarkdown({ value: `test@email.com`, isTrusted: true });
assert.strictEqual(result.innerHTML, `<p><a href="mailto:test@email.com" data-href="mailto:test@email.com" title="mailto:test@email.com" is-absolute="false">test@email.com</a></p>`);
assert.strictEqual(result.innerHTML, `<p><a href="mailto:test@email.com" data-href="mailto:test@email.com" title="mailto:test@email.com" is-markdown="true" is-absolute="false">test@email.com</a></p>`);
});
test('link to https with query parameters', () => {
let result: HTMLElement = notebookMarkdownRenderer.renderMarkdown({ value: `[test](https://www.test.com?test=&test2=)`, isTrusted: true });
assert.strictEqual(result.innerHTML, `<p><a href="https://www.test.com?test=&amp;test2=" data-href="https://www.test.com?test=&amp;test2=" title="https://www.test.com?test=&amp;test2=" is-absolute="false">test</a></p>`);
assert.strictEqual(result.innerHTML, `<p><a href="https://www.test.com?test=&amp;test2=" data-href="https://www.test.com?test=&amp;test2=" title="https://www.test.com?test=&amp;test2=" is-markdown="true" is-absolute="false">test</a></p>`);
});
test('cell attachment image', () => {