Skip to content

Commit dd192f2

Browse files
DonJayamanneluabud
authored andcommitted
Ignore formatting in ipynb when dealing with trust (microsoft#14333)
1 parent 6b51b85 commit dd192f2

File tree

2 files changed

+27
-5
lines changed

2 files changed

+27
-5
lines changed

src/client/datascience/interactive-ipynb/trustService.ts

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import { createHmac } from 'crypto';
22
import { inject, injectable } from 'inversify';
33
import { EventEmitter, Uri } from 'vscode';
4+
import { traceError } from '../../common/logger';
45
import { IConfigurationService } from '../../common/types';
6+
import { sortObjectPropertiesRecursively } from '../notebookStorage/vscNotebookModel';
57
import { IDigestStorage, ITrustService } from '../types';
68

79
@injectable()
@@ -29,9 +31,14 @@ export class TrustService implements ITrustService {
2931
if (this.alwaysTrustNotebooks) {
3032
return true; // Skip check if user manually overrode our trust checking
3133
}
32-
// Compute digest and see if notebook is trusted
33-
const digest = await this.computeDigest(notebookContents);
34-
return this.digestStorage.containsDigest(uri, digest);
34+
// Compute digest and see if notebook is trusted.
35+
// Check formatted & unformatted notebook. Possible user saved nb using old extension & opening using new extension.
36+
const [digest1, digest2] = await Promise.all([
37+
this.computeDigest(notebookContents),
38+
this.computeDigest(this.getFormattedContents(notebookContents))
39+
]);
40+
41+
return this.digestStorage.containsDigest(uri, digest1) || this.digestStorage.containsDigest(uri, digest2);
3542
}
3643

3744
/**
@@ -41,13 +48,28 @@ export class TrustService implements ITrustService {
4148
*/
4249
public async trustNotebook(uri: Uri, notebookContents: string) {
4350
if (!this.alwaysTrustNotebooks) {
51+
notebookContents = this.getFormattedContents(notebookContents);
4452
// Only update digest store if the user wants us to check trust
4553
const digest = await this.computeDigest(notebookContents);
4654
await this.digestStorage.saveDigest(uri, digest);
4755
this._onDidSetNotebookTrust.fire();
4856
}
4957
}
50-
58+
/**
59+
* If a notebook is opened & saved in Jupyter, even without making any changes, the JSON in ipynb could be different from the format saved by VSC.
60+
* Similarly, the JSON saved by native notebooks could be different when compared to how they are saved by standard notebooks.
61+
* When trusting a notebook, we don't trust the raw bytes in ipynb, we trust the contents, & ipynb stores JSON,
62+
* Hence when computing a hash we need to ensure the hash is always the same regardless of indentation of JSON & the order of properties in json.
63+
* This method returns the contents of the ipynb in a manner thats solves formatting issues related to JSON.
64+
*/
65+
private getFormattedContents(notebookContents: string) {
66+
try {
67+
return JSON.stringify(sortObjectPropertiesRecursively(JSON.parse(notebookContents)));
68+
} catch (ex) {
69+
traceError('Notebook cannot be parsed into JSON', ex);
70+
return notebookContents;
71+
}
72+
}
5173
private async computeDigest(notebookContents: string) {
5274
const hmac = createHmac('sha256', await this.digestStorage.key);
5375
hmac.update(notebookContents);

src/client/datascience/notebookStorage/vscNotebookModel.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import { BaseNotebookModel, getDefaultNotebookContentForNativeNotebooks } from '
1919

2020
// https://github.com/microsoft/vscode-python/issues/13155
2121
// tslint:disable-next-line: no-any
22-
function sortObjectPropertiesRecursively(obj: any): any {
22+
export function sortObjectPropertiesRecursively(obj: any): any {
2323
if (Array.isArray(obj)) {
2424
return obj.map(sortObjectPropertiesRecursively);
2525
}

0 commit comments

Comments
 (0)