Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
81 changes: 81 additions & 0 deletions src/__tests__/LDClient-useNetModule-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import * as LDClient from '../index';
import * as packageJson from '../../package.json';

import { TestHttpHandlers, TestHttpServer, withCloseable } from 'launchdarkly-js-test-helpers';

describe('LDClient', () => {
const envName = 'UNKNOWN_ENVIRONMENT_ID';
const user = { key: 'user' };

it('should exist', () => {
expect(LDClient).toBeDefined();
});

it('should report correct version', () => {
expect(LDClient.version).toEqual(packageJson.version);
});

describe('initialization', () => {
it('should initialize successfully', async () => {
await withCloseable(TestHttpServer.start, async server => {
const data = { flag: { value: 3 } };
server.byDefault(TestHttpHandlers.respondJson(data));

const client = LDClient.initializeInMain(envName, user, {
baseUrl: server.url,
sendEvents: false,
useNetModule: true,
});
await withCloseable(client, async () => {
await client.waitForInitialization();

expect(client.variation('flag')).toEqual(3);
});
});
});

it('sends correct User-Agent in request', async () => {
await withCloseable(TestHttpServer.start, async server => {
const data = { flag: { value: 3 } };
server.byDefault(TestHttpHandlers.respondJson(data));

const client = LDClient.initializeInMain(envName, user, {
baseUrl: server.url,
sendEvents: false,
useNetModule: true,
});
await withCloseable(client, async () => {
await client.waitForInitialization();

expect(server.requests.length()).toEqual(1);
const req = await server.nextRequest();
expect(req.headers['x-launchdarkly-user-agent']).toMatch(/^ElectronClient\/1\./);
});
});
});
});

describe('track()', () => {
it('should not warn when tracking an arbitrary custom event', async () => {
const logger = {
debug: jest.fn(),
info: jest.fn(),
warn: jest.fn(),
error: jest.fn(),
};
const client = LDClient.initializeInMain(envName, user, {
bootstrap: {},
sendEvents: false,
logger: logger,
useNetModule: true,
});
await withCloseable(client, async () => {
await client.waitForInitialization();

client.track('whatever');
expect(logger.warn).not.toHaveBeenCalled();
expect(logger.error).not.toHaveBeenCalled();
});
});
});
});
1 change: 1 addition & 0 deletions src/__tests__/rendererIntegrationTests/testAppMain.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const ldOptions = {
logger: ldElectron.createConsoleLogger('debug'),
streaming: args.streaming,
useReport: true,
useNetModule: args.useNetModule === true,
};

const testConfig = {
Expand Down
3 changes: 2 additions & 1 deletion src/electronPlatform.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ function makeElectronPlatform(options) {

const ret = {};

ret.httpRequest = (method, url, headers, body) => newHttpRequest(method, url, headers, body, tlsParams);
ret.httpRequest = (method, url, headers, body) =>
newHttpRequest(method, url, headers, body, tlsParams, options && options.useNetModule);

ret.httpAllowsPost = () => true;

Expand Down
38 changes: 29 additions & 9 deletions src/httpRequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,20 @@ const http = require('http');
const https = require('https');
const url = require('url');

function newHttpRequest(method, requestUrl, headers, body, tlsParams) {
let electronNet;
try {
electronNet = require('electron').net;
} catch (_) {
// Not in Electron or outside main process
}

function newHttpRequest(method, requestUrl, headers, body, tlsParams, useNetModule) {
const urlParams = url.parse(requestUrl);
const isHttps = urlParams.protocol === 'https:';

const requestParams = Object.assign({}, isHttps ? tlsParams : {}, urlParams, {
method: method,
headers: headers,
body: body,
});

let request;
const p = new Promise((resolve, reject) => {
request = (isHttps ? https : http).request(requestParams, res => {
const onResponse = res => {
let resBody = '';
res.on('data', chunk => {
resBody += chunk;
Expand All @@ -29,12 +30,31 @@ function newHttpRequest(method, requestUrl, headers, body, tlsParams) {
body: resBody,
});
});
});
};

if (useNetModule && electronNet) {
request = electronNet.request({ method, url: requestUrl });

for (const [key, value] of Object.entries(headers)) {
request.setHeader(key, value);
}

request.on('response', onResponse);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

TLS parameters silently ignored when using net module

Medium Severity

When useNetModule is true, the tlsParams argument is accepted but never applied — the electronNet.request() call doesn't incorporate any TLS configuration. A user who sets both useNetModule: true and tlsParams (e.g. a custom ca for self-signed certificates, as shown in LDClient-tls-test.js) will have their TLS configuration silently dropped, potentially causing connection failures or unintended security behavior with no indication of what went wrong.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit de42f02. Configure here.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think if we introduce this toggle, then we also need to add to the docs that using native net will delegate tls concerns to chromium (I think). This may not work for everyone.

I would also like to have a warning logged if there is a custom tlsParam defined and we are trying to use net.

} else {
const requestParams = Object.assign({}, isHttps ? tlsParams : {}, urlParams, {
method: method,
headers: headers,
body: body,
});

request = (isHttps ? https : http).request(requestParams, onResponse);
}

request.on('error', reject);
if (body) {
request.write(body);
}

request.end();
});

Expand Down
5 changes: 3 additions & 2 deletions tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@
"lib": [
"es6",
"dom"
]
],
"types": []
},
"files": [
"typings.d.ts",
"test-types.ts"
]
}
}
1 change: 1 addition & 0 deletions typings.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ declare module 'launchdarkly-electron-client-sdk' {
* Initialization options for the LaunchDarkly Electron SDK.
*/
export interface LDOptions extends LDOptionsBase {
useNetModule?: boolean;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

See above, I think we need to document the caveats as well as what exactly this option does.

}

/**
Expand Down