Skip to content

Commit 0390509

Browse files
authored
fix(rush-lib): replace deprecated url.parse() with WHATWG URL API in Git.normalizeGitUrlForComparison (microsoft#5657)
Replace the deprecated Node.js url.parse() call with the WHATWG URL constructor in Git.normalizeGitUrlForComparison(). The URL constructor is wrapped in a try/catch to gracefully handle non-URL strings (local paths, etc.) that would throw. The WHATWG URL API correctly strips default ports (e.g. port 80 for HTTP), which is more correct for URL comparison purposes. Updated the corresponding test expectation accordingly.
1 parent 9a55130 commit 0390509

3 files changed

Lines changed: 37 additions & 21 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"changes": [
3+
{
4+
"comment": "",
5+
"type": "none",
6+
"packageName": "@microsoft/rush"
7+
}
8+
],
9+
"packageName": "@microsoft/rush",
10+
"email": "iclanton@users.noreply.github.com"
11+
}

libraries/rush-lib/src/logic/Git.ts

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33

44
import type child_process from 'node:child_process';
55
import * as path from 'node:path';
6-
import * as url from 'node:url';
76

87
import gitInfo from 'git-repo-info';
98
import { trueCasePathSync } from 'true-case-path';
@@ -487,25 +486,31 @@ export class Git {
487486
}
488487
}
489488

490-
const parsedUrl: url.UrlWithStringQuery = url.parse(result);
491-
492-
// Only convert recognized schemes
493-
494-
switch (parsedUrl.protocol) {
495-
case 'http:':
496-
case 'https:':
497-
case 'ssh:':
498-
case 'ftp:':
499-
case 'ftps:':
500-
case 'git:':
501-
case 'git+http:':
502-
case 'git+https:':
503-
case 'git+ssh:':
504-
case 'git+ftp:':
505-
case 'git+ftps:':
506-
// Assemble the parts we want:
507-
result = `https://${parsedUrl.host}${parsedUrl.pathname}`;
508-
break;
489+
// Use the WHATWG URL API instead of the deprecated url.parse().
490+
// The URL constructor throws for non-standard URLs (e.g. local paths), so we
491+
// catch and leave the result unchanged in that case.
492+
try {
493+
const parsedUrl: URL = new URL(result);
494+
495+
// Only convert recognized schemes
496+
switch (parsedUrl.protocol) {
497+
case 'http:':
498+
case 'https:':
499+
case 'ssh:':
500+
case 'ftp:':
501+
case 'ftps:':
502+
case 'git:':
503+
case 'git+http:':
504+
case 'git+https:':
505+
case 'git+ssh:':
506+
case 'git+ftp:':
507+
case 'git+ftps:':
508+
// Assemble the parts we want:
509+
result = `https://${parsedUrl.host}${parsedUrl.pathname}`;
510+
break;
511+
}
512+
} catch {
513+
// Not a valid URL (e.g. a local path) -- leave result unchanged
509514
}
510515

511516
// Trim ".git" or ".git/" from the end

libraries/rush-lib/src/logic/test/Git.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ describe(Git.name, () => {
1919
'https://host.xz/path/to/repo'
2020
);
2121
expect(Git.normalizeGitUrlForComparison('http://host.xz:80/path/to/repo')).toEqual(
22-
'https://host.xz:80/path/to/repo'
22+
'https://host.xz/path/to/repo'
2323
);
2424
expect(Git.normalizeGitUrlForComparison('host.xz:path/to/repo.git/')).toEqual(
2525
'https://host.xz/path/to/repo'

0 commit comments

Comments
 (0)