From dd06688bbf6676efdb2ad2c57df4e29f7895b6b5 Mon Sep 17 00:00:00 2001 From: Akos Kitta Date: Thu, 4 May 2023 14:30:38 +0200 Subject: [PATCH] fix: copy when punctuation marks in sketch path Changed the `source` and `cwd` args to avoid accidentally creating an invalid `glob` patterns when doing the brace expansion by `cpy`. Closes #2043 Signed-off-by: Akos Kitta --- .../src/node/sketches-service-impl.ts | 5 +- .../node/sketches-service-impl.slow-test.ts | 89 +++++++++++++++++-- 2 files changed, 87 insertions(+), 7 deletions(-) diff --git a/arduino-ide-extension/src/node/sketches-service-impl.ts b/arduino-ide-extension/src/node/sketches-service-impl.ts index c6fd56a8a..d5c82ff13 100644 --- a/arduino-ide-extension/src/node/sketches-service-impl.ts +++ b/arduino-ide-extension/src/node/sketches-service-impl.ts @@ -444,7 +444,7 @@ export class SketchesServiceImpl * For example, on Windows, instead of getting an [8.3 filename](https://en.wikipedia.org/wiki/8.3_filename), callers will get a fully resolved path. * `C:\\Users\\KITTAA~1\\AppData\\Local\\Temp\\.arduinoIDE-unsaved2022615-21100-iahybb.yyvh\\sketch_jul15a` will be `C:\\Users\\kittaakos\\AppData\\Local\\Temp\\.arduinoIDE-unsaved2022615-21100-iahybb.yyvh\\sketch_jul15a` */ - createTempFolder(): Promise { + private createTempFolder(): Promise { return new Promise((resolve, reject) => { temp.mkdir({ prefix: TempSketchPrefix }, (createError, dirPath) => { if (createError) { @@ -523,13 +523,14 @@ export class SketchesServiceImpl } else { filter = () => true; } - await cpy(source, destination, { + await cpy(sourceFolderBasename, destination, { rename: (basename) => sourceFolderBasename !== destinationFolderBasename && basename === `${sourceFolderBasename}.ino` ? `${destinationFolderBasename}.ino` : basename, filter, + cwd: path.dirname(source), }); const copiedSketch = await this.doLoadSketch(destinationUri, false); return copiedSketch; diff --git a/arduino-ide-extension/src/test/node/sketches-service-impl.slow-test.ts b/arduino-ide-extension/src/test/node/sketches-service-impl.slow-test.ts index 90ceb6679..8daf3a6e5 100644 --- a/arduino-ide-extension/src/test/node/sketches-service-impl.slow-test.ts +++ b/arduino-ide-extension/src/test/node/sketches-service-impl.slow-test.ts @@ -2,6 +2,7 @@ import { Disposable, DisposableCollection, } from '@theia/core/lib/common/disposable'; +import { isWindows } from '@theia/core/lib/common/os'; import { FileUri } from '@theia/core/lib/node/file-uri'; import { Container } from '@theia/core/shared/inversify'; import { expect } from 'chai'; @@ -226,16 +227,94 @@ describe('sketches-service-impl', () => { expect(mainFileContentOneAfterCopy).to.be.equal(contentOne); expect(mainFileContentTwoAfterCopy).to.be.equal(contentOne); }); + + ( + [ + ['(', ')', 'parentheses'], + ['[', ']', 'brackets'], + ['{', '}', 'braces'], + [ + '<', + '>', + 'chevrons', + { + predicate: () => isWindows, + why: '< (less than) and > (greater than) are reserved characters on Windows (https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file#naming-conventions)', + }, + ], + ] as [ + open: string, + close: string, + name: string, + skip?: { predicate: () => boolean; why: string } + ][] + ).map(([open, close, name, skip]) => + it(`should copy a sketch when the path contains ${name} in the sketch folder path: '${open},${close}'`, async function () { + if (skip) { + const { predicate, why } = skip; + if (predicate()) { + console.info(why); + return this.skip(); + } + } + this.timeout(testTimeout); + const sketchesService = + container.get(SketchesService); + const content = `// special content when ${name} are in the path`; + const tempRoot = await sketchesService['createTempFolder'](); + toDispose.push(disposeFolder(tempRoot)); + const sketch = await sketchesService.createNewSketch( + 'punctuation_marks', + content + ); + toDispose.push(disposeSketch(sketch)); + + // the destination path contains punctuation marks + const tempRootUri = FileUri.create(tempRoot); + const testSegment = `path segment with ${open}${name}${close}`; + const firstDestinationUri = tempRootUri + .resolve(testSegment) + .resolve('first') + .resolve(sketch.name); + + const firstSketchCopy = await sketchesService.copy(sketch, { + destinationUri: firstDestinationUri.toString(), + }); + expect(firstSketchCopy).to.be.not.undefined; + expect(firstSketchCopy.mainFileUri).to.be.equal( + firstDestinationUri.resolve(`${sketch.name}.ino`).toString() + ); + const firstCopyContent = await mainFileContentOf(firstSketchCopy); + expect(firstCopyContent).to.be.equal(content); + + // the source path contains punctuation marks. yes, the target too, but it does not matter + const secondDestinationUri = tempRootUri + .resolve(testSegment) + .resolve('second') + .resolve(sketch.name); + const secondSketchCopy = await sketchesService.copy(firstSketchCopy, { + destinationUri: secondDestinationUri.toString(), + }); + expect(secondSketchCopy).to.be.not.undefined; + expect(secondSketchCopy.mainFileUri).to.be.equal( + secondDestinationUri.resolve(`${sketch.name}.ino`).toString() + ); + const secondCopyContent = await mainFileContentOf(secondSketchCopy); + expect(secondCopyContent).to.be.equal(content); + }) + ); }); }); function disposeSketch(...sketch: Sketch[]): Disposable { + return disposeFolder(...sketch.map(({ uri }) => FileUri.fsPath(uri))); +} + +function disposeFolder(...paths: string[]): Disposable { return new DisposableCollection( - ...sketch - .map(({ uri }) => FileUri.fsPath(uri)) - .map((path) => - Disposable.create(() => rimrafSync(path, { maxBusyTries: 5 })) - ) + ...paths.map((path) => + Disposable.create(() => rimrafSync(path, { maxBusyTries: 5 })) + ) ); }