diff --git a/CHANGELOG.md b/CHANGELOG.md index 8943aa947ac2..ff19cd03a647 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,12 @@ 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! +## 2019.6.1 (9 July 2019) + +### Fixes + +1. Fixes to A/B testing. + ([#6400](https://github.com/microsoft/vscode-python/issues/6400)) ## 2019.6.0 (25 June 2019) diff --git a/src/client/common/crypto.ts b/src/client/common/crypto.ts index c25ea6cdc089..c0115ddf8b9f 100644 --- a/src/client/common/crypto.ts +++ b/src/client/common/crypto.ts @@ -4,8 +4,9 @@ // tslint:disable: no-any -import { createHash, HexBase64Latin1Encoding } from 'crypto'; +import { createHash } from 'crypto'; import { injectable } from 'inversify'; +import { traceError } from './logger'; import { ICryptoUtils, IHashFormat } from './types'; /** @@ -13,8 +14,15 @@ import { ICryptoUtils, IHashFormat } from './types'; */ @injectable() export class CryptoUtils implements ICryptoUtils { - public createHash(data: string, encoding: HexBase64Latin1Encoding, hashFormat: E): IHashFormat[E] { - const hash = createHash('sha512').update(data).digest(encoding); - return hashFormat === 'number' ? parseInt(hash, undefined) : hash as any; + public createHash(data: string, hashFormat: E): IHashFormat[E] { + const hash = createHash('sha512').update(data).digest('hex'); + if (hashFormat === 'number') { + const result = parseInt(hash, 16); + if (isNaN(result)) { + traceError(`Number hash for data '${data}' is NaN`); + } + return result as any; + } + return hash as any; } } diff --git a/src/client/common/experiments.ts b/src/client/common/experiments.ts index 9fcc87c5525e..27fefd8bc67d 100644 --- a/src/client/common/experiments.ts +++ b/src/client/common/experiments.ts @@ -148,7 +148,7 @@ export class ExperimentsManager implements IExperimentsManager { if (typeof (this.appEnvironment.machineId) !== 'string') { throw new Error('Machine ID should be a string'); } - const hash = this.crypto.createHash(`${this.appEnvironment.machineId}+${salt}`, 'hex', 'number'); + const hash = this.crypto.createHash(`${this.appEnvironment.machineId}+${salt}`, 'number'); return hash % 100 >= min && hash % 100 < max; } diff --git a/src/client/common/types.ts b/src/client/common/types.ts index 6c0c390bc7ee..f55fcea6e35e 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -2,7 +2,6 @@ // Licensed under the MIT License. 'use strict'; -import { HexBase64Latin1Encoding } from 'crypto'; import { Socket } from 'net'; import { Request as RequestResult } from 'request'; import { ConfigurationTarget, DiagnosticSeverity, Disposable, DocumentSymbolProvider, Event, Extension, ExtensionContext, OutputChannel, Uri, WorkspaceEdit } from 'vscode'; @@ -485,10 +484,9 @@ export interface ICryptoUtils { * Creates hash using the data and encoding specified * @returns hash as number, or string * @param data The string to hash - * @param encoding Data encoding to use * @param hashFormat Return format of the hash, number or string */ - createHash(data: string, encoding: HexBase64Latin1Encoding, hashFormat: E): IHashFormat[E]; + createHash(data: string, hashFormat: E): IHashFormat[E]; } export const IAsyncDisposableRegistry = Symbol('IAsyncDisposableRegistry'); diff --git a/src/test/common/crypto.unit.test.ts b/src/test/common/crypto.unit.test.ts index a7274f3900af..6dfef4737eef 100644 --- a/src/test/common/crypto.unit.test.ts +++ b/src/test/common/crypto.unit.test.ts @@ -12,11 +12,37 @@ suite('Crypto Utils', async () => { crypto = new CryptoUtils(); }); test('If hashFormat equals `number`, method createHash() returns a number', async () => { - const hash = crypto.createHash('blabla', 'hex', 'number'); + const hash = crypto.createHash('blabla', 'number'); assert.typeOf(hash, 'number', 'Type should be a number'); }); test('If hashFormat equals `string`, method createHash() returns a string', async () => { - const hash = crypto.createHash('blabla', 'hex', 'string'); + const hash = crypto.createHash('blabla', 'string'); assert.typeOf(hash, 'string', 'Type should be a string'); }); + test('If hashFormat equals `number`, the hash should not be NaN', async () => { + let hash = crypto.createHash('test', 'number'); + assert.isNotNaN(hash, 'Number hash should not be NaN'); + hash = crypto.createHash('hash', 'number'); + assert.isNotNaN(hash, 'Number hash should not be NaN'); + hash = crypto.createHash('HASH1', 'number'); + assert.isNotNaN(hash, 'Number hash should not be NaN'); + }); + test('If hashFormat equals `string`, the hash should not be undefined', async () => { + let hash = crypto.createHash('test', 'string'); + assert.isDefined(hash, 'String hash should not be undefined'); + hash = crypto.createHash('hash', 'string'); + assert.isDefined(hash, 'String hash should not be undefined'); + hash = crypto.createHash('HASH1', 'string'); + assert.isDefined(hash, 'String hash should not be undefined'); + }); + test('If hashFormat equals `number`, hashes with different data should return different number hashes', async () => { + const hash1 = crypto.createHash('hash1', 'number'); + const hash2 = crypto.createHash('hash2', 'number'); + assert.notEqual(hash1, hash2, 'Hashes should be different numbers'); + }); + test('If hashFormat equals `string`, hashes with different data should return different string hashes', async () => { + const hash1 = crypto.createHash('hash1', 'string'); + const hash2 = crypto.createHash('hash2', 'string'); + assert.notEqual(hash1, hash2, 'Hashes should be different strings'); + }); }); diff --git a/src/test/common/experiments.unit.test.ts b/src/test/common/experiments.unit.test.ts index 870f47f665fb..ef38ffbc8935 100644 --- a/src/test/common/experiments.unit.test.ts +++ b/src/test/common/experiments.unit.test.ts @@ -599,10 +599,10 @@ suite('A/B experiments', () => { expect(() => expManager.isUserInRange(79, 94, 'salt')).to.throw(); } else if (testParams.error) { const error = new Error('Kaboom'); - when(crypto.createHash(anything(), 'hex', 'number')).thenThrow(error); + when(crypto.createHash(anything(), 'number')).thenThrow(error); expect(() => expManager.isUserInRange(79, 94, 'salt')).to.throw(error); } else { - when(crypto.createHash(anything(), 'hex', 'number')).thenReturn(testParams.hash); + when(crypto.createHash(anything(), 'number')).thenReturn(testParams.hash); expect(expManager.isUserInRange(79, 94, 'salt')).to.equal(testParams.expectedResult, 'Incorrectly identified'); } }); @@ -636,7 +636,7 @@ suite('A/B experiments', () => { .returns(() => testParams.experimentStorageValue); when(appEnvironment.machineId).thenReturn('101'); if (testParams.hash) { - when(crypto.createHash(anything(), 'hex', 'number')).thenReturn(testParams.hash); + when(crypto.createHash(anything(), 'number')).thenReturn(testParams.hash); } expManager.populateUserExperiments(); assert.deepEqual(expManager.userExperiments, testParams.expectedResult);