Skip to content

Commit c9f91e7

Browse files
authored
Fix coping and moving files around using the file tree (#568)
* Implement write/read buffers in electron fill This makes cutting and copy files from the file tree work. * Implement fs.createReadStream This is used by the file tree to copy files. * Allow passing proxies back from client to server This makes things like piping streams possible. * Synchronously bind to proxy events This eliminates any chance whatsoever of missing events due to binding too late. * Make it possible to bind some events on demand * Add some protocol documentation
1 parent 30b8565 commit c9f91e7

21 files changed

+541
-273
lines changed

packages/ide/src/fill/electron.ts

+12-2
Original file line numberDiff line numberDiff line change
@@ -171,8 +171,10 @@ const newCreateElement = <K extends keyof HTMLElementTagNameMap>(tagName: K): HT
171171
document.createElement = newCreateElement;
172172

173173
class Clipboard {
174-
public has(): boolean {
175-
return false;
174+
private readonly buffers = new Map<string, Buffer>();
175+
176+
public has(format: string): boolean {
177+
return this.buffers.has(format);
176178
}
177179

178180
public readFindText(): string {
@@ -190,6 +192,14 @@ class Clipboard {
190192
public readText(): Promise<string> {
191193
return clipboard.readText();
192194
}
195+
196+
public writeBuffer(format: string, buffer: Buffer): void {
197+
this.buffers.set(format, buffer);
198+
}
199+
200+
public readBuffer(format: string): Buffer | undefined {
201+
return this.buffers.get(format);
202+
}
193203
}
194204

195205
class Shell {

packages/protocol/README.md

+47
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
# Protocol
2+
3+
This module provides a way for the browser to run Node modules like `fs`, `net`,
4+
etc.
5+
6+
## Internals
7+
8+
### Server-side proxies
9+
The server-side proxies are regular classes that call native Node functions. The
10+
only thing special about them is that they must return promises and they must
11+
return serializable values.
12+
13+
The only exception to the promise rule are event-related methods such as
14+
`onEvent` and `onDone` (these are synchronous). The server will simply
15+
immediately bind and push all events it can to the client. It doesn't wait for
16+
the client to start listening. This prevents issues with the server not
17+
receiving the client's request to start listening in time.
18+
19+
However, there is a way to specify events that should not bind immediately and
20+
should wait for the client to request it, because some events (like `data` on a
21+
stream) cannot be bound immediately (because doing so changes how the stream
22+
behaves).
23+
24+
### Client-side proxies
25+
Client-side proxies are `Proxy` instances. They simply make remote calls for any
26+
method you call on it. The only exception is for events. Each client proxy has a
27+
local emitter which it uses in place of a remote call (this allows the call to
28+
be completed synchronously on the client). Then when an event is received from
29+
the server, it gets emitted on that local emitter.
30+
31+
When an event is listened to, the proxy also notifies the server so it can start
32+
listening in case it isn't already (see the `data` example above). This only
33+
works for events that only fire after they are bound.
34+
35+
### Client-side fills
36+
The client-side fills implement the Node API and make calls to the server-side
37+
proxies using the client-side proxies.
38+
39+
When a proxy returns a proxy (for example `fs.createWriteStream`), that proxy is
40+
a promise (since communicating with the server is asynchronous). We have to
41+
return the fill from `fs.createWriteStream` synchronously, so that means the
42+
fill has to contain a proxy promise. To eliminate the need for calling `then`
43+
and to keep the code looking clean every time you use the proxy, the proxy is
44+
itself wrapped in another proxy which just calls the method after a `then`. This
45+
works since all the methods return promises (aside from the event methods, but
46+
those are not used by the fills directly—they are only used internally to
47+
forward events to the fill if it is an event emitter).

packages/protocol/src/browser/client.ts

+8-4
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { promisify } from "util";
44
import { Emitter } from "@coder/events";
55
import { logger, field } from "@coder/logger";
66
import { ReadWriteConnection, InitData, SharedProcessData } from "../common/connection";
7-
import { Module, ServerProxy } from "../common/proxy";
7+
import { ClientServerProxy, Module, ServerProxy } from "../common/proxy";
88
import { argumentToProto, protoToArgument, moduleToProto, protoToModule, protoToOperatingSystem } from "../common/util";
99
import { Argument, Ping, ServerMessage, ClientMessage, Method, Event, Callback } from "../proto";
1010
import { FsModule, ChildProcessModule, NetModule, NodePtyModule, SpdlogModule, TrashModule } from "./modules";
@@ -224,7 +224,11 @@ export class Client {
224224
field("method", method),
225225
]);
226226

227-
proxyMessage.setArgsList(args.map((a) => argumentToProto(a, storeCallback)));
227+
proxyMessage.setArgsList(args.map((a) => argumentToProto<ClientServerProxy>(
228+
a,
229+
storeCallback,
230+
(p) => p.proxyId,
231+
)));
228232

229233
const clientMessage = new ClientMessage();
230234
clientMessage.setMethod(message);
@@ -429,7 +433,7 @@ export class Client {
429433
/**
430434
* Return a proxy that makes remote calls.
431435
*/
432-
private createProxy<T>(proxyId: number | Module, promise: Promise<any> = Promise.resolve()): T {
436+
private createProxy<T extends ClientServerProxy>(proxyId: number | Module, promise: Promise<any> = Promise.resolve()): T {
433437
logger.trace(() => [
434438
"creating proxy",
435439
field("proxyId", proxyId),
@@ -449,7 +453,7 @@ export class Client {
449453
cb(event.event, ...event.args);
450454
});
451455
},
452-
}, {
456+
} as ClientServerProxy, {
453457
get: (target: any, name: string): any => {
454458
// When resolving a promise with a proxy, it will check for "then".
455459
if (name === "then") {

packages/protocol/src/browser/modules/child_process.ts

+21-6
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,22 @@ import * as cp from "child_process";
22
import * as net from "net";
33
import * as stream from "stream";
44
import { callbackify } from "util";
5-
import { ClientProxy } from "../../common/proxy";
6-
import { ChildProcessModuleProxy, ChildProcessProxy, ChildProcessProxies } from "../../node/modules/child_process";
7-
import { Readable, Writable } from "./stream";
5+
import { ClientProxy, ClientServerProxy } from "../../common/proxy";
6+
import { ChildProcessModuleProxy, ChildProcessProxy } from "../../node/modules/child_process";
7+
import { ClientWritableProxy, ClientReadableProxy, Readable, Writable } from "./stream";
88

99
// tslint:disable completed-docs
1010

11-
export class ChildProcess extends ClientProxy<ChildProcessProxy> implements cp.ChildProcess {
11+
export interface ClientChildProcessProxy extends ChildProcessProxy, ClientServerProxy<cp.ChildProcess> {}
12+
13+
export interface ClientChildProcessProxies {
14+
childProcess: ClientChildProcessProxy;
15+
stdin?: ClientWritableProxy | null;
16+
stdout?: ClientReadableProxy | null;
17+
stderr?: ClientReadableProxy | null;
18+
}
19+
20+
export class ChildProcess extends ClientProxy<ClientChildProcessProxy> implements cp.ChildProcess {
1221
public readonly stdin: stream.Writable;
1322
public readonly stdout: stream.Readable;
1423
public readonly stderr: stream.Readable;
@@ -18,7 +27,7 @@ export class ChildProcess extends ClientProxy<ChildProcessProxy> implements cp.C
1827
private _killed: boolean = false;
1928
private _pid = -1;
2029

21-
public constructor(proxyPromises: Promise<ChildProcessProxies>) {
30+
public constructor(proxyPromises: Promise<ClientChildProcessProxies>) {
2231
super(proxyPromises.then((p) => p.childProcess));
2332
this.stdin = new Writable(proxyPromises.then((p) => p.stdin!));
2433
this.stdout = new Readable(proxyPromises.then((p) => p.stdout!));
@@ -99,8 +108,14 @@ export class ChildProcess extends ClientProxy<ChildProcessProxy> implements cp.C
99108
}
100109
}
101110

111+
interface ClientChildProcessModuleProxy extends ChildProcessModuleProxy, ClientServerProxy {
112+
exec(command: string, options?: { encoding?: string | null } & cp.ExecOptions | null, callback?: ((error: cp.ExecException | null, stdin: string | Buffer, stdout: string | Buffer) => void)): Promise<ClientChildProcessProxies>;
113+
fork(modulePath: string, args?: string[], options?: cp.ForkOptions): Promise<ClientChildProcessProxies>;
114+
spawn(command: string, args?: string[], options?: cp.SpawnOptions): Promise<ClientChildProcessProxies>;
115+
}
116+
102117
export class ChildProcessModule {
103-
public constructor(private readonly proxy: ChildProcessModuleProxy) {}
118+
public constructor(private readonly proxy: ClientChildProcessModuleProxy) {}
104119

105120
public exec = (
106121
command: string,

packages/protocol/src/browser/modules/fs.ts

+37-8
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
11
import * as fs from "fs";
22
import { callbackify } from "util";
3-
import { ClientProxy, Batch } from "../../common/proxy";
3+
import { Batch, ClientProxy, ClientServerProxy } from "../../common/proxy";
44
import { IEncodingOptions, IEncodingOptionsCallback } from "../../common/util";
5-
import { FsModuleProxy, Stats as IStats, WatcherProxy, WriteStreamProxy } from "../../node/modules/fs";
6-
import { Writable } from "./stream";
5+
import { FsModuleProxy, ReadStreamProxy, Stats as IStats, WatcherProxy, WriteStreamProxy } from "../../node/modules/fs";
6+
import { Readable, Writable } from "./stream";
77

8-
// tslint:disable no-any
9-
// tslint:disable completed-docs
8+
// tslint:disable completed-docs no-any
109

1110
class StatBatch extends Batch<IStats, { path: fs.PathLike }> {
1211
public constructor(private readonly proxy: FsModuleProxy) {
@@ -38,7 +37,9 @@ class ReaddirBatch extends Batch<Buffer[] | fs.Dirent[] | string[], { path: fs.P
3837
}
3938
}
4039

41-
class Watcher extends ClientProxy<WatcherProxy> implements fs.FSWatcher {
40+
interface ClientWatcherProxy extends WatcherProxy, ClientServerProxy<fs.FSWatcher> {}
41+
42+
class Watcher extends ClientProxy<ClientWatcherProxy> implements fs.FSWatcher {
4243
public close(): void {
4344
this.catch(this.proxy.close());
4445
}
@@ -48,7 +49,25 @@ class Watcher extends ClientProxy<WatcherProxy> implements fs.FSWatcher {
4849
}
4950
}
5051

51-
class WriteStream extends Writable<WriteStreamProxy> implements fs.WriteStream {
52+
interface ClientReadStreamProxy extends ReadStreamProxy, ClientServerProxy<fs.ReadStream> {}
53+
54+
class ReadStream extends Readable<ClientReadStreamProxy> implements fs.ReadStream {
55+
public get bytesRead(): number {
56+
throw new Error("not implemented");
57+
}
58+
59+
public get path(): string | Buffer {
60+
throw new Error("not implemented");
61+
}
62+
63+
public close(): void {
64+
this.catch(this.proxy.close());
65+
}
66+
}
67+
68+
interface ClientWriteStreamProxy extends WriteStreamProxy, ClientServerProxy<fs.WriteStream> {}
69+
70+
class WriteStream extends Writable<ClientWriteStreamProxy> implements fs.WriteStream {
5271
public get bytesWritten(): number {
5372
throw new Error("not implemented");
5473
}
@@ -62,12 +81,18 @@ class WriteStream extends Writable<WriteStreamProxy> implements fs.WriteStream {
6281
}
6382
}
6483

84+
interface ClientFsModuleProxy extends FsModuleProxy, ClientServerProxy {
85+
createReadStream(path: fs.PathLike, options?: any): Promise<ClientReadStreamProxy>;
86+
createWriteStream(path: fs.PathLike, options?: any): Promise<ClientWriteStreamProxy>;
87+
watch(filename: fs.PathLike, options?: IEncodingOptions): Promise<ClientWatcherProxy>;
88+
}
89+
6590
export class FsModule {
6691
private readonly statBatch: StatBatch;
6792
private readonly lstatBatch: LstatBatch;
6893
private readonly readdirBatch: ReaddirBatch;
6994

70-
public constructor(private readonly proxy: FsModuleProxy) {
95+
public constructor(private readonly proxy: ClientFsModuleProxy) {
7196
this.statBatch = new StatBatch(this.proxy);
7297
this.lstatBatch = new LstatBatch(this.proxy);
7398
this.readdirBatch = new ReaddirBatch(this.proxy);
@@ -110,6 +135,10 @@ export class FsModule {
110135
);
111136
}
112137

138+
public createReadStream = (path: fs.PathLike, options?: any): fs.ReadStream => {
139+
return new ReadStream(this.proxy.createReadStream(path, options));
140+
}
141+
113142
public createWriteStream = (path: fs.PathLike, options?: any): fs.WriteStream => {
114143
return new WriteStream(this.proxy.createWriteStream(path, options));
115144
}

packages/protocol/src/browser/modules/net.ts

+18-6
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,18 @@
11
import * as net from "net";
22
import { callbackify } from "util";
3-
import { ClientProxy } from "../../common/proxy";
3+
import { ClientProxy, ClientServerProxy } from "../../common/proxy";
44
import { NetModuleProxy, NetServerProxy, NetSocketProxy } from "../../node/modules/net";
55
import { Duplex } from "./stream";
66

77
// tslint:disable completed-docs
88

9-
export class Socket extends Duplex<NetSocketProxy> implements net.Socket {
9+
interface ClientNetSocketProxy extends NetSocketProxy, ClientServerProxy<net.Socket> {}
10+
11+
export class Socket extends Duplex<ClientNetSocketProxy> implements net.Socket {
1012
private _connecting: boolean = false;
1113
private _destroyed: boolean = false;
1214

13-
public constructor(proxyPromise: Promise<NetSocketProxy> | NetSocketProxy, connecting?: boolean) {
15+
public constructor(proxyPromise: Promise<ClientNetSocketProxy> | ClientNetSocketProxy, connecting?: boolean) {
1416
super(proxyPromise);
1517
if (connecting) {
1618
this._connecting = connecting;
@@ -126,12 +128,16 @@ export class Socket extends Duplex<NetSocketProxy> implements net.Socket {
126128
}
127129
}
128130

129-
export class Server extends ClientProxy<NetServerProxy> implements net.Server {
131+
interface ClientNetServerProxy extends NetServerProxy, ClientServerProxy<net.Server> {
132+
onConnection(cb: (proxy: ClientNetSocketProxy) => void): Promise<void>;
133+
}
134+
135+
export class Server extends ClientProxy<ClientNetServerProxy> implements net.Server {
130136
private socketId = 0;
131137
private readonly sockets = new Map<number, net.Socket>();
132138
private _listening: boolean = false;
133139

134-
public constructor(proxyPromise: Promise<NetServerProxy> | NetServerProxy) {
140+
public constructor(proxyPromise: Promise<ClientNetServerProxy> | ClientNetServerProxy) {
135141
super(proxyPromise);
136142

137143
this.catch(this.proxy.onConnection((socketProxy) => {
@@ -208,11 +214,17 @@ export class Server extends ClientProxy<NetServerProxy> implements net.Server {
208214

209215
type NodeNet = typeof net;
210216

217+
interface ClientNetModuleProxy extends NetModuleProxy, ClientServerProxy {
218+
createSocket(options?: net.SocketConstructorOpts): Promise<ClientNetSocketProxy>;
219+
createConnection(target: string | number | net.NetConnectOpts, host?: string): Promise<ClientNetSocketProxy>;
220+
createServer(options?: { allowHalfOpen?: boolean, pauseOnConnect?: boolean }): Promise<ClientNetServerProxy>;
221+
}
222+
211223
export class NetModule implements NodeNet {
212224
public readonly Socket: typeof net.Socket;
213225
public readonly Server: typeof net.Server;
214226

215-
public constructor(private readonly proxy: NetModuleProxy) {
227+
public constructor(private readonly proxy: ClientNetModuleProxy) {
216228
// @ts-ignore this is because Socket is missing things from the Stream
217229
// namespace but I'm unsure how best to provide them (finished,
218230
// finished.__promisify__, pipeline, and some others) or if it even matters.

packages/protocol/src/browser/modules/node-pty.ts

+14-6
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,17 @@
11
import * as pty from "node-pty";
2-
import { ClientProxy } from "../../common/proxy";
2+
import { ClientProxy, ClientServerProxy } from "../../common/proxy";
33
import { NodePtyModuleProxy, NodePtyProcessProxy } from "../../node/modules/node-pty";
44

55
// tslint:disable completed-docs
66

7-
export class NodePtyProcess extends ClientProxy<NodePtyProcessProxy> implements pty.IPty {
7+
interface ClientNodePtyProcessProxy extends NodePtyProcessProxy, ClientServerProxy {}
8+
9+
export class NodePtyProcess extends ClientProxy<ClientNodePtyProcessProxy> implements pty.IPty {
810
private _pid = -1;
911
private _process = "";
1012

1113
public constructor(
12-
private readonly moduleProxy: NodePtyModuleProxy,
14+
private readonly moduleProxy: ClientNodePtyModuleProxy,
1315
private readonly file: string,
1416
private readonly args: string[] | string,
1517
private readonly options: pty.IPtyForkOptions,
@@ -18,10 +20,12 @@ export class NodePtyProcess extends ClientProxy<NodePtyProcessProxy> implements
1820
this.on("process", (process) => this._process = process);
1921
}
2022

21-
protected initialize(proxyPromise: Promise<NodePtyProcessProxy>): void {
22-
super.initialize(proxyPromise);
23+
protected initialize(proxyPromise: Promise<ClientNodePtyProcessProxy>): ClientNodePtyProcessProxy {
24+
const proxy = super.initialize(proxyPromise);
2325
this.catch(this.proxy.getPid().then((p) => this._pid = p));
2426
this.catch(this.proxy.getProcess().then((p) => this._process = p));
27+
28+
return proxy;
2529
}
2630

2731
public get pid(): number {
@@ -53,8 +57,12 @@ export class NodePtyProcess extends ClientProxy<NodePtyProcessProxy> implements
5357

5458
type NodePty = typeof pty;
5559

60+
interface ClientNodePtyModuleProxy extends NodePtyModuleProxy, ClientServerProxy {
61+
spawn(file: string, args: string[] | string, options: pty.IPtyForkOptions): Promise<ClientNodePtyProcessProxy>;
62+
}
63+
5664
export class NodePtyModule implements NodePty {
57-
public constructor(private readonly proxy: NodePtyModuleProxy) {}
65+
public constructor(private readonly proxy: ClientNodePtyModuleProxy) {}
5866

5967
public spawn = (file: string, args: string[] | string, options: pty.IPtyForkOptions): pty.IPty => {
6068
return new NodePtyProcess(this.proxy, file, args, options);

packages/protocol/src/browser/modules/spdlog.ts

+10-4
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
import * as spdlog from "spdlog";
2-
import { ClientProxy } from "../../common/proxy";
2+
import { ClientProxy, ClientServerProxy } from "../../common/proxy";
33
import { RotatingLoggerProxy, SpdlogModuleProxy } from "../../node/modules/spdlog";
44

55
// tslint:disable completed-docs
66

7-
class RotatingLogger extends ClientProxy<RotatingLoggerProxy> implements spdlog.RotatingLogger {
7+
interface ClientRotatingLoggerProxy extends RotatingLoggerProxy, ClientServerProxy {}
8+
9+
class RotatingLogger extends ClientProxy<ClientRotatingLoggerProxy> implements spdlog.RotatingLogger {
810
public constructor(
9-
private readonly moduleProxy: SpdlogModuleProxy,
11+
private readonly moduleProxy: ClientSpdlogModuleProxy,
1012
private readonly name: string,
1113
private readonly filename: string,
1214
private readonly filesize: number,
@@ -31,10 +33,14 @@ class RotatingLogger extends ClientProxy<RotatingLoggerProxy> implements spdlog.
3133
}
3234
}
3335

36+
interface ClientSpdlogModuleProxy extends SpdlogModuleProxy, ClientServerProxy {
37+
createLogger(name: string, filePath: string, fileSize: number, fileCount: number): Promise<ClientRotatingLoggerProxy>;
38+
}
39+
3440
export class SpdlogModule {
3541
public readonly RotatingLogger: typeof spdlog.RotatingLogger;
3642

37-
public constructor(private readonly proxy: SpdlogModuleProxy) {
43+
public constructor(private readonly proxy: ClientSpdlogModuleProxy) {
3844
this.RotatingLogger = class extends RotatingLogger {
3945
public constructor(name: string, filename: string, filesize: number, filecount: number) {
4046
super(proxy, name, filename, filesize, filecount);

0 commit comments

Comments
 (0)