Skip to content

Commit 3bceb5d

Browse files
DonJayamanneDonJayamanne
authored
Changed logic used to determine validity of debugger executable (microsoft#2708)
For microsoft#2351
1 parent 7ac1b6d commit 3bceb5d

6 files changed

Lines changed: 124 additions & 25 deletions

File tree

‎pythonFiles/printOne.py‎

Lines changed: 0 additions & 4 deletions
This file was deleted.

‎src/client/debugger/executableValidator.ts‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
'use strict';
5+
6+
import { inject, injectable } from 'inversify';
7+
import { IFileSystem } from '../common/platform/types';
8+
import { IProcessServiceFactory } from '../common/process/types';
9+
import { IServiceContainer } from '../ioc/types';
10+
import { IExcutableValidator } from './types';
11+
12+
@injectable()
13+
export class ExcutableValidator implements IExcutableValidator {
14+
constructor(@inject(IServiceContainer) private readonly serviceContainer: IServiceContainer) { }
15+
/**
16+
* Checks the validity of the executable.
17+
* Do not use `<python> --version` as the output in 2.7 comes in stderr.
18+
* Check if either one of the following is true:
19+
* 1. Use `<python> -c print('1')` as the executable could python.
20+
* 2. Check if the executable file exists (in case of `spark-submit`)
21+
* @param {string} exePath
22+
* @returns {Promise<boolean>}
23+
* @memberof DebuggerExcutableValidator
24+
*/
25+
public async validateExecutable(exePath: string): Promise<boolean> {
26+
const processFactory = this.serviceContainer.get<IProcessServiceFactory>(IProcessServiceFactory);
27+
const fs = this.serviceContainer.get<IFileSystem>(IFileSystem);
28+
const processService = await processFactory.create();
29+
const [valid, execuableExists] = await Promise.all([
30+
processService.exec(exePath, ['-c', 'print("1")'])
31+
.then(output => output.stdout.trim() === '1')
32+
.catch(() => false),
33+
fs.fileExists(exePath)
34+
]);
35+
36+
return valid || execuableExists;
37+
}
38+
39+
}

‎src/client/debugger/mainV2.ts‎

Lines changed: 3 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,15 @@ import { DebugProtocol } from 'vscode-debugprotocol';
2020
import '../../client/common/extensions';
2121
import { createDeferred, Deferred, sleep } from '../../utils/async';
2222
import { noop } from '../../utils/misc';
23-
import { EXTENSION_ROOT_DIR } from '../common/constants';
2423
import { isNotInstalledError } from '../common/helpers';
2524
import { IFileSystem } from '../common/platform/types';
26-
import { IProcessServiceFactory } from '../common/process/types';
2725
import { ICurrentProcess } from '../common/types';
2826
import { IServiceContainer } from '../ioc/types';
2927
import { AttachRequestArguments, LaunchRequestArguments } from './Common/Contracts';
3028
import { CreateAttachDebugClient, CreateLaunchDebugClient } from './DebugClients/DebugFactory';
3129
import { BaseDebugServer } from './DebugServers/BaseDebugServer';
3230
import { initializeIoc } from './serviceRegistry';
33-
import { IDebugStreamProvider, IProtocolLogger, IProtocolMessageWriter, IProtocolParser } from './types';
31+
import { IDebugStreamProvider, IExcutableValidator, IProtocolLogger, IProtocolMessageWriter, IProtocolParser } from './types';
3432
const killProcessTree = require('tree-kill');
3533

3634
const DEBUGGER_CONNECT_TIMEOUT = 20000;
@@ -124,25 +122,10 @@ export class PythonDebugger extends DebugSession {
124122
this.sendErrorResponse(response, { format: message, id: 1 }, undefined, undefined, ErrorDestination.User);
125123
});
126124
}
127-
/**
128-
* Checks the validity of the executable.
129-
* Do not use `<python> --version` as the output in 2.7 comes in stderr.
130-
* Do not use `<python> -c print('1')` as the executable could be pyspark.
131-
* Use `<python> xyz.py` to check output as this is absolutely necessary for debugger to start.
132-
* @private
133-
* @param {DebugProtocol.LaunchResponse} response
134-
* @param {LaunchRequestArguments} args
135-
* @returns {Promise<boolean>}
136-
* @memberof PythonDebugger
137-
*/
138125
private async validatePythonPath(response: DebugProtocol.LaunchResponse, args: LaunchRequestArguments): Promise<boolean> {
139126
const pythonPath = typeof args.pythonPath === 'string' && args.pythonPath.length > 0 ? args.pythonPath : 'python';
140-
const processFactory = this.serviceContainer.get<IProcessServiceFactory>(IProcessServiceFactory);
141-
const processService = await processFactory.create();
142-
const pythonFile = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'printOne.py');
143-
const valid = await processService.exec(pythonPath, [pythonFile])
144-
.then(output => output.stdout.trim() === '1' || (output.stderr || '').trim() === '1')
145-
.catch(() => false);
127+
const validator = this.serviceContainer.get<IExcutableValidator>(IExcutableValidator);
128+
const valid = await validator.validateExecutable(pythonPath);
146129
if (!valid) {
147130
this.sendErrorResponse(response, { format: InvalidPythonPathInDebuggerMessage, id: 2 }, undefined, undefined, ErrorDestination.User);
148131
}

