From edf62453dce23780817eb28a98828cc4bdbd431d Mon Sep 17 00:00:00 2001 From: Alex Dima Date: Tue, 27 Sep 2016 17:56:22 +0300 Subject: [PATCH] Fixes #12329: Prevent that edits produce invalid UTF-16 --- src/vs/base/common/strings.ts | 16 ++- .../common/controller/cursorMoveHelper.ts | 7 +- src/vs/editor/common/model/textModel.ts | 91 +++++++++++--- .../common/model/editableTextModel.test.ts | 73 ++++++++++- .../model/editableTextModelTestUtils.ts | 8 +- .../test/common/model/textModel.test.ts | 119 +++++++++++++++++- 6 files changed, 276 insertions(+), 38 deletions(-) diff --git a/src/vs/base/common/strings.ts b/src/vs/base/common/strings.ts index d84699a8e1c..9e35fefb235 100644 --- a/src/vs/base/common/strings.ts +++ b/src/vs/base/common/strings.ts @@ -396,15 +396,13 @@ export function commonSuffixLength(a: string, b: string): number { // } // return chrCode; //} -//export function isLeadSurrogate(chr:string) { -// let chrCode = chr.charCodeAt(0); -// return ; -//} -// -//export function isTrailSurrogate(chr:string) { -// let chrCode = chr.charCodeAt(0); -// return 0xDC00 <= chrCode && chrCode <= 0xDFFF; -//} +export function isHighSurrogate(charCode:number): boolean { + return (0xD800 <= charCode && charCode <= 0xDBFF); +} + +export function isLowSurrogate(charCode:number): boolean { + return (0xDC00 <= charCode && charCode <= 0xDFFF); +} export function isFullWidthCharacter(charCode:number): boolean { // Do a cheap trick to better support wrapping of wide characters, treat them as 2 columns diff --git a/src/vs/editor/common/controller/cursorMoveHelper.ts b/src/vs/editor/common/controller/cursorMoveHelper.ts index a63c59dac36..26fe369ed21 100644 --- a/src/vs/editor/common/controller/cursorMoveHelper.ts +++ b/src/vs/editor/common/controller/cursorMoveHelper.ts @@ -6,6 +6,7 @@ import {IPosition} from 'vs/editor/common/editorCommon'; import {Selection} from 'vs/editor/common/core/selection'; +import * as strings from 'vs/base/common/strings'; export interface IMoveResult { lineNumber:number; @@ -51,13 +52,11 @@ export interface IConfiguration { } function isHighSurrogate(model:ICursorMoveHelperModel, lineNumber:number, column:number) { - let code = model.getLineContent(lineNumber).charCodeAt(column - 1); - return 0xD800 <= code && code <= 0xDBFF; + return strings.isHighSurrogate(model.getLineContent(lineNumber).charCodeAt(column - 1)); } function isLowSurrogate(model:ICursorMoveHelperModel, lineNumber:number, column:number) { - let code = model.getLineContent(lineNumber).charCodeAt(column - 1); - return 0xDC00 <= code && code <= 0xDFFF; + return strings.isLowSurrogate(model.getLineContent(lineNumber).charCodeAt(column - 1)); } export class CursorMoveHelper { diff --git a/src/vs/editor/common/model/textModel.ts b/src/vs/editor/common/model/textModel.ts index 13e766b5680..a3743ef3dbb 100644 --- a/src/vs/editor/common/model/textModel.ts +++ b/src/vs/editor/common/model/textModel.ts @@ -196,7 +196,7 @@ export class TextModel extends OrderGuaranteeEventEmitter implements editorCommo } public getOffsetAt(rawPosition: editorCommon.IPosition): number { - let position = this.validatePosition(rawPosition); + let position = this._validatePosition(rawPosition.lineNumber, rawPosition.column, false); this._ensureLineStarts(); return this._lineStarts.getAccumulatedValue(position.lineNumber - 2) + position.column - 1; } @@ -611,35 +611,86 @@ export class TextModel extends OrderGuaranteeEventEmitter implements editorCommo return lineNumber; } - public validatePosition(position:editorCommon.IPosition): Position { - var lineNumber = position.lineNumber ? position.lineNumber : 1; - var column = position.column ? position.column : 1; + /** + * @param strict Do NOT allow a position inside a high-low surrogate pair + */ + private _validatePosition(_lineNumber:number, _column:number, strict:boolean): Position { + const lineNumber = Math.floor(typeof _lineNumber === 'number' ? _lineNumber : 1); + const column = Math.floor(typeof _column === 'number' ? _column : 1); if (lineNumber < 1) { - lineNumber = 1; - column = 1; + return new Position(1, 1); } - else if (lineNumber > this._lines.length) { - lineNumber = this._lines.length; - column = this.getLineMaxColumn(lineNumber); + + if (lineNumber > this._lines.length) { + return new Position(this._lines.length, this.getLineMaxColumn(this._lines.length)); } - else { - var maxColumn = this.getLineMaxColumn(lineNumber); - if (column < 1) { - column = 1; - } - else if (column > maxColumn) { - column = maxColumn; + + if (column <= 1) { + return new Position(lineNumber, 1); + } + + const maxColumn = this.getLineMaxColumn(lineNumber); + if (column >= maxColumn) { + return new Position(lineNumber, maxColumn); + } + + if (strict) { + // If the position would end up in the middle of a high-low surrogate pair, + // we move it to before the pair + // !!At this point, column > 1 + const charCodeBefore = this._lines[lineNumber - 1].text.charCodeAt(column - 2); + if (strings.isHighSurrogate(charCodeBefore)) { + return new Position(lineNumber, column - 1); } } return new Position(lineNumber, column); } - public validateRange(range:editorCommon.IRange): Range { - var start = this.validatePosition(new Position(range.startLineNumber, range.startColumn)); - var end = this.validatePosition(new Position(range.endLineNumber, range.endColumn)); - return new Range(start.lineNumber, start.column, end.lineNumber, end.column); + public validatePosition(position:editorCommon.IPosition): Position { + return this._validatePosition(position.lineNumber, position.column, true); + } + + public validateRange(_range:editorCommon.IRange): Range { + const start = this._validatePosition(_range.startLineNumber, _range.startColumn, false); + const end = this._validatePosition(_range.endLineNumber, _range.endColumn, false); + + const startLineNumber = start.lineNumber; + const startColumn = start.column; + const endLineNumber = end.lineNumber; + const endColumn = end.column; + + const startLineText = this._lines[startLineNumber - 1].text; + const endLineText = this._lines[endLineNumber - 1].text; + + const charCodeBeforeStart = (startColumn > 1 ? startLineText.charCodeAt(startColumn - 2) : 0); + const charCodeBeforeEnd = (endColumn > 1 && endColumn <= endLineText.length ? endLineText.charCodeAt(endColumn - 2) : 0); + + const startInsideSurrogatePair = strings.isHighSurrogate(charCodeBeforeStart); + const endInsideSurrogatePair = strings.isHighSurrogate(charCodeBeforeEnd); + + if (!startInsideSurrogatePair && !endInsideSurrogatePair) { + return new Range(startLineNumber, startColumn, endLineNumber, endColumn); + } + + if (startLineNumber === endLineNumber && startColumn === endColumn) { + // do not expand a collapsed range, simply move it to a valid location + return new Range(startLineNumber, startColumn - 1, endLineNumber, endColumn - 1); + } + + if (startInsideSurrogatePair && endInsideSurrogatePair) { + // expand range at both ends + return new Range(startLineNumber, startColumn - 1, endLineNumber, endColumn + 1); + } + + if (startInsideSurrogatePair) { + // only expand range at the start + return new Range(startLineNumber, startColumn - 1, endLineNumber, endColumn); + } + + // only expand range at the end + return new Range(startLineNumber, startColumn, endLineNumber, endColumn + 1); } public modifyPosition(rawPosition: editorCommon.IPosition, offset: number) : Position { diff --git a/src/vs/editor/test/common/model/editableTextModel.test.ts b/src/vs/editor/test/common/model/editableTextModel.test.ts index a40af4dd82f..a04e32d8e26 100644 --- a/src/vs/editor/test/common/model/editableTextModel.test.ts +++ b/src/vs/editor/test/common/model/editableTextModel.test.ts @@ -527,7 +527,78 @@ suite('EditorModel - EditableTextModel.applyEdits', () => { }; } - + test('high-low surrogates 1', () => { + testApplyEditsWithSyncedModels( + [ + '📚some', + 'very nice', + 'text' + ], + [ + editOp(1, 2, 1, 2, ['a']) + ], + [ + 'a📚some', + 'very nice', + 'text' + ], + /*inputEditsAreInvalid*/true + ); + }); + test('high-low surrogates 2', () => { + testApplyEditsWithSyncedModels( + [ + '📚some', + 'very nice', + 'text' + ], + [ + editOp(1, 2, 1, 3, ['a']) + ], + [ + 'asome', + 'very nice', + 'text' + ], + /*inputEditsAreInvalid*/true + ); + }); + test('high-low surrogates 3', () => { + testApplyEditsWithSyncedModels( + [ + '📚some', + 'very nice', + 'text' + ], + [ + editOp(1, 1, 1, 2, ['a']) + ], + [ + 'asome', + 'very nice', + 'text' + ], + /*inputEditsAreInvalid*/true + ); + }); + test('high-low surrogates 4', () => { + testApplyEditsWithSyncedModels( + [ + '📚some', + 'very nice', + 'text' + ], + [ + editOp(1, 1, 1, 3, ['a']) + ], + [ + 'asome', + 'very nice', + 'text' + ], + /*inputEditsAreInvalid*/true + ); + }); test('Bug 19872: Undo is funky', () => { testApplyEditsWithSyncedModels( diff --git a/src/vs/editor/test/common/model/editableTextModelTestUtils.ts b/src/vs/editor/test/common/model/editableTextModelTestUtils.ts index de9640a4145..8aeef0f5ba3 100644 --- a/src/vs/editor/test/common/model/editableTextModelTestUtils.ts +++ b/src/vs/editor/test/common/model/editableTextModelTestUtils.ts @@ -12,7 +12,7 @@ import {MirrorModel2} from 'vs/editor/common/model/mirrorModel2'; import {TextModel} from 'vs/editor/common/model/textModel'; import {Position} from 'vs/editor/common/core/position'; -export function testApplyEditsWithSyncedModels(original:string[], edits:editorCommon.IIdentifiedSingleEditOperation[], expected:string[]): void { +export function testApplyEditsWithSyncedModels(original:string[], edits:editorCommon.IIdentifiedSingleEditOperation[], expected:string[], inputEditsAreInvalid:boolean = false): void { var originalStr = original.join('\n'); var expectedStr = expected.join('\n'); @@ -31,8 +31,10 @@ export function testApplyEditsWithSyncedModels(original:string[], edits:editorCo // Assert the inverse edits brought back model to original state assert.deepEqual(model.getValue(editorCommon.EndOfLinePreference.LF), originalStr); - // Assert the inverse of the inverse edits are the original edits - assert.deepEqual(inverseInverseEdits, edits); + if (!inputEditsAreInvalid) { + // Assert the inverse of the inverse edits are the original edits + assert.deepEqual(inverseInverseEdits, edits); + } assertMirrorModels(); }); diff --git a/src/vs/editor/test/common/model/textModel.test.ts b/src/vs/editor/test/common/model/textModel.test.ts index 0dcfe908a42..78e3d9806bc 100644 --- a/src/vs/editor/test/common/model/textModel.test.ts +++ b/src/vs/editor/test/common/model/textModel.test.ts @@ -437,7 +437,7 @@ suite('Editor Model - TextModel', () => { test('validatePosition', () => { - var m = new TextModel([], TextModel.toRawText('line one\nline two', TextModel.DEFAULT_CREATION_OPTIONS)); + let m = new TextModel([], TextModel.toRawText('line one\nline two', TextModel.DEFAULT_CREATION_OPTIONS)); assert.deepEqual(m.validatePosition(new Position(0, 0)), new Position(1, 1)); assert.deepEqual(m.validatePosition(new Position(0, 1)), new Position(1, 1)); @@ -457,6 +457,123 @@ suite('Editor Model - TextModel', () => { assert.deepEqual(m.validatePosition(new Position(30, 30)), new Position(2, 9)); + assert.deepEqual(m.validatePosition(new Position(-123.123, -0.5)), new Position(1, 1)); + assert.deepEqual(m.validatePosition(new Position(Number.MIN_VALUE, Number.MIN_VALUE)), new Position(1, 1)); + + assert.deepEqual(m.validatePosition(new Position(Number.MAX_VALUE, Number.MAX_VALUE)), new Position(2, 9)); + assert.deepEqual(m.validatePosition(new Position(123.23, 47.5)), new Position(2, 9)); + }); + + test('validatePosition around high-low surrogate pairs 1', () => { + + let m = new TextModel([], TextModel.toRawText('a📚b', TextModel.DEFAULT_CREATION_OPTIONS)); + + assert.deepEqual(m.validatePosition(new Position(0, 0)), new Position(1, 1)); + assert.deepEqual(m.validatePosition(new Position(0, 1)), new Position(1, 1)); + assert.deepEqual(m.validatePosition(new Position(0, 7)), new Position(1, 1)); + + assert.deepEqual(m.validatePosition(new Position(1, 1)), new Position(1, 1)); + assert.deepEqual(m.validatePosition(new Position(1, 2)), new Position(1, 2)); + assert.deepEqual(m.validatePosition(new Position(1, 3)), new Position(1, 2)); + assert.deepEqual(m.validatePosition(new Position(1, 4)), new Position(1, 4)); + assert.deepEqual(m.validatePosition(new Position(1, 5)), new Position(1, 5)); + assert.deepEqual(m.validatePosition(new Position(1, 30)), new Position(1, 5)); + + assert.deepEqual(m.validatePosition(new Position(2, 0)), new Position(1, 5)); + assert.deepEqual(m.validatePosition(new Position(2, 1)), new Position(1, 5)); + assert.deepEqual(m.validatePosition(new Position(2, 2)), new Position(1, 5)); + assert.deepEqual(m.validatePosition(new Position(2, 30)), new Position(1, 5)); + + assert.deepEqual(m.validatePosition(new Position(-123.123, -0.5)), new Position(1, 1)); + assert.deepEqual(m.validatePosition(new Position(Number.MIN_VALUE, Number.MIN_VALUE)), new Position(1, 1)); + + assert.deepEqual(m.validatePosition(new Position(Number.MAX_VALUE, Number.MAX_VALUE)), new Position(1, 5)); + assert.deepEqual(m.validatePosition(new Position(123.23, 47.5)), new Position(1, 5)); + }); + + test('validatePosition around high-low surrogate pairs 2', () => { + + let m = new TextModel([], TextModel.toRawText('a📚📚b', TextModel.DEFAULT_CREATION_OPTIONS)); + + assert.deepEqual(m.validatePosition(new Position(1, 1)), new Position(1, 1)); + assert.deepEqual(m.validatePosition(new Position(1, 2)), new Position(1, 2)); + assert.deepEqual(m.validatePosition(new Position(1, 3)), new Position(1, 2)); + assert.deepEqual(m.validatePosition(new Position(1, 4)), new Position(1, 4)); + assert.deepEqual(m.validatePosition(new Position(1, 5)), new Position(1, 4)); + assert.deepEqual(m.validatePosition(new Position(1, 6)), new Position(1, 6)); + assert.deepEqual(m.validatePosition(new Position(1, 7)), new Position(1, 7)); + + }); + + test('validateRange around high-low surrogate pairs 1', () => { + + let m = new TextModel([], TextModel.toRawText('a📚b', TextModel.DEFAULT_CREATION_OPTIONS)); + + assert.deepEqual(m.validateRange(new Range(0, 0, 0, 1)), new Range(1, 1, 1, 1)); + assert.deepEqual(m.validateRange(new Range(0, 0, 0, 7)), new Range(1, 1, 1, 1)); + + assert.deepEqual(m.validateRange(new Range(1, 1, 1, 1)), new Range(1, 1, 1, 1)); + assert.deepEqual(m.validateRange(new Range(1, 1, 1, 2)), new Range(1, 1, 1, 2)); + assert.deepEqual(m.validateRange(new Range(1, 1, 1, 3)), new Range(1, 1, 1, 4)); + assert.deepEqual(m.validateRange(new Range(1, 1, 1, 4)), new Range(1, 1, 1, 4)); + assert.deepEqual(m.validateRange(new Range(1, 1, 1, 5)), new Range(1, 1, 1, 5)); + + assert.deepEqual(m.validateRange(new Range(1, 2, 1, 2)), new Range(1, 2, 1, 2)); + assert.deepEqual(m.validateRange(new Range(1, 2, 1, 3)), new Range(1, 2, 1, 4)); + assert.deepEqual(m.validateRange(new Range(1, 2, 1, 4)), new Range(1, 2, 1, 4)); + assert.deepEqual(m.validateRange(new Range(1, 2, 1, 5)), new Range(1, 2, 1, 5)); + + assert.deepEqual(m.validateRange(new Range(1, 3, 1, 3)), new Range(1, 2, 1, 2)); + assert.deepEqual(m.validateRange(new Range(1, 3, 1, 4)), new Range(1, 2, 1, 4)); + assert.deepEqual(m.validateRange(new Range(1, 3, 1, 5)), new Range(1, 2, 1, 5)); + + assert.deepEqual(m.validateRange(new Range(1, 4, 1, 4)), new Range(1, 4, 1, 4)); + assert.deepEqual(m.validateRange(new Range(1, 4, 1, 5)), new Range(1, 4, 1, 5)); + + assert.deepEqual(m.validateRange(new Range(1, 5, 1, 5)), new Range(1, 5, 1, 5)); + }); + + test('validateRange around high-low surrogate pairs 2', () => { + + let m = new TextModel([], TextModel.toRawText('a📚📚b', TextModel.DEFAULT_CREATION_OPTIONS)); + + assert.deepEqual(m.validateRange(new Range(0, 0, 0, 1)), new Range(1, 1, 1, 1)); + assert.deepEqual(m.validateRange(new Range(0, 0, 0, 7)), new Range(1, 1, 1, 1)); + + assert.deepEqual(m.validateRange(new Range(1, 1, 1, 1)), new Range(1, 1, 1, 1)); + assert.deepEqual(m.validateRange(new Range(1, 1, 1, 2)), new Range(1, 1, 1, 2)); + assert.deepEqual(m.validateRange(new Range(1, 1, 1, 3)), new Range(1, 1, 1, 4)); + assert.deepEqual(m.validateRange(new Range(1, 1, 1, 4)), new Range(1, 1, 1, 4)); + assert.deepEqual(m.validateRange(new Range(1, 1, 1, 5)), new Range(1, 1, 1, 6)); + assert.deepEqual(m.validateRange(new Range(1, 1, 1, 6)), new Range(1, 1, 1, 6)); + assert.deepEqual(m.validateRange(new Range(1, 1, 1, 7)), new Range(1, 1, 1, 7)); + + assert.deepEqual(m.validateRange(new Range(1, 2, 1, 2)), new Range(1, 2, 1, 2)); + assert.deepEqual(m.validateRange(new Range(1, 2, 1, 3)), new Range(1, 2, 1, 4)); + assert.deepEqual(m.validateRange(new Range(1, 2, 1, 4)), new Range(1, 2, 1, 4)); + assert.deepEqual(m.validateRange(new Range(1, 2, 1, 5)), new Range(1, 2, 1, 6)); + assert.deepEqual(m.validateRange(new Range(1, 2, 1, 6)), new Range(1, 2, 1, 6)); + assert.deepEqual(m.validateRange(new Range(1, 2, 1, 7)), new Range(1, 2, 1, 7)); + + assert.deepEqual(m.validateRange(new Range(1, 3, 1, 3)), new Range(1, 2, 1, 2)); + assert.deepEqual(m.validateRange(new Range(1, 3, 1, 4)), new Range(1, 2, 1, 4)); + assert.deepEqual(m.validateRange(new Range(1, 3, 1, 5)), new Range(1, 2, 1, 6)); + assert.deepEqual(m.validateRange(new Range(1, 3, 1, 6)), new Range(1, 2, 1, 6)); + assert.deepEqual(m.validateRange(new Range(1, 3, 1, 7)), new Range(1, 2, 1, 7)); + + assert.deepEqual(m.validateRange(new Range(1, 4, 1, 4)), new Range(1, 4, 1, 4)); + assert.deepEqual(m.validateRange(new Range(1, 4, 1, 5)), new Range(1, 4, 1, 6)); + assert.deepEqual(m.validateRange(new Range(1, 4, 1, 6)), new Range(1, 4, 1, 6)); + assert.deepEqual(m.validateRange(new Range(1, 4, 1, 7)), new Range(1, 4, 1, 7)); + + assert.deepEqual(m.validateRange(new Range(1, 5, 1, 5)), new Range(1, 4, 1, 4)); + assert.deepEqual(m.validateRange(new Range(1, 5, 1, 6)), new Range(1, 4, 1, 6)); + assert.deepEqual(m.validateRange(new Range(1, 5, 1, 7)), new Range(1, 4, 1, 7)); + + assert.deepEqual(m.validateRange(new Range(1, 6, 1, 6)), new Range(1, 6, 1, 6)); + assert.deepEqual(m.validateRange(new Range(1, 6, 1, 7)), new Range(1, 6, 1, 7)); + + assert.deepEqual(m.validateRange(new Range(1, 7, 1, 7)), new Range(1, 7, 1, 7)); }); test('modifyPosition', () => { -- GitLab