Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Refactor] Replace url parse format resolve with whatwg url #2910

Merged
Show file tree
Hide file tree
Changes from 51 commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
4a67aec
update test/server_integration/http/ssl_redirect/config.js Url to for…
wanglam Nov 7, 2022
c37c80d
update for examples/url_generators_examples/public/url_generator.ts
wanglam Nov 7, 2022
c2511a8
update packages/osd-dev-utils/src/osd_client/osd_client_requester.ts
wanglam Nov 7, 2022
2921fda
update src/cli_plugin/install/download.js
wanglam Nov 7, 2022
3247937
update packages/osd-test/src/failed_tests_reporter/github_api.ts
wanglam Nov 7, 2022
defe504
update packages/osd-test/src/functional_test_runner/lib/docker_server…
wanglam Nov 7, 2022
12cc53f
update src/cli/serve/serve.js
wanglam Nov 7, 2022
2218ede
update src/core/public/application/utils/parse_app_url.ts
wanglam Nov 7, 2022
3fc62a3
update src/core/public/chrome/chrome_service.tsx
wanglam Nov 7, 2022
d1cc130
update src/core/public/chrome/ui/header/header_logo.tsx
wanglam Nov 7, 2022
14b5497
update src/core/server/http/base_path_proxy_server.ts
wanglam Nov 7, 2022
a76fd60
update src/core/server/http/http_server.mocks.ts
wanglam Nov 7, 2022
592301b
update src/core/server/http/http_server.ts
wanglam Nov 7, 2022
5fabfd3
update src/core/server/http/https_redirect_server.ts
wanglam Nov 7, 2022
7c5e014
update src/core/server/opensearch/legacy/opensearch_client_config.ts
wanglam Nov 7, 2022
1517b5a
update src/plugins/console/server/lib/opensearch_proxy_config.ts
wanglam Nov 8, 2022
7248b3f
update src/plugins/console/server/lib/proxy_config.ts
wanglam Nov 8, 2022
fea59fe
update src/plugins/console/server/routes/api/console/proxy/create_han…
wanglam Nov 8, 2022
73a0cb4
update src/plugins/maps_legacy/public/map/service_settings.test.js
wanglam Nov 8, 2022
0a293d1
update src/plugins/share/public/lib/url_shortener.ts
wanglam Nov 8, 2022
7792efa
update test/plugin_functional/test_suites/application_links/redirect_…
wanglam Nov 8, 2022
652a81c
update getOpenSearchDashboardsUrl method in funtional tests
wanglam Nov 8, 2022
7b0a2fc
change config servers.opensearch and servers.opensearchDashboards to URL
wanglam Nov 8, 2022
90e29a1
update src/plugins/share/public/components/url_panel_content.tsx
wanglam Nov 10, 2022
9e8818c
update expected url due to parentheses and single quotation mark encoded
wanglam Nov 22, 2022
f683ba2
remove unnecessary imported URL
wanglam Dec 13, 2022
b53ee93
add force validate new URL create
wanglam Dec 13, 2022
c910dea
add changelog
wanglam Dec 13, 2022
3d84d50
Merge branch 'main' into refactor-replace-url-parse-format-resolve-wi…
joshuarrrr Dec 15, 2022
b73ac85
update to use object spread construct URLSearchParams
wanglam Dec 16, 2022
507c3a8
renaming getUrlParts to getURL
wanglam Dec 16, 2022
3bf1cad
Merge branch 'main' into refactor-replace-url-parse-format-resolve-wi…
wanglam Dec 20, 2022
f5a13f0
fix force url validate remove hash part
wanglam Dec 29, 2022
1731fc0
update opensearch config getUrlParts to getURL
wanglam Dec 29, 2022
2d007f0
remove search string compare in header_logo
wanglam Dec 30, 2022
c2feeb4
fix: url was truncated incorrectly
wanglam Jan 12, 2023
610ca30
feat: parse URL string port to number port
wanglam Jan 12, 2023
91c36b9
Revert "change config servers.opensearch and servers.opensearchDashbo…
wanglam Jan 28, 2023
a2d7108
feat: add fullURL to urlPartsSchema and use fullURL generate url string
wanglam Feb 9, 2023
32ec422
fix: change getURLParts to getUrlParts
wanglam Feb 9, 2023
3408cc5
Merge branch 'main' into refactor-replace-url-parse-format-resolve-wi…
wanglam Feb 9, 2023
27529b1
fix: url parts pass to getUrl.noAuth in test/functional/page_objects/…
wanglam Feb 9, 2023
803c96a
fix: colon suffix missed in protocol
wanglam Feb 10, 2023
63a8e87
fix: remove all slash suffix for fullURL toString
wanglam Feb 10, 2023
12d2e78
fix: slice(0, 1) cause invalid URL
wanglam Feb 10, 2023
4b061b3
Merge branch 'main' into refactor-replace-url-parse-format-resolve-wi…
wanglam Feb 10, 2023
ec8ce6a
fix: use pared protocol, hostname and port
wanglam Feb 10, 2023
d8ad24d
fix: clone new URL and modify username and password
wanglam Feb 10, 2023
c74698b
fix: slice(0 - 1) cause invalid opensearch host
wanglam Feb 10, 2023
9154295
Merge branch 'main' into refactor-replace-url-parse-format-resolve-wi…
wanglam Feb 13, 2023
6ebdd20
update src/core/public/chrome/ui/header/home_loader.tsx
wanglam Feb 14, 2023
3d93282
Merge branch 'main' into refactor-replace-url-parse-format-resolve-wi…
wanglam Mar 21, 2023
a846ba1
Add serverUrl property and refactor slice(0, -1) to it
wanglam Mar 21, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- Temporary workaround for task-kill exceptions on Windows when it is passed a pid for a process that is already dead ([#2842](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2842))
- [Vis Builder] Fix empty workspace animation does not work in firefox ([#2853](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2853))
- Bumped `del` version to fix MacOS race condition ([#2847](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2873))
- [Chore] Update deprecated url methods (url.parse(), url.format()) ([#1561](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/1561))
- [Build] Fixed "Last Access Time" not being set by `scanCopy` on Windows ([#2964](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2964))
- [Vis Builder] Add global data persistence for vis builder #2896 ([#2896](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2896))
- Update `leaflet-vega` and fix its usage ([#3005](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3005))
Expand Down
14 changes: 4 additions & 10 deletions examples/url_generators_examples/public/url_generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
* under the License.
*/

import url from 'url';
import { UrlGeneratorState, UrlGeneratorsDefinition } from '../../../src/plugins/share/public';

/**
Expand All @@ -53,16 +52,11 @@ export const createHelloPageLinkGenerator = (
createUrl: async (state) => {
const startServices = await getStartServices();
const appBasePath = startServices.appBasePath;
const parsedUrl = url.parse(window.location.href);

return url.format({
protocol: parsedUrl.protocol,
host: parsedUrl.host,
pathname: `${appBasePath}/hello`,
query: {
...state,
},
});
const url = new URL(`${appBasePath}/hello`, window.location.origin);
url.search = new URLSearchParams({ ...state }).toString();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


return url.toString();
},
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
* under the License.
*/

import Url from 'url';
import Https from 'https';
import Axios, { AxiosResponse } from 'axios';

Expand Down Expand Up @@ -93,7 +92,7 @@ export class OsdClientRequester {
constructor(private readonly log: ToolingLog, options: Options) {
this.url = options.url;
this.httpsAgent =
Url.parse(options.url).protocol === 'https:'
new URL(options.url).protocol === 'https:'
? new Https.Agent({
ca: options.certificateAuthorities,
})
Expand All @@ -105,11 +104,11 @@ export class OsdClientRequester {
}

public resolveUrl(relativeUrl: string = '/') {
return Url.resolve(this.pickUrl(), relativeUrl);
return new URL(relativeUrl, this.pickUrl()).toString();
}

async request<T>(options: ReqOptions): Promise<AxiosResponse<T>> {
const url = Url.resolve(this.pickUrl(), options.path);
const url = new URL(options.path, this.pickUrl()).toString();
const description = options.description || `${options.method} ${url}`;
let attempt = 0;
const maxAttempts = options.retries ?? DEFAULT_MAX_ATTEMPTS;
Expand Down
8 changes: 5 additions & 3 deletions packages/osd-opensearch-archiver/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
*************************************************************/

import Path from 'path';
import Url from 'url';
import readline from 'readline';

import { RunWithCommands, createFlagError } from '@osd/dev-utils';
Expand Down Expand Up @@ -72,7 +71,7 @@ export function runCli() {
throw createFlagError('--opensearch-url must be a string');
}
if (!opensearchUrl && config) {
opensearchUrl = Url.format(config.get('servers.opensearch'));
opensearchUrl = config.get('servers.opensearch').fullURL.toString().slice(0, -1);
}
if (!opensearchUrl) {
throw createFlagError('--opensearch-url or --config must be defined');
Expand All @@ -83,7 +82,10 @@ export function runCli() {
throw createFlagError('--opensearch-dashboards-url must be a string');
}
if (!opensearchDashboardsUrl && config) {
opensearchDashboardsUrl = Url.format(config.get('servers.opensearchDashboards'));
opensearchDashboardsUrl = config
.get('servers.opensearchDashboards')
.fullURL.toString()
.slice(0, -1) as string;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we still need the as string?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. The config.get('servers.opensearchDashboards') will return an any object. The TS complier won't know its type. So I add the as string.

}
if (!opensearchDashboardsUrl) {
throw createFlagError('---url or --config must be defined');
Expand Down
10 changes: 4 additions & 6 deletions packages/osd-test/src/failed_tests_reporter/github_api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@
* under the License.
*/

import Url from 'url';

import Axios, { AxiosRequestConfig, AxiosInstance } from 'axios';
import parseLinkHeader from 'parse-link-header';
import { ToolingLog, isAxiosResponseError, isAxiosRequestError } from '@osd/dev-utils';
Expand Down Expand Up @@ -98,7 +96,7 @@ export class GithubApi {
nextRequest: {
safeForDryRun: true,
method: 'GET',
url: Url.resolve(BASE_URL, 'issues'),
url: new URL('issues', BASE_URL).toString(),
params: {
state: 'all',
per_page: '100',
Expand Down Expand Up @@ -158,7 +156,7 @@ export class GithubApi {
await this.request(
{
method: 'PATCH',
url: Url.resolve(BASE_URL, `issues/${encodeURIComponent(issueNumber)}`),
url: new URL(`issues/${encodeURIComponent(issueNumber)}`, BASE_URL).toString(),
data: {
state: 'open', // Reopen issue if it was closed.
body: newBody,
Expand All @@ -172,7 +170,7 @@ export class GithubApi {
await this.request(
{
method: 'POST',
url: Url.resolve(BASE_URL, `issues/${encodeURIComponent(issueNumber)}/comments`),
url: new URL(`issues/${encodeURIComponent(issueNumber)}/comments`, BASE_URL).toString(),
data: {
body: commentBody,
},
Expand All @@ -185,7 +183,7 @@ export class GithubApi {
const resp = await this.request<GithubIssueMini>(
{
method: 'POST',
url: Url.resolve(BASE_URL, 'issues'),
url: new URL('issues', BASE_URL).toString(),
data: {
title,
body,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,10 @@ export class Config {
if (typeof v === 'function') {
return v;
}

if (v instanceof URL) {
return new URL(v.toString());
AMoo-Miki marked this conversation as resolved.
Show resolved Hide resolved
}
});
}

Expand All @@ -134,6 +138,10 @@ export class Config {
if (typeof v === 'function') {
return v;
}

if (v instanceof URL) {
return new URL(v.toString());
}
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ const urlPartsSchema = () =>
pathname: Joi.string().regex(/^\//, 'start with a /'),
hash: Joi.string().regex(/^\//, 'start with a /'),
certificateAuthorities: Joi.array().items(Joi.binary()).optional(),
fullURL: Joi.object().type(URL),
})
.default();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
* under the License.
*/

import Url from 'url';
import execa from 'execa';
import * as Rx from 'rxjs';
import { filter, take, map } from 'rxjs/operators';
Expand All @@ -54,11 +53,7 @@ export class DockerServersService {
this.servers = Object.entries(configs).map(([name, config]) => ({
...config,
name,
url: Url.format({
protocol: 'http:',
hostname: 'localhost',
port: config.port,
}),
url: new URL(`http://localhost:${config.port}`).toString(),
}));

this.lifecycle.beforeTests.add(async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
*/

import { resolve } from 'path';
import { format } from 'url';
import { get, toPath } from 'lodash';
import { Cluster } from '@osd/opensearch';
import { CI_PARALLEL_PROCESS_PREFIX } from '../ci_parallel_process_prefix';
Expand Down Expand Up @@ -135,10 +134,10 @@ export function createLegacyOpenSearchTestCluster(options = {}) {
}

getUrl() {
const parts = opensearchTestConfig.getUrlParts();
parts.port = port;
const url = new URL(opensearchTestConfig.getUrlParts().fullURL);
url.port = port;

return format(parts);
return url.toString().slice(0, -1);
}
})();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
* under the License.
*/

import url, { format as formatUrl } from 'url';
import pkg from '../../../../package.json';
import { adminTestUser } from '../osd';

Expand All @@ -42,7 +41,7 @@ export const opensearchTestConfig = new (class OpenSearchTestConfig {
}

getUrl() {
return formatUrl(this.getUrlParts());
return this.getUrlParts().fullURL.toString().slice(0, -1);
}

getBuildFrom() {
Expand All @@ -56,30 +55,37 @@ export const opensearchTestConfig = new (class OpenSearchTestConfig {
getUrlParts() {
// Allow setting one complete TEST_OPENSEARCH_URL for opensearch like https://opensearch:[email protected]:9200
if (process.env.TEST_OPENSEARCH_URL) {
const testOpenSearchUrl = url.parse(process.env.TEST_OPENSEARCH_URL);
const testOpenSearchUrl = new URL('', process.env.TEST_OPENSEARCH_URL);
joshuarrrr marked this conversation as resolved.
Show resolved Hide resolved
testOpenSearchUrl.pathname = '';
return {
// have to remove the ":" off protocol
protocol: testOpenSearchUrl.protocol.slice(0, -1),
hostname: testOpenSearchUrl.hostname,
port: parseInt(testOpenSearchUrl.port, 10),
username: testOpenSearchUrl.auth.split(':')[0],
password: testOpenSearchUrl.auth.split(':')[1],
auth: testOpenSearchUrl.auth,
username: testOpenSearchUrl.username,
password: testOpenSearchUrl.password,
auth: `${testOpenSearchUrl.username}:${testOpenSearchUrl.password}`,
fullURL: testOpenSearchUrl,
};
}

const username = process.env.TEST_OPENSEARCH_USERNAME || adminTestUser.username;
const password = process.env.TEST_OPENSEARCH_PASSWORD || adminTestUser.password;
const protocol = process.env.TEST_OPENSEARCH_PROTOCOL || 'http';
const hostname = process.env.TEST_OPENSEARCH_HOSTNAME || 'localhost';
const port = parseInt(process.env.TEST_OPENSEARCH_PORT, 10) || 9220;
const fullURL = new URL('', `${protocol}://${username}:${password}@${hostname}:${port}`);

return {
// Allow setting any individual component(s) of the URL,
// or use default values (username and password from ../osd/users.js)
protocol: process.env.TEST_OPENSEARCH_PROTOCOL || 'http',
hostname: process.env.TEST_OPENSEARCH_HOSTNAME || 'localhost',
port: parseInt(process.env.TEST_OPENSEARCH_PORT, 10) || 9220,
protocol,
hostname,
port,
auth: `${username}:${password}`,
username: username,
password: password,
fullURL,
};
}
})();
31 changes: 21 additions & 10 deletions packages/osd-test/src/osd/osd_test_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
* under the License.
*/

import url from 'url';
import { opensearchDashboardsTestUser } from './users';

interface UrlParts {
Expand All @@ -38,6 +37,7 @@ interface UrlParts {
auth?: string;
username?: string;
password?: string;
fullURL: URL;
}

export const osdTestConfig = new (class OsdTestConfig {
Expand All @@ -48,32 +48,43 @@ export const osdTestConfig = new (class OsdTestConfig {
getUrlParts(): UrlParts {
// allow setting one complete TEST_OPENSEARCH_DASHBOARDS_URL for opensearch like https://opensearch:[email protected]:9200
if (process.env.TEST_OPENSEARCH_DASHBOARDS_URL) {
const testOpenSearchDashboardsUrl = url.parse(process.env.TEST_OPENSEARCH_DASHBOARDS_URL);
const testOpenSearchDashboardsUrl = new URL('', process.env.TEST_OPENSEARCH_DASHBOARDS_URL);
testOpenSearchDashboardsUrl.pathname = '';
return {
protocol: testOpenSearchDashboardsUrl.protocol?.slice(0, -1),
hostname: testOpenSearchDashboardsUrl.hostname ?? undefined,
port: testOpenSearchDashboardsUrl.port
? parseInt(testOpenSearchDashboardsUrl.port, 10)
: undefined,
auth: testOpenSearchDashboardsUrl.auth ?? undefined,
username: testOpenSearchDashboardsUrl.auth?.split(':')[0],
password: testOpenSearchDashboardsUrl.auth?.split(':')[1],
auth:
testOpenSearchDashboardsUrl.username && testOpenSearchDashboardsUrl.password
? `${testOpenSearchDashboardsUrl.username}:${testOpenSearchDashboardsUrl.password}`
: undefined,
username: testOpenSearchDashboardsUrl.username ?? undefined,
password: testOpenSearchDashboardsUrl.password ?? undefined,
fullURL: testOpenSearchDashboardsUrl,
};
}

const username =
process.env.TEST_OPENSEARCH_DASHBOARDS_USERNAME || opensearchDashboardsTestUser.username;
const password =
process.env.TEST_OPENSEARCH_DASHBOARDS_PASSWORD || opensearchDashboardsTestUser.password;
const protocol = process.env.TEST_OPENSEARCH_DASHBOARDS_PROTOCOL || 'http';
const hostname = process.env.TEST_OPENSEARCH_DASHBOARDS_HOSTNAME || 'localhost';
const port = process.env.TEST_OPENSEARCH_DASHBOARDS_PORT
? parseInt(process.env.TEST_OPENSEARCH_DASHBOARDS_PORT, 10)
: 5620;
const fullURL = new URL(`${protocol}://${username}:${password}@${hostname}:${port}`);

return {
protocol: process.env.TEST_OPENSEARCH_DASHBOARDS_PROTOCOL || 'http',
hostname: process.env.TEST_OPENSEARCH_DASHBOARDS_HOSTNAME || 'localhost',
port: process.env.TEST_OPENSEARCH_DASHBOARDS_PORT
? parseInt(process.env.TEST_OPENSEARCH_DASHBOARDS_PORT, 10)
: 5620,
protocol,
hostname,
port,
auth: `${username}:${password}`,
username,
password,
fullURL,
joshuarrrr marked this conversation as resolved.
Show resolved Hide resolved
};
}
})();
3 changes: 1 addition & 2 deletions src/cli/serve/serve.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import { set as lodashSet } from '@elastic/safer-lodash-set';
import _ from 'lodash';
import { statSync } from 'fs';
import { resolve } from 'path';
import url from 'url';

import { getConfigPath } from '@osd/utils';
import { IS_OPENSEARCH_DASHBOARDS_DISTRIBUTABLE } from '../../legacy/utils';
Expand Down Expand Up @@ -110,7 +109,7 @@ function applyConfigOverrides(rawConfig, opts, extraCliOptions) {
const opensearchHosts = (
(customOpenSearchHosts.length > 0 && customOpenSearchHosts) || ['https://localhost:9200']
).map((hostUrl) => {
const parsedUrl = url.parse(hostUrl);
const parsedUrl = new URL('', hostUrl);
if (parsedUrl.hostname !== 'localhost') {
throw new Error(
`Hostname "${parsedUrl.hostname}" can't be used with --ssl. Must be "localhost" to work with certificates.`
Expand Down
11 changes: 7 additions & 4 deletions src/cli_plugin/install/download.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@
* under the License.
*/

import { parse } from 'url';

import { UnsupportedProtocolError } from '../lib/errors';
import { downloadHttpFile } from './downloaders/http';
import { downloadLocalFile } from './downloaders/file';
Expand All @@ -56,14 +54,19 @@ export function _checkFilePathDeprecation(sourceUrl, logger) {
}

export function _downloadSingle(settings, logger, sourceUrl) {
const urlInfo = parse(sourceUrl);
let urlInfo;
try {
urlInfo = new URL('', sourceUrl);
} catch (e) {
return Promise.reject(new UnsupportedProtocolError());
}
let downloadPromise;

if (/^file/.test(urlInfo.protocol)) {
_checkFilePathDeprecation(sourceUrl, logger);
downloadPromise = downloadLocalFile(
logger,
_getFilePath(urlInfo.path, sourceUrl),
_getFilePath(urlInfo.pathname, sourceUrl),
settings.tempArchiveFile
);
} else if (/^https?/.test(urlInfo.protocol)) {
Expand Down
Loading