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.

Sign up for GitHub

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

Jump to bottom

RTC-13399 only resolve proxy if passed #1709

Merged
merged 5 commits into from
Feb 20, 2023
Merged

RTC-13399 only resolve proxy if passed #1709

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
610e090
only add proxy arguments if passed to SDA
swasunb Feb 16, 2023
4b8e794
tests
swasunb Feb 17, 2023
2e8c91d
Merge branch 'main' into swasunb/RTC-13399-only-resolve-proxy-if-passed
swasunb Feb 17, 2023
bd8c429
Merge remote-tracking branch 'upstream/main' into swasunb/RTC-13399-o…
swasunb Feb 17, 2023
173f58e
Merge branch 'swasunb/RTC-13399-only-resolve-proxy-if-passed' of http…
swasunb Feb 17, 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
49 changes: 48 additions & 1 deletion spec/c9ShellHandler.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,17 @@ describe('C9 shell handler', () => {
});

it('args', async () => {
mockGetCommandLineArgs.mockImplementation((_, name) =>
name === '--proxy-server=' ? '--proxy-server=512' : null,
);
webContentsMocked.session.resolveProxy = jest
.fn()
.mockImplementation(() => Promise.resolve(''));
mockGetGuid.mockReturnValue('just-another-guid');

const { loadC9Shell } = require('../src/app/c9-shell-handler');

await loadC9Shell(webContentsMocked as any);

expect(mockSpawn).toBeCalledWith(
expect.stringContaining('c9shell.exe'),
['--symphonyHost', 'just-another-guid', '--proxyServer', ''],
Expand All @@ -124,6 +131,9 @@ describe('C9 shell handler', () => {
});

it('args, when resolveProxy returns DIRECT', async () => {
mockGetCommandLineArgs.mockImplementation((_, name) =>
name === '--proxy-server=' ? '--proxy-server=512' : null,
);
webContentsMocked.session.resolveProxy = jest
.fn()
.mockImplementation(() => Promise.resolve('DIRECT'));
Expand All @@ -140,6 +150,9 @@ describe('C9 shell handler', () => {
});

it('args, when resolveProxy returns string starting with PROXY ', async () => {
mockGetCommandLineArgs.mockImplementation((_, name) =>
name === '--proxy-server=' ? '--proxy-server=512' : null,
);
webContentsMocked.session.resolveProxy = jest
.fn()
.mockImplementation(() => Promise.resolve('PROXY 52.207.140.132:8443'));
Expand All @@ -160,6 +173,40 @@ describe('C9 shell handler', () => {
);
});

it('args, when --proxy-server= is not passed as argument, do not pass resolved proxy to cloud9', async () => {
mockGetCommandLineArgs.mockReturnValue('');
webContentsMocked.session.resolveProxy = jest
.fn()
.mockImplementation(() => Promise.resolve('DIRECT'));
mockGetGuid.mockReturnValue('just-another-guid');
const { loadC9Shell } = require('../src/app/c9-shell-handler');

await loadC9Shell(webContentsMocked as any);

expect(mockSpawn).toBeCalledWith(
expect.stringContaining('c9shell.exe'),
['--symphonyHost', 'just-another-guid'],
{ stdio: 'pipe' },
);
});

it('args, when --proxy-pac-url= is not passed as argument, do not pass resolved proxy to cloud9', async () => {
mockGetCommandLineArgs.mockReturnValue('');
webContentsMocked.session.resolveProxy = jest
.fn()
.mockImplementation(() => Promise.resolve('DIRECT'));
mockGetGuid.mockReturnValue('just-another-guid');
const { loadC9Shell } = require('../src/app/c9-shell-handler');

await loadC9Shell(webContentsMocked as any);

expect(mockSpawn).toBeCalledWith(
expect.stringContaining('c9shell.exe'),
['--symphonyHost', 'just-another-guid'],
{ stdio: 'pipe' },
);
});

it('non-windows', async () => {
mockIsWindows = false;
const { loadC9Shell } = require('../src/app/c9-shell-handler');
Expand Down
37 changes: 33 additions & 4 deletions src/app/c9-shell-handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,15 +99,40 @@ class C9ShellHandler {
}
}

/**
* only return resolved proxy from Electron, if proxy-server or proxy-pac-url
* was passed as arguments
*/
private async _getCloud9ProxyArgs() {
const hasProxyServerArgs = getCommandLineArgs(
process.argv,
'--proxy-server=',
false,
);
const hasProxyPacFileArgs = getCommandLineArgs(
process.argv,
'--proxy-pac-url=',
false,
);

if (hasProxyPacFileArgs || hasProxyServerArgs) {
const proxy = (
await this._sender.session.resolveProxy(this._sender.getURL() ?? '')
)
.split(';')[0]
.replace('PROXY ', '');

return ['--proxyServer', proxy];
}
return [];
}

/**
* Launches the correct c9shell process
*/
private async _launchC9Shell(): Promise<ChildProcess | undefined> {
this._curStatus = undefined;
const uniquePipeName = getGuid();
const proxy = (
await this._sender.session.resolveProxy(this._sender.getURL() ?? '')
).replace('PROXY ', '');

const c9ShellPath = isDevEnv
? path.join(
Expand All @@ -126,7 +151,11 @@ class C9ShellHandler {
: [];

customC9ShellArgList.push(
...['--symphonyHost', uniquePipeName, '--proxyServer', proxy],
...[
'--symphonyHost',
uniquePipeName,
...(await this._getCloud9ProxyArgs()),
],
);

logger.info(
Expand Down