Skip to content

Commit

Permalink
fix: debugger failing on Node <=12 (#1627)
Browse files Browse the repository at this point in the history
* fix: sourcemap lookups on ipv6 localhost addresses

TIL that assigning an invalid value to `url.hostname` silently fails
if invalid.

Fixes microsoft/vscode#167353

* fix: debugger failing on Node <=12

Fixes #1624

Updates @vscode/l10n to allow it to be tree-shaken away.

Also moves `checkContentHash` that likewise had a dependency on more
modern language features. I think webpack was just extra aggressive
about tree shaking before and assumed `new Hasher()` was side effect
free. Which is was, it's just a big assumption to have made.

And when I had updated versions in our pipelines, I accidentally updated
the minspec to no longer target Node 8 🤦‍♂️. Targeting the latest 10 should
be good. Node 8 thankfully <0.25% of users nowadays.

* keep broken minspec for the moment
  • Loading branch information
connor4312 committed Mar 31, 2023
1 parent 1c46464 commit 1fd5f82
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 41 deletions.
14 changes: 7 additions & 7 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
"dependencies": {
"@c4312/chromehash": "^0.3.0",
"@vscode/js-debug-browsers": "^1.0.8",
"@vscode/l10n": "^0.0.11",
"@vscode/l10n": "^0.0.13",
"@vscode/win32-app-container-tokens": "^0.1.0",
"acorn": "^8.7.0",
"acorn-loose": "^8.3.0",
Expand Down
3 changes: 2 additions & 1 deletion src/adapter/sources.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { URL } from 'url';
import Cdp from '../cdp/api';
import { MapUsingProjection } from '../common/datastructure/mapUsingProjection';
import { EventEmitter } from '../common/events';
import { checkContentHash } from '../common/hash/checkContentHash';
import { ILogger, LogTag } from '../common/logging';
import { once } from '../common/objUtils';
import { forceForwardSlashes, isSubdirectoryOf, properResolve } from '../common/pathUtils';
Expand Down Expand Up @@ -173,7 +174,7 @@ export class Source {
this._name = this._humanName();
this.setSourceMapUrl(sourceMapUrl);

this._existingAbsolutePath = sourceUtils.checkContentHash(
this._existingAbsolutePath = checkContentHash(
this.absolutePath,
// Inline scripts will never match content of the html file. We skip the content check.
inlineScriptOffset || runtimeScriptOffset ? undefined : contentHash,
Expand Down
36 changes: 36 additions & 0 deletions src/common/hash/checkContentHash.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*---------------------------------------------------------
* Copyright (C) Microsoft Corporation. All rights reserved.
*--------------------------------------------------------*/

import { Hasher } from '.';
import { LocalFsUtils } from '../fsUtils';
import { isWithinAsar } from '../pathUtils';
import { promises as fsPromises } from 'fs';

const hasher = new Hasher();

export async function checkContentHash(
absolutePath: string,
contentHash?: string,
contentOverride?: string,
): Promise<string | undefined> {
if (!absolutePath) {
return undefined;
}

if (isWithinAsar(absolutePath)) {
return undefined;
}

if (!contentHash) {
const exists = await new LocalFsUtils(fsPromises).exists(absolutePath);
return exists ? absolutePath : undefined;
}

const result =
typeof contentOverride === 'string'
? await hasher.verifyBytes(contentOverride, contentHash, true)
: await hasher.verifyFile(absolutePath, contentHash, true);

return result ? absolutePath : undefined;
}
32 changes: 0 additions & 32 deletions src/common/sourceUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,8 @@ import {
Program,
Statement,
} from 'estree';
import { promises as fsPromises } from 'fs';
import { NullablePosition, Position, SourceMapConsumer, SourceMapGenerator } from 'source-map';
import { LineColumn } from '../adapter/breakpoints/breakpointBase';
import { LocalFsUtils } from './fsUtils';
import { Hasher } from './hash';
import { isWithinAsar } from './pathUtils';
import { acornOptions, parseProgram } from './sourceCodeManipulations';
import { SourceMap } from './sourceMaps/sourceMap';

Expand Down Expand Up @@ -248,34 +244,6 @@ export function parseSourceMappingUrl(content: string): string | undefined {
return sourceMapUrl.trim();
}

const hasher = new Hasher();

export async function checkContentHash(
absolutePath: string,
contentHash?: string,
contentOverride?: string,
): Promise<string | undefined> {
if (!absolutePath) {
return undefined;
}

if (isWithinAsar(absolutePath)) {
return undefined;
}

if (!contentHash) {
const exists = await new LocalFsUtils(fsPromises).exists(absolutePath);
return exists ? absolutePath : undefined;
}

const result =
typeof contentOverride === 'string'
? await hasher.verifyBytes(contentOverride, contentHash, true)
: await hasher.verifyFile(absolutePath, contentHash, true);

return result ? absolutePath : undefined;
}

interface INotNullRange {
line: number;
column: number;
Expand Down

0 comments on commit 1fd5f82

Please sign in to comment.