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

load all assets from localhost to set origin properly #387

Merged
merged 7 commits into from
Jan 15, 2022
Merged

load all assets from localhost to set origin properly #387

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6db4b65
load all assets from localhost to set origin properly
mbektas Jan 7, 2022
63a6a60
refactoring
mbektas Jan 7, 2022
2f03f27
use system assigned port
mbektas Jan 8, 2022
7f1a723
split handlers into separate functions
mbektas Jan 8, 2022
a813a9b
make sure that only the assets in appAssetsDir are served
mbektas Jan 8, 2022
3d4d393
simplify cookie handling
mbektas Jan 13, 2022
7f5396b
security measures
mbektas Jan 13, 2022
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
2 changes: 1 addition & 1 deletion scripts/copyassets.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ function copyAssests() {

// Copy html into build directory
const htmlPath = path.join('browser', 'index.html');
fs.copySync(path.join(srcDir, htmlPath), path.join(dest, htmlPath));
fs.copySync(path.join(srcDir, htmlPath), path.join(dest, '../index.html'));

const envInfoPath = path.join('main', 'env_info.py');
fs.copySync(path.join(srcDir, envInfoPath), path.join(dest, envInfoPath));
Expand Down
2 changes: 1 addition & 1 deletion src/browser/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"appVersion": "3.2.5-2",
"pageUrl": "lab/"
}</script>
<script type="text/javascript" src="../../browser.bundle.js"></script>
<script type="text/javascript" src="browser.bundle.js"></script>
</head>

<body>
Expand Down
42 changes: 41 additions & 1 deletion src/main/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,46 @@ import log from 'electron-log';
import * as yargs from 'yargs';
import * as path from 'path';
import * as fs from 'fs';
import { randomBytes } from 'crypto';
import { AddressInfo, createServer } from 'net';

import { appConfig } from './utils';

async function getFreePort(): Promise<number> {
return new Promise<number>((resolve) => {

const getPort = () => {
const server = createServer((socket) => {
socket.write('Echo server\r\n');
socket.pipe(socket);
});

server.on('error', function (e) {
getPort();
});
server.on('listening', function (e: any) {
const port = (server.address() as AddressInfo).port;
server.close();

resolve(port);
});

server.listen(0, '127.0.0.1');
};

getPort();
});
};

async function setAppConfig(): Promise<void> {
return new Promise<void>((resolve) => {
getFreePort().then((port) => {
appConfig.jlabPort = port;
appConfig.token = randomBytes(24).toString('hex');
resolve();
});
});
}

