From c25f4b4a0b0faad559dc7e0c8865abe5afd1dded Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Mon, 8 Jul 2019 11:44:53 -0700 Subject: [PATCH 1/3] Simplify & fix crypto.createHash --- src/client/common/crypto.ts | 8 ++++---- src/client/common/experiments.ts | 2 +- src/client/common/types.ts | 4 +--- src/test/common/crypto.unit.test.ts | 4 ++-- src/test/common/experiments.unit.test.ts | 6 +++--- 5 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/client/common/crypto.ts b/src/client/common/crypto.ts index c25ea6cdc089..1d599737d46e 100644 --- a/src/client/common/crypto.ts +++ b/src/client/common/crypto.ts @@ -4,7 +4,7 @@ // tslint:disable: no-any -import { createHash, HexBase64Latin1Encoding } from 'crypto'; +import { createHash } from 'crypto'; import { injectable } from 'inversify'; import { ICryptoUtils, IHashFormat } from './types'; @@ -13,8 +13,8 @@ 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'); + return hashFormat === 'number' ? parseInt(hash, 16) : hash as any; } } diff --git a/src/client/common/experiments.ts b/src/client/common/experiments.ts index 4960dc96ff41..eb922edb0ddb 100644 --- a/src/client/common/experiments.ts +++ b/src/client/common/experiments.ts @@ -150,7 +150,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 8d35997b6670..145395af7ee8 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'; @@ -439,10 +438,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..bca660256d17 100644 --- a/src/test/common/crypto.unit.test.ts +++ b/src/test/common/crypto.unit.test.ts @@ -12,11 +12,11 @@ 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'); }); }); diff --git a/src/test/common/experiments.unit.test.ts b/src/test/common/experiments.unit.test.ts index 82cf97bce3e7..0dd97a433b8a 100644 --- a/src/test/common/experiments.unit.test.ts +++ b/src/test/common/experiments.unit.test.ts @@ -588,10 +588,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'); } }); @@ -625,7 +625,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); From e899f2112aac423dc2a1f3e1b8eddfdd73a37dd7 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Mon, 8 Jul 2019 11:45:00 -0700 Subject: [PATCH 2/3] Add tests --- src/test/common/crypto.unit.test.ts | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/test/common/crypto.unit.test.ts b/src/test/common/crypto.unit.test.ts index bca660256d17..6dfef4737eef 100644 --- a/src/test/common/crypto.unit.test.ts +++ b/src/test/common/crypto.unit.test.ts @@ -19,4 +19,30 @@ suite('Crypto Utils', async () => { 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'); + }); }); From 01b3b977502091f9e209edc687c965361b5d83ea Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 1 Jul 2019 10:07:51 -0700 Subject: [PATCH 3/3] Fix python test in news folder (#6398) * Fix broken python tests in news * Revert changes --- news/test_announce.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/news/test_announce.py b/news/test_announce.py index 54233e30115f..acc125a7c360 100644 --- a/news/test_announce.py +++ b/news/test_announce.py @@ -187,7 +187,8 @@ def fake_git_rm(path): def test_complete_news(): version = "2019.3.0" - date = datetime.date.today().strftime("%d %B %Y") + # Remove leading `0`. + date = datetime.date.today().strftime("%d %B %Y").lstrip("0") news = ann.complete_news(version, NEW_NEWS, f"{TITLE}\n\n\n{OLD_NEWS}") expected = f"{TITLE}\n\n## {version} ({date})\n\n{NEW_NEWS.strip()}\n\n\n{OLD_NEWS.strip()}" assert news == expected