‎src/client/debugger/serviceRegistry.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ import { DebuggerProcessServiceFactory } from './Common/processServiceFactory';
2121
import { ProtocolLogger } from './Common/protocolLogger';
2222
import { ProtocolParser } from './Common/protocolParser';
2323
import { ProtocolMessageWriter } from './Common/protocolWriter';
24-
import { IDebuggerBanner, IDebugStreamProvider, IProtocolLogger, IProtocolMessageWriter, IProtocolParser } from './types';
24+
import { ExcutableValidator } from './executableValidator';
25+
import { IDebuggerBanner, IDebugStreamProvider, IExcutableValidator, IProtocolLogger, IProtocolMessageWriter, IProtocolParser } from './types';
2526

2627
export function initializeIoc(): IServiceContainer {
2728
const cont = new Container();
@@ -43,6 +44,7 @@ function registerDebuggerTypes(serviceManager: IServiceManager) {
4344
serviceManager.addSingleton<IProtocolMessageWriter>(IProtocolMessageWriter, ProtocolMessageWriter);
4445
serviceManager.addSingleton<IBufferDecoder>(IBufferDecoder, BufferDecoder);
4546
serviceManager.addSingleton<IProcessServiceFactory>(IProcessServiceFactory, DebuggerProcessServiceFactory);
47+
serviceManager.addSingleton<IExcutableValidator>(IExcutableValidator, ExcutableValidator);
4648
}
4749

4850
export function registerTypes(serviceManager: IServiceManager) {

‎src/client/debugger/types.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,3 +42,8 @@ export const IDebuggerBanner = Symbol('IDebuggerBanner');
4242
export interface IDebuggerBanner {
4343
initialize(): void;
4444
}
45+
46+
export const IExcutableValidator = Symbol('IExcutableValidator');
47+
export interface IExcutableValidator {
48+
validateExecutable(exePath: string): Promise<boolean>;
49+
}

‎src/test/debugger/excutableValidator.unit.test.ts‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
'use strict';
5+
6+
// tslint:disable:no-any
7+
8+
import { expect } from 'chai';
9+
import * as path from 'path';
10+
import * as typeMoq from 'typemoq';
11+
import { IFileSystem } from '../../client/common/platform/types';
12+
import { ExecutionResult, IProcessService, IProcessServiceFactory } from '../../client/common/process/types';
13+
import { ExcutableValidator } from '../../client/debugger/executableValidator';
14+
import { IServiceContainer } from '../../client/ioc/types';
15+
16+
suite('Debugger Executable Validator', () => {
17+
let fs: typeMoq.IMock<IFileSystem>;
18+
let processService: typeMoq.IMock<IProcessService>;
19+
let validator: ExcutableValidator;
20+
setup(() => {
21+
const serviceContainer = typeMoq.Mock.ofType<IServiceContainer>();
22+
fs = typeMoq.Mock.ofType<IFileSystem>(undefined, typeMoq.MockBehavior.Strict);
23+
processService = typeMoq.Mock.ofType<IProcessService>();
24+
processService.setup((p: any) => p.then).returns(() => undefined);
25+
const processFactory = typeMoq.Mock.ofType<IProcessServiceFactory>();
26+
processFactory.setup(p => p.create()).returns(() => Promise.resolve(processService.object));
27+
28+
serviceContainer.setup(c => c.get(typeMoq.It.isValue(IFileSystem))).returns(() => fs.object);
29+
serviceContainer.setup(c => c.get(typeMoq.It.isValue(IProcessServiceFactory))).returns(() => processFactory.object);
30+
validator = new ExcutableValidator(serviceContainer.object);
31+
});
32+
33+
async function validate(pythonPath: string, expectedResult: boolean, fileExists: boolean, processOutput: ExecutionResult<string>) {
34+
fs.setup(f => f.fileExists(typeMoq.It.isValue(pythonPath))).returns(() => Promise.resolve(fileExists));
35+
processService.setup(p => p.exec(typeMoq.It.isValue(pythonPath), typeMoq.It.isValue(['-c', 'print("1")'])))
36+
.returns(() => Promise.resolve(processOutput))
37+
.verifiable(typeMoq.Times.once());
38+
39+
const isValid = await validator.validateExecutable(pythonPath);
40+
41+
expect(isValid).to.be.equal(expectedResult, 'Incorrect value');
42+
fs.verifyAll();
43+
processService.verifyAll();
44+
}
45+
test('Validate \'python\' command', async () => {
46+
const pythonPath = 'python';
47+
const output = { stdout: '1' };
48+
await validate(pythonPath, true, false, output);
49+
});
50+
51+
test('Validate \'python\' Executable with a valida path', async () => {
52+
const pythonPath = path.join('a', 'b', 'bin', 'python');
53+
const output = { stdout: '1' };
54+
await validate(pythonPath, true, true, output);
55+
});
56+
57+
test('Validate \'spark-submit\'', async () => {
58+
const pythonPath = path.join('a', 'b', 'bin', 'spark-submit');
59+
const output = { stderr: 'ex', stdout: '' };
60+
await validate(pythonPath, true, true, output);
61+
});
62+
63+
test('Validate invalid \'python\' command', async () => {
64+
const pythonPath = path.join('python');
65+
const output = { stderr: 'ex', stdout: '' };
66+
await validate(pythonPath, false, false, output);
67+
});
68+
69+
test('Validate invalid executable', async () => {
70+
const pythonPath = path.join('a', 'b', 'bin', 'spark-submit');
71+
const output = { stderr: 'ex', stdout: '' };
72+
await validate(pythonPath, false, false, output);
73+
});
74+
});

0 commit comments

Comments
 (0)