// handle opening file or directory with command-line arguments
if (process.argv.length > 1) {
Expand Down Expand Up @@ -126,7 +166,7 @@ app.on('open-file', (event: Electron.Event, _path: string) => {
* ready.
*/
app.on('ready', () => {
handOverArguments()
Promise.all([setAppConfig(), handOverArguments()])
.then( () => {
let serviceManager = new Bottle();
let autostarts: string[] = [];
Expand Down
16 changes: 5 additions & 11 deletions src/main/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,11 @@ import {
ArrayExt
} from '@lumino/algorithm';

import {
randomBytes
} from 'crypto';

import log from 'electron-log';

import * as fs from 'fs';
import { IPythonEnvironment } from './tokens';
import { appConfig } from './utils';

export
class JupyterServer {
Expand Down Expand Up @@ -61,15 +58,15 @@ class JupyterServer {

this._startServer = new Promise<JupyterServer.IInfo>((resolve, reject) => {
let urlRegExp = /https?:\/\/localhost:\d+\/\S*/g;
let baseRegExp = /https?:\/\/localhost:\d+\//g;
let serverVersionPattern = /Jupyter Server (?<version>.*) is running at/g;
const home = process.env.JLAB_DESKTOP_HOME || app.getPath('home');
const pythonPath = this._info.environment.path;
if (!fs.existsSync(pythonPath)) {
dialog.showMessageBox({message: `Environment not found at: ${pythonPath}`, type: 'error' });
reject();
}
this._info.token = randomBytes(24).toString('hex');
this._info.url = `http://localhost:${appConfig.jlabPort}`;
this._info.token = appConfig.token;

// note: traitlets<5.0 require fully specified arguments to
// be followed by equals sign without a space; this can be
Expand All @@ -79,6 +76,7 @@ class JupyterServer {
'--no-browser',
// do not use any config file
'--JupyterApp.config_file_name=""',
`--ServerApp.port=${appConfig.jlabPort}`,
// use our token rather than any pre-configured password
'--ServerApp.password=""',
'--ServerApp.allow_origin="*"',
Expand All @@ -89,7 +87,7 @@ class JupyterServer {
env: {
...process.env,
PATH: this._registry.getAdditionalPathIncludesForPythonPath(this._info.environment.path),
JUPYTER_TOKEN: this._info.token,
JUPYTER_TOKEN: appConfig.token,
JUPYTER_CONFIG_DIR: process.env.JLAB_DESKTOP_CONFIG_DIR || app.getPath('userData')
}
});
Expand Down Expand Up @@ -121,10 +119,6 @@ class JupyterServer {
this._info.version = versionMatch.groups.version;
}
if (urlMatch) {
let url = urlMatch[0].toString();

this._info.url = (url.match(baseRegExp))[0];

started = true;
this._cleanupListeners();
return resolve(this._info);
Expand Down
112 changes: 104 additions & 8 deletions src/main/sessions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,16 @@ import {

import { IRegistry } from './registry';

import { appConfig } from './utils';

import {
EventEmitter
} from 'events';

import * as url from 'url';
import * as fs from 'fs';
import * as path from 'path';
import { URL } from "url";
import * as path from 'path';
import { request as httpRequest, IncomingMessage } from 'http';

export
interface ISessions extends EventEmitter {
Expand Down Expand Up @@ -420,12 +422,106 @@ class JupyterLabSession {
this._window.show();
});

this._window.loadURL(url.format({
pathname: path.join(__dirname, '../browser/index.html'),
protocol: 'file:',
slashes: true,
search: encodeURIComponent(JSON.stringify(this.info))
}));
const DESKTOP_APP_ASSETS_PATH = 'desktop-app-assets';

this._window.loadURL(`http://localhost:${appConfig.jlabPort}/${DESKTOP_APP_ASSETS_PATH}/index.html?${encodeURIComponent(JSON.stringify(this.info))}`);

const cookies: Map<string, string> = new Map();

const parseCookieName = (cookie: string): string | undefined => {
const parts = cookie.split(';');
if (parts.length < 1) {
return undefined;
}
const firstPart = parts[0];
const eqLoc = firstPart.indexOf('=');
if (eqLoc === -1) {
return undefined;
}
return firstPart.substring(0, eqLoc).trim();
};

const jlabBaseUrl = `http://localhost:${appConfig.jlabPort}/`;
const desktopAppAssetsPrefix = `${jlabBaseUrl}${DESKTOP_APP_ASSETS_PATH}`;
const appAssetsDir = path.normalize(path.join(__dirname, '../../'));

const handleDesktopAppAssetRequest = (req: Electron.ProtocolRequest, callback: (response: (Buffer) | (Electron.ProtocolResponse)) => void) => {
let assetPath = req.url.substring(desktopAppAssetsPrefix.length + 1);
const qMark = assetPath.indexOf('?');
if (qMark !== -1) {
assetPath = assetPath.substring(0, qMark);
}
const assetFilePath = path.normalize(path.join(appAssetsDir, assetPath));

if (assetFilePath.indexOf(appAssetsDir) === 0) {
const assetContent = fs.readFileSync(assetFilePath);
callback(assetContent);
}
};

const handleRemoteAssetRequest = (req: Electron.ProtocolRequest, callback: (response: (Buffer) | (Electron.ProtocolResponse)) => void) => {
const headers: any = {...req.headers, 'Referer': req.referrer, 'Authorization': `token ${appConfig.token}` };
const request = httpRequest(req.url, {headers: headers, method: req.method });
request.on('response', (res: IncomingMessage) => {
if (req.url.startsWith(jlabBaseUrl) && ('set-cookie' in res.headers)) {
for (let cookie of res.headers['set-cookie']) {
const cookieName = parseCookieName(cookie);
if (cookieName) {
cookies.set(cookieName, cookie);
}
}
}

const chunks: Buffer[] = [];

res.on('data', (chunk: any) => {
chunks.push(Buffer.from(chunk));
})

res.on('end', async () => {
const file = Buffer.concat(chunks);
callback({
statusCode: res.statusCode,
headers: res.headers,
method: res.method,
url: res.url,
data: file,
});
})
})

if (req.uploadData) {
req.uploadData.forEach(part => {
if (part.bytes) {
request.write(part.bytes);
} else if (part.file) {
request.write(fs.readFileSync(part.file));
}
})
}

request.end();
};

this._window.webContents.session.protocol.interceptBufferProtocol("http", (req, callback) => {
if (req.url.startsWith(desktopAppAssetsPrefix)) {
handleDesktopAppAssetRequest(req, callback);
} else {
Copy link
Member

Choose a reason for hiding this comment

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

Could you elaborate on what is happening in the else clause? My current understanding is that the if () clase is for hosting static assets and else assumes that if it is not a static asset, then this is a server request and modifies cookies and handles file upload, but it is not clear to me why specific parts are needed.

Should we split up the handler code into separate methods to better signal the intent and have easier time in the future?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, you are right. since interceptBufferProtocol doesn't provide a default callback handler, we are having to make the requests to JupyterLab server in the else section. During my experiments, I found out that cookies were not properly set for some of the requests (websocket handshake?). So, to be on the safe side, I am setting them with both this._window.webContents.session.cookies.set(cookieObject); and requestHeaders['Cookie'] = Array.from(cookies.values()).join('; '); for every request. I am not sure at the moment how we can optimize this.

handleRemoteAssetRequest(req, callback);
}
});

const filter = {
urls: [`ws://localhost:${appConfig.jlabPort}/*`]
};

this._window.webContents.session.webRequest.onBeforeSendHeaders(filter, (details, callback) => {
const requestHeaders: Record<string, string> = {...details.requestHeaders};
if (cookies.size > 0) {
requestHeaders['Cookie'] = Array.from(cookies.values()).join('; ');
}
callback({cancel: false, requestHeaders})
});

this._window.on('focus', () => {
this._sessionManager.setFocusedSession(this);
Expand Down
12 changes: 12 additions & 0 deletions src/main/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,18 @@ import * as path from 'path';
import * as fs from 'fs';
import log from 'electron-log';

export
interface IAppConfiguration {
jlabPort: number;
token: string;
};

export
const appConfig: IAppConfiguration = {
jlabPort: 8888,
token: 'jlab-token'
};
Comment on lines +39 to +42
Copy link
Member

Choose a reason for hiding this comment

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

I anticipate that a hard-coded global will cause issues once we will go for full front/back separation. But I am happy to address it myself later.

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, we can definitely improve this when we support remote backends etc. by the way, these are just default values and token is generated on the fly before server launch.


export
interface ISaveOptions {
id: string;
Expand Down