Skip to content

Port escape fix to release branch #14202

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

Merged
rchiodo merged 3 commits into from
Oct 1, 2020
Merged

Port escape fix to release branch #14202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d683d13
A different way of fixing escaping (#14186)
rchiodo Oct 1, 2020
7a92cdb
Update changelog
rchiodo Oct 1, 2020
378a154
Merge branch 'release-2020.09' into rchiodo/port_escape_fix
rchiodo Oct 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
# Changelog

## 2020.9.2 (1 October 2020)
## 2020.9.2 (2 October 2020)

### Fixes

1. Support nbconvert version 6+ for exporting notebooks to python code.
Copy link
Author

@rchiodo rchiodo Oct 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added Ian's in here too. I'll submit after him and fix up the conflict.

Copy link
Member

@IanMatthewHuff IanMatthewHuff Oct 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you have the right date anyways.

([#14169](https://github.com/Microsoft/vscode-python/issues/14169))
1. Do not escape output in the actual ipynb file.
([#14182](https://github.com/Microsoft/vscode-python/issues/14182))

### Thanks

Expand Down Expand Up @@ -61,7 +63,6 @@ And finally thanks to the [Python](https://www.python.org/) development team and
community for creating a fantastic programming language and community to be a
part of!


## 2020.9.1 (29 September 2020)

### Fixes
Expand Down
14 changes: 9 additions & 5 deletions src/client/datascience/editor-integration/cellhashprovider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ import {

// tslint:disable-next-line:no-require-imports no-var-requires
const _escapeRegExp = require('lodash/escapeRegExp') as typeof import('lodash/escapeRegExp'); // NOSONAR
const LineNumberMatchRegex = /(;32m[ ->]*?)(\d+)/g;
// tslint:disable-next-line: no-require-imports no-var-requires
const _escape = require('lodash/escape') as typeof import('lodash/escape'); // NOSONAR
const LineNumberMatchRegex = /(;32m[ ->]*?)(\d+)(.*)/g;

interface IRangedCellHash extends ICellHash {
code: string;
Expand Down Expand Up @@ -133,7 +135,7 @@ export class CellHashProvider implements ICellHashProvider, INotebookExecutionLo
...msg,
content: {
...msg.content,
traceback: this.modifyTraceback(msg as KernelMessage.IErrorMsg) // NOSONAR
transient: this.modifyTraceback(msg as KernelMessage.IErrorMsg) // NOSONAR
}
};
}
Expand Down Expand Up @@ -423,14 +425,16 @@ export class CellHashProvider implements ICellHashProvider, INotebookExecutionLo
// Now attempt to find a cell that matches these source lines
const offset = this.findCellOffset(this.hashes.get(match[0]), sourceLines);
if (offset !== undefined) {
return traceFrame.replace(LineNumberMatchRegex, (_s, prefix, num) => {
return traceFrame.replace(LineNumberMatchRegex, (_s, prefix, num, suffix) => {
const n = parseInt(num, 10);
const newLine = offset + n - 1;
return `${prefix}<a href='file://${match[0]}?line=${newLine}'>${newLine + 1}</a>`;
return `${_escape(prefix)}<a href='file://${match[0]}?line=${newLine}'>${newLine + 1}</a>${_escape(
suffix
)}`;
});
}
}
return traceFrame;
return _escape(traceFrame);
}
}

Expand Down
6 changes: 1 addition & 5 deletions src/client/datascience/jupyter/jupyterDebugger.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
'use strict';
import type { nbformat } from '@jupyterlab/coreutils';
import { inject, injectable, named } from 'inversify';
// tslint:disable-next-line: no-require-imports
import unescape = require('lodash/unescape');
import * as path from 'path';
import * as uuid from 'uuid/v4';
import { DebugConfiguration, Disposable } from 'vscode';
Expand Down Expand Up @@ -475,10 +473,8 @@ export class JupyterDebugger implements IJupyterDebugger, ICellHashListener {
if (outputs.length > 0) {
const data = outputs[0].data;
if (data && data.hasOwnProperty('text/plain')) {
// Plain text should be escaped by our execution engine. Unescape it so
// we can parse it.
// tslint:disable-next-line:no-any
return unescape((data as any)['text/plain']);
return (data as any)['text/plain'];
}
if (outputs[0].output_type === 'stream') {
const stream = outputs[0] as nbformat.IStream;
Expand Down
72 changes: 33 additions & 39 deletions src/client/datascience/jupyter/jupyterNotebook.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,7 @@ import { KernelConnectionMetadata } from './kernels/types';

// tslint:disable-next-line: no-require-imports
import cloneDeep = require('lodash/cloneDeep');
// tslint:disable-next-line: no-require-imports
import escape = require('lodash/escape');
// tslint:disable-next-line: no-require-imports
import unescape = require('lodash/unescape');
import { concatMultilineString, formatStreamText } from '../../../datascience-ui/common';
import { concatMultilineString, formatStreamText, splitMultilineString } from '../../../datascience-ui/common';
import { RefBool } from '../../common/refBool';
import { PythonEnvironment } from '../../pythonEnvironments/info';
import { getInterpreterFromKernelConnectionMetadata, isPythonKernelConnection } from './kernels/helpers';
Expand Down Expand Up @@ -787,12 +783,12 @@ export class JupyterNotebookBase implements INotebook {
outputs.forEach((o) => {
if (o.output_type === 'stream') {
const stream = o as nbformat.IStream;
result = result.concat(formatStreamText(unescape(concatMultilineString(stream.text, true))));
result = result.concat(formatStreamText(concatMultilineString(stream.text, true)));
} else {
const data = o.data;
if (data && data.hasOwnProperty('text/plain')) {
// tslint:disable-next-line:no-any
result = result.concat(unescape((data as any)['text/plain']));
result = result.concat((data as any)['text/plain']);
}
}
});
Expand Down Expand Up @@ -1206,12 +1202,7 @@ export class JupyterNotebookBase implements INotebook {

private addToCellData = (
cell: ICell,
output:
| nbformat.IUnrecognizedOutput
| nbformat.IExecuteResult
| nbformat.IDisplayData
| nbformat.IStream
| nbformat.IError,
output: nbformat.IExecuteResult | nbformat.IDisplayData | nbformat.IStream | nbformat.IError,
clearState: RefBool
) => {
const data: nbformat.ICodeCell = cell.data as nbformat.ICodeCell;
Expand All @@ -1237,7 +1228,10 @@ export class JupyterNotebookBase implements INotebook {
) {
// Check our length on text output
if (msg.content.data && msg.content.data.hasOwnProperty('text/plain')) {
msg.content.data['text/plain'] = escape(trimFunc(msg.content.data['text/plain'] as string));
msg.content.data['text/plain'] = splitMultilineString(
// tslint:disable-next-line: no-any
trimFunc(concatMultilineString(msg.content.data['text/plain'] as any))
);
}

this.addToCellData(
Expand Down Expand Up @@ -1265,14 +1259,15 @@ export class JupyterNotebookBase implements INotebook {
reply.payload.forEach((o) => {
if (o.data && o.data.hasOwnProperty('text/plain')) {
// tslint:disable-next-line: no-any
const str = (o.data as any)['text/plain'].toString();
const data = escape(trimFunc(str)) as string;
const str = concatMultilineString((o.data as any)['text/plain']); // NOSONAR
const data = trimFunc(str);
this.addToCellData(
cell,
{
// Mark as stream output so the text is formatted because it likely has ansi codes in it.
output_type: 'stream',
text: data,
text: splitMultilineString(data),
name: 'stdout',
metadata: {},
execution_count: reply.execution_count
},
Expand Down Expand Up @@ -1313,23 +1308,25 @@ export class JupyterNotebookBase implements INotebook {
? data.outputs[data.outputs.length - 1]
: undefined;
if (existing) {
// tslint:disable-next-line:restrict-plus-operands
existing.text = existing.text + escape(msg.content.text);
const originalText = formatStreamText(concatMultilineString(existing.text));
const originalText = formatStreamText(
// tslint:disable-next-line: no-any
`${concatMultilineString(existing.text as any)}${concatMultilineString(msg.content.text)}`
);
originalTextLength = originalText.length;
existing.text = trimFunc(originalText);
trimmedTextLength = existing.text.length;
const newText = trimFunc(originalText);
trimmedTextLength = newText.length;
existing.text = splitMultilineString(newText);
} else {
const originalText = formatStreamText(concatMultilineString(escape(msg.content.text)));
const originalText = formatStreamText(concatMultilineString(msg.content.text));
originalTextLength = originalText.length;
// Create a new stream entry
const output: nbformat.IStream = {
output_type: 'stream',
name: msg.content.name,
text: trimFunc(originalText)
text: [trimFunc(originalText)]
};
data.outputs = [...data.outputs, output];
trimmedTextLength = output.text.length;
trimmedTextLength = output.text[0].length;
cell.data = data;
}

Expand All @@ -1338,23 +1335,16 @@ export class JupyterNotebookBase implements INotebook {
// the output is trimmed and what setting changes that.
// * If data.metadata.tags is undefined, define it so the following
// code is can rely on it being defined.
if (data.metadata.tags === undefined) {
data.metadata.tags = [];
}

data.metadata.tags = data.metadata.tags.filter((t) => t !== 'outputPrepend');

if (trimmedTextLength < originalTextLength) {
if (data.metadata.tags === undefined) {
data.metadata.tags = [];
}
data.metadata.tags = data.metadata.tags.filter((t) => t !== 'outputPrepend');
data.metadata.tags.push('outputPrepend');
}
}

private handleDisplayData(msg: KernelMessage.IDisplayDataMsg, clearState: RefBool, cell: ICell) {
// Escape text output
if (msg.content.data && msg.content.data.hasOwnProperty('text/plain')) {
msg.content.data['text/plain'] = escape(msg.content.data['text/plain'] as string);
}

const output: nbformat.IDisplayData = {
output_type: 'display_data',
data: msg.content.data,
Expand Down Expand Up @@ -1402,10 +1392,14 @@ export class JupyterNotebookBase implements INotebook {
private handleError(msg: KernelMessage.IErrorMsg, clearState: RefBool, cell: ICell) {
const output: nbformat.IError = {
output_type: 'error',
ename: escape(msg.content.ename),
evalue: escape(msg.content.evalue),
traceback: msg.content.traceback.map(escape)
ename: msg.content.ename,
evalue: msg.content.evalue,
traceback: msg.content.traceback
};
if (msg.content.hasOwnProperty('transient')) {
// tslint:disable-next-line: no-any
output.transient = (msg.content as any).transient;
}
this.addToCellData(cell, output, clearState);
cell.state = CellState.error;

Expand Down
6 changes: 2 additions & 4 deletions src/client/datascience/jupyter/kernelVariables.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import { inject, injectable } from 'inversify';
import stripAnsi from 'strip-ansi';
import * as uuid from 'uuid/v4';

// tslint:disable-next-line: no-require-imports
import unescape = require('lodash/unescape');
import { CancellationToken, Event, EventEmitter, Uri } from 'vscode';
import { PYTHON_LANGUAGE } from '../../common/constants';
import { traceError } from '../../common/logger';
Expand Down Expand Up @@ -248,7 +246,7 @@ export class KernelVariables implements IJupyterVariables {

// Pull our text result out of the Jupyter cell
private deserializeJupyterResult<T>(cells: ICell[]): T {
const text = unescape(this.extractJupyterResultText(cells));
const text = this.extractJupyterResultText(cells);
return JSON.parse(text) as T;
}

Expand Down Expand Up @@ -373,7 +371,7 @@ export class KernelVariables implements IJupyterVariables {
// Now execute the query
if (notebook && query) {
const cells = await notebook.execute(query.query, Identifiers.EmptyFileName, 0, uuid(), token, true);
const text = unescape(this.extractJupyterResultText(cells));
const text = this.extractJupyterResultText(cells);

// Apply the expression to it
const matches = this.getAllMatches(query.parser, text);
Expand Down
6 changes: 2 additions & 4 deletions src/client/datascience/jupyter/oldJupyterVariables.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ import { Event, EventEmitter, Uri } from 'vscode';
import { PYTHON_LANGUAGE } from '../../common/constants';
import { traceError } from '../../common/logger';

// tslint:disable-next-line: no-require-imports
import unescape = require('lodash/unescape');
import { IConfigurationService } from '../../common/types';
import * as localize from '../../common/utils/localize';
import { EXTENSION_ROOT_DIR } from '../../constants';
Expand Down Expand Up @@ -234,7 +232,7 @@ export class OldJupyterVariables implements IJupyterVariables {

// Pull our text result out of the Jupyter cell
private deserializeJupyterResult<T>(cells: ICell[]): T {
const text = unescape(this.extractJupyterResultText(cells));
const text = this.extractJupyterResultText(cells);
return JSON.parse(text) as T;
}

Expand Down Expand Up @@ -359,7 +357,7 @@ export class OldJupyterVariables implements IJupyterVariables {
// Now execute the query
if (notebook && query) {
const cells = await notebook.execute(query.query, Identifiers.EmptyFileName, 0, uuid(), undefined, true);
const text = unescape(this.extractJupyterResultText(cells));
const text = this.extractJupyterResultText(cells);

// Apply the expression to it
const matches = this.getAllMatches(query.parser, text);
Expand Down
20 changes: 16 additions & 4 deletions src/datascience-ui/interactive-common/cellOutput.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import { getRichestMimetype, getTransform, isIPyWidgetOutput, isMimeTypeSupporte

// tslint:disable-next-line: no-var-requires no-require-imports
const ansiToHtml = require('ansi-to-html');
// tslint:disable-next-line: no-var-requires no-require-imports
const lodashEscape = require('lodash/escape');

// tslint:disable-next-line: no-require-imports no-var-requires
const cloneDeep = require('lodash/cloneDeep');
Expand Down Expand Up @@ -328,7 +330,7 @@ export class CellOutput extends React.Component<ICellOutputProps> {
// tslint:disable-next-line: no-any
const text = (input as any)['text/plain'];
input = {
'text/html': text // XML tags should have already been escaped.
'text/html': lodashEscape(concatMultilineString(text))
};
} else if (output.output_type === 'stream') {
mimeType = 'text/html';
Expand All @@ -337,7 +339,7 @@ export class CellOutput extends React.Component<ICellOutputProps> {
renderWithScrollbars = true;
// Sonar is wrong, TS won't compile without this AS
const stream = output as nbformat.IStream; // NOSONAR
const concatted = concatMultilineString(stream.text);
const concatted = lodashEscape(concatMultilineString(stream.text));
input = {
'text/html': concatted // XML tags should have already been escaped.
};
Expand All @@ -363,14 +365,18 @@ export class CellOutput extends React.Component<ICellOutputProps> {
const error = output as nbformat.IError; // NOSONAR
try {
const converter = new CellOutput.ansiToHtmlClass(CellOutput.getAnsiToHtmlOptions());
const trace = error.traceback.length ? converter.toHtml(error.traceback.join('\n')) : error.evalue;
// Modified traceback may exist. If so use that instead. It's only at run time though
const traceback: string[] = error.transient
? (error.transient as string[])
: error.traceback.map(lodashEscape);
const trace = traceback ? converter.toHtml(traceback.join('\n')) : error.evalue;
input = {
'text/html': trace
};
} catch {
// This can fail during unit tests, just use the raw data
input = {
'text/html': error.evalue
'text/html': lodashEscape(error.evalue)
};
}
} else if (input) {
Expand All @@ -395,6 +401,12 @@ export class CellOutput extends React.Component<ICellOutputProps> {
data = fixMarkdown(concatMultilineString(data as nbformat.MultilineString, true), true);
}

// Make sure text output is escaped (nteract texttransform won't)
if (mimeType === 'text/plain' && data) {
data = lodashEscape(data.toString());
mimeType = 'text/html';
}

return {
isText,
isError,
Expand Down
2 changes: 1 addition & 1 deletion src/datascience-ui/react-common/postOffice.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export class PostOffice implements IDisposable {
// See ./src/datascience-ui/native-editor/index.html
// tslint:disable-next-line: no-any
const api = (this.vscodeApi as any) as { handleMessage?: Function };
if (api.handleMessage) {
if (api && api.handleMessage) {
api.handleMessage(this.handleMessages.bind(this));
}
} catch {
Expand Down
8 changes: 8 additions & 0 deletions src/test/datascience/interactiveWindow.functional.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ import {
verifyLastCellInputState
} from './testHelpers';
import { ITestInteractiveWindowProvider } from './testInteractiveWindowProvider';
// tslint:disable-next-line: no-require-imports no-var-requires
const _escape = require('lodash/escape') as typeof import('lodash/escape'); // NOSONAR

// tslint:disable:max-func-body-length trailing-comma no-any no-multiline-string
suite('DataScience Interactive Window output tests', () => {
Expand Down Expand Up @@ -385,6 +387,9 @@ for _ in range(50):
time.sleep(0.1)
sys.stdout.write('\\r')`;

const exception = 'raise Exception("<html check>")';
addMockData(ioc, exception, `"<html check>"`, 'text/html', 'error');

addMockData(ioc, badPanda, `pandas has no attribute 'read'`, 'text/html', 'error');
addMockData(ioc, goodPanda, `<td>A table</td>`, 'text/html');
addMockData(ioc, matPlotLib, matPlotLibResults, 'text/html');
Expand All @@ -401,6 +406,9 @@ for _ in range(50):
return Promise.resolve({ result: result, haveMore: loops > 0 });
});

await addCode(ioc, exception, true);
verifyHtmlOnInteractiveCell(_escape(`<html check>`), CellPosition.Last);

await addCode(ioc, badPanda, true);
verifyHtmlOnInteractiveCell(`has no attribute 'read'`, CellPosition.Last);

Expand Down
Loading