Skip to content

Commit c0d2288

Browse files
author
brizental
committed
Remove usage of the URL object
That is a browser API and fails at Qt/QML envs.
1 parent 0297cb4 commit c0d2288

File tree

10 files changed

+94
-55
lines changed

10 files changed

+94
-55
lines changed

src/config.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ interface Configuration {
1111
// The user visible version string fro the application running Glean.js.
1212
readonly appDisplayVersion?: string,
1313
// The server pings are sent to.
14-
readonly serverEndpoint?: URL
14+
readonly serverEndpoint?: string
1515
}
1616

1717
export default Configuration;

src/glean.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import Configuration from "config";
77
import MetricsDatabase from "metrics/database";
88
import PingsDatabase from "pings/database";
99
import PingUploader from "upload";
10-
import { isUndefined, sanitizeApplicationId } from "utils";
10+
import { isUndefined, sanitizeApplicationId, validateURL } from "utils";
1111
import { CoreMetrics } from "internal_metrics";
1212
import { Lifetime } from "metrics";
1313
import { DatetimeMetric } from "metrics/types/datetime";
@@ -33,7 +33,7 @@ class Glean {
3333
// The application ID (will be sanitized during initialization).
3434
private _applicationId?: string;
3535
// The server pings are sent to.
36-
private _serverEndpoint?: URL;
36+
private _serverEndpoint?: string;
3737
// Whether or not to record metrics.
3838
private _uploadEnabled?: boolean;
3939

@@ -181,8 +181,11 @@ class Glean {
181181
}
182182
Glean.instance._applicationId = sanitizeApplicationId(applicationId);
183183

184-
Glean.instance._serverEndpoint = config
185-
? config.serverEndpoint : new URL(DEFAULT_TELEMETRY_ENDPOINT);
184+
if (config && config.serverEndpoint && !validateURL(config.serverEndpoint)) {
185+
throw new Error(`Unable to initialize Glean, serverEndpoint ${config.serverEndpoint} is an invalid URL.`);
186+
}
187+
Glean.instance._serverEndpoint = (config && config.serverEndpoint)
188+
? config.serverEndpoint : DEFAULT_TELEMETRY_ENDPOINT;
186189

187190
if (uploadEnabled) {
188191
await Glean.onUploadEnabled();
@@ -232,7 +235,7 @@ class Glean {
232235
return Glean.instance._applicationId;
233236
}
234237

235-
static get serverEndpoint(): URL | undefined {
238+
static get serverEndpoint(): string | undefined {
236239
if (!Glean.instance._initialized) {
237240
console.warn("Attempted to access the Glean.serverEndpoint before Glean was initialized.");
238241
}

src/upload/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ class PingUploader implements PingsDatabaseObserver {
155155

156156
const finalPing = this.preparePingForUpload(ping);
157157
const result = await Uploader.post(
158-
new URL(ping.path, Glean.serverEndpoint),
158+
`${Glean.serverEndpoint}${ping.path}`,
159159
finalPing.payload,
160160
finalPing.headers
161161
);

src/upload/uploader/browser.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { UploadResult, UploadResultStatus } from "upload";
66
import { Uploader } from ".";
77

88
class BrowserUploader extends Uploader {
9-
async post(url: URL, body: string, headers: Record<string, string> = {}): Promise<UploadResult> {
9+
async post(url: string, body: string, headers: Record<string, string> = {}): Promise<UploadResult> {
1010
const controller = new AbortController();
1111
const timeout = setTimeout(() => controller.abort(), this.defaultTimeout);
1212

src/upload/uploader/index.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ export abstract class Uploader {
2020
*
2121
* @returns The status code of the response.
2222
*/
23-
abstract post(url: URL, body: string, headers?: Record<string, string>): Promise<UploadResult>;
23+
abstract post(url: string, body: string, headers?: Record<string, string>): Promise<UploadResult>;
2424
}
2525

2626
// Default export for tests sake.
@@ -35,7 +35,7 @@ export abstract class Uploader {
3535
// TODO: remove this comment once Bug 1687516 is resolved.
3636
class MockUploader extends Uploader {
3737
// eslint-disable-next-line @typescript-eslint/no-unused-vars
38-
post(_url: URL, _body: string, _headers?: Record<string, string>): Promise<UploadResult> {
38+
post(_url: string, _body: string, _headers?: Record<string, string>): Promise<UploadResult> {
3939
const result: UploadResult = {
4040
result: UploadResultStatus.Success,
4141
status: 200

src/utils.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,3 +104,15 @@ export function isNumber(v: unknown): v is number {
104104
export function sanitizeApplicationId(applicationId: string): string {
105105
return applicationId.replace(/[^a-z0-9]+/gi, "-").toLowerCase();
106106
}
107+
108+
/**
109+
* Check that a given string is a valid URL.
110+
*
111+
* @param v The string to validate.
112+
*
113+
* @returns Whether or not the given string is a valid url.
114+
*/
115+
export function validateURL(v: string): boolean {
116+
const urlPattern = /^(http|https):\/\/[a-zA-Z0-9._-]+(:\d+){0,1}(\/{0,1})$/i;
117+
return urlPattern.test(v);
118+
}

tests/glean.spec.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,16 @@ describe("Glean", function() {
144144
}
145145
});
146146

147+
it("initialization throws if serverEndpoint is an invalida URL", async function() {
148+
await Glean.testUninitialize();
149+
try {
150+
await Glean.initialize(GLOBAL_APPLICATION_ID, true, { serverEndpoint: "" });
151+
assert.ok(false);
152+
} catch {
153+
assert.ok(true);
154+
}
155+
});
156+
147157
it("disabling upload should disable metrics recording", async function() {
148158
const pings = ["aPing", "twoPing", "threePing"];
149159
const metric = new StringMetricType({

tests/pings/maker.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,10 @@ describe("PingMaker", function() {
3939
assert.ok("telemetry_sdk_build" in clientInfo1);
4040

4141
// Initialize will also initialize core metrics that are part of the client info.
42-
await Glean.initialize("something something", {
42+
await Glean.initialize("something something", true, {
4343
appBuild:"build",
4444
appDisplayVersion: "display version",
45-
serverEndpoint: new URL("localhost:8080")
45+
serverEndpoint: "http://localhost:8080"
4646
});
4747

4848
const clientInfo2 = await PingMaker.buildClientInfoSection(ping);

tests/upload/uploader.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,15 +66,15 @@ describe("Uploader/browser", function () {
6666
for (const [index, status] of [200, 400, 500].entries()) {
6767
stub.onCall(index).returns(Promise.resolve(createResponse(status)));
6868
assert.deepStrictEqual(
69-
await BrowserUploader.post(new URL("htpps://localhost:8080"), ""),
69+
await BrowserUploader.post("https://localhost:8080", ""),
7070
{ status: status, result: UploadResultStatus.Success });
7171
}
7272
});
7373

7474
it("doesn't throw if upload action throws", async function () {
7575
sandbox.stub(global, "fetch").callsFake(() => Promise.reject());
7676
assert.deepStrictEqual(
77-
await BrowserUploader.post(new URL("htpps://localhost:8080"), ""),
77+
await BrowserUploader.post("https://localhost:8080", ""),
7878
{ result: UploadResultStatus.RecoverableFailure }
7979
);
8080
});

tests/utils.spec.ts

Lines changed: 55 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -3,85 +3,99 @@
33
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
44

55
import assert from "assert";
6-
import { isBoolean, isNumber, isObject, isString, isUndefined, sanitizeApplicationId } from "utils";
6+
import * as utils from "utils";
77

88
describe("utils", function() {
99
it("isObject validates correctly", function() {
1010
class RandomClass {}
1111

1212
// Invalid values
13-
assert.strictEqual(isObject(new RandomClass()), false);
14-
assert.strictEqual(isObject(null), false);
15-
assert.strictEqual(isObject(NaN), false);
13+
assert.strictEqual(utils.isObject(new RandomClass()), false);
14+
assert.strictEqual(utils.isObject(null), false);
15+
assert.strictEqual(utils.isObject(NaN), false);
1616

1717
// Valid values
18-
assert.strictEqual(isObject({}), true);
19-
assert.strictEqual(isObject(new Object()), true);
20-
assert.strictEqual(isObject({ 1: "test" }), true);
21-
assert.strictEqual(isObject({ "test": "test" }), true);
18+
assert.strictEqual(utils.isObject({}), true);
19+
assert.strictEqual(utils.isObject(new Object()), true);
20+
assert.strictEqual(utils.isObject({ 1: "test" }), true);
21+
assert.strictEqual(utils.isObject({ "test": "test" }), true);
2222
});
2323

2424
it("isUndefined validates correctly", function() {
2525
const obj: Record<string, unknown> = { "test": "test" };
2626

2727
// Invalid values
28-
assert.strictEqual(isUndefined(obj["test"]), false);
29-
assert.strictEqual(isUndefined("something else"), false);
30-
assert.strictEqual(isUndefined(null), false);
31-
assert.strictEqual(isUndefined(NaN), false);
28+
assert.strictEqual(utils.isUndefined(obj["test"]), false);
29+
assert.strictEqual(utils.isUndefined("something else"), false);
30+
assert.strictEqual(utils.isUndefined(null), false);
31+
assert.strictEqual(utils.isUndefined(NaN), false);
3232

3333
// Valid values
34-
assert.strictEqual(isUndefined(undefined), true);
35-
assert.strictEqual(isUndefined(obj["prop"]), true);
34+
assert.strictEqual(utils.isUndefined(undefined), true);
35+
assert.strictEqual(utils.isUndefined(obj["prop"]), true);
3636
});
3737

3838
it("isString validates correctly", function() {
3939
const obj: Record<string, unknown> = { "test": "test" };
4040

4141
// Invalid values
42-
assert.strictEqual(isString(undefined), false);
43-
assert.strictEqual(isString({}), false);
44-
assert.strictEqual(isString(obj["prop"]), false);
45-
assert.strictEqual(isString(null), false);
46-
assert.strictEqual(isString(NaN), false);
42+
assert.strictEqual(utils.isString(undefined), false);
43+
assert.strictEqual(utils.isString({}), false);
44+
assert.strictEqual(utils.isString(obj["prop"]), false);
45+
assert.strictEqual(utils.isString(null), false);
46+
assert.strictEqual(utils.isString(NaN), false);
4747

4848
// Valid values
49-
assert.strictEqual(isString(""), true);
50-
assert.strictEqual(isString("something else"), true);
51-
assert.strictEqual(isString(obj["test"]), true);
52-
assert.strictEqual(isString(new String("check")), true);
49+
assert.strictEqual(utils.isString(""), true);
50+
assert.strictEqual(utils.isString("something else"), true);
51+
assert.strictEqual(utils.isString(obj["test"]), true);
52+
assert.strictEqual(utils.isString(new String("check")), true);
5353
});
5454

5555
it("isBoolean validates correctly", function() {
5656
// Invalid values
57-
assert.strictEqual(isBoolean(undefined), false);
58-
assert.strictEqual(isBoolean("something else"), false);
59-
assert.strictEqual(isBoolean({}), false);
57+
assert.strictEqual(utils.isBoolean(undefined), false);
58+
assert.strictEqual(utils.isBoolean("something else"), false);
59+
assert.strictEqual(utils.isBoolean({}), false);
6060

6161
// Valid values
62-
assert.strictEqual(isBoolean(true), true);
63-
assert.strictEqual(isBoolean(false), true);
64-
assert.strictEqual(isBoolean(new Boolean(true)), true);
65-
assert.strictEqual(isBoolean(!!"something else"), true);
62+
assert.strictEqual(utils.isBoolean(true), true);
63+
assert.strictEqual(utils.isBoolean(false), true);
64+
assert.strictEqual(utils.isBoolean(new Boolean(true)), true);
65+
assert.strictEqual(utils.isBoolean(!!"something else"), true);
6666
});
6767

6868
it("isNumber validates correctly", function() {
6969
// Invalid values
70-
assert.strictEqual(isNumber(undefined), false);
71-
assert.strictEqual(isNumber("10"), false);
72-
assert.strictEqual(isNumber({}), false);
73-
assert.strictEqual(isNumber(NaN), false);
70+
assert.strictEqual(utils.isNumber(undefined), false);
71+
assert.strictEqual(utils.isNumber("10"), false);
72+
assert.strictEqual(utils.isNumber({}), false);
73+
assert.strictEqual(utils.isNumber(NaN), false);
7474

7575
// Valid values
76-
assert.strictEqual(isNumber(10), true);
77-
assert.strictEqual(isNumber(-10), true);
78-
assert.strictEqual(isNumber(new Number(10)), true);
76+
assert.strictEqual(utils.isNumber(10), true);
77+
assert.strictEqual(utils.isNumber(-10), true);
78+
assert.strictEqual(utils.isNumber(new Number(10)), true);
7979
});
8080

8181
it("sanitizeApplicationId works correctly", function() {
82-
assert.strictEqual(sanitizeApplicationId("org.mozilla.test-app"), "org-mozilla-test-app");
83-
assert.strictEqual(sanitizeApplicationId("org.mozilla..test---app"), "org-mozilla-test-app");
84-
assert.strictEqual(sanitizeApplicationId("org-mozilla-test-app"), "org-mozilla-test-app");
85-
assert.strictEqual(sanitizeApplicationId("org.mozilla.Test.App"), "org-mozilla-test-app");
82+
assert.strictEqual(utils.sanitizeApplicationId("org.mozilla.test-app"), "org-mozilla-test-app");
83+
assert.strictEqual(utils.sanitizeApplicationId("org.mozilla..test---app"), "org-mozilla-test-app");
84+
assert.strictEqual(utils.sanitizeApplicationId("org-mozilla-test-app"), "org-mozilla-test-app");
85+
assert.strictEqual(utils.sanitizeApplicationId("org.mozilla.Test.App"), "org-mozilla-test-app");
86+
});
87+
88+
it("validateURL works correctly", function() {
89+
// Invalid values
90+
assert.strictEqual(utils.validateURL(""), false);
91+
assert.strictEqual(utils.validateURL("crealy not a url"), false);
92+
assert.strictEqual(utils.validateURL("glean://wrong.protocol"), false);
93+
assert.strictEqual(utils.validateURL("http://"), false);
94+
95+
// Valid values
96+
assert.strictEqual(utils.validateURL("http://incoming.telemetry.mozilla.org"), true);
97+
assert.strictEqual(utils.validateURL("http://localhost/"), true);
98+
assert.strictEqual(utils.validateURL("https://incoming.telemetry.mozilla.org"), true);
99+
assert.strictEqual(utils.validateURL("https://localhost:3000/"), true);
86100
});
87101
});

0 commit comments

Comments
 (0)