Skip to content

Commit a6fac78

Browse files
committed
Fix proxying of Docker exec
This notably broke tools like ddev which use 'exec' as a key part of their setup workflow. This broke because the initial request is an upgrade _and_ includes a body with the exec attach options. Previously we only forwarded the body after Docker accepted the upgrade, which deadlocked: Docker never replied, waiting for the params, and we never sent them, waiting for the upgrade. We now send the params upstream with the initial request.
1 parent 4d72f77 commit a6fac78

File tree

2 files changed

+33
-2
lines changed

2 files changed

+33
-2
lines changed

src/interceptors/docker/docker-proxy.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,6 @@ async function createDockerProxy(proxyPort: number, httpsConfig: { certPath: str
9494
path: req.url,
9595
});
9696

97-
9897
bodyStream.pipe(dockerReq);
9998

10099
return dockerReq;
@@ -282,6 +281,11 @@ async function createDockerProxy(proxyPort: number, httpsConfig: { certPath: str
282281
return;
283282
}
284283

284+
// We have to include initial data in the upstream request. This is especially important for
285+
// commands like /exec/.../start, which send JSON config {detach:...} in the initial request,
286+
// which the docker daemon waits before it will accept the upgrade (or 400s, if it's missing).
287+
req.unshift(head);
288+
285289
const dockerReq = sendToDocker(req);
286290
dockerReq.on('error', (e) => {
287291
console.error('Docker proxy error', e);
@@ -339,7 +343,6 @@ async function createDockerProxy(proxyPort: number, httpsConfig: { certPath: str
339343
// We only write upgrade head data if it's non-empty. For some bizarre reason on
340344
// Windows, writing empty data to a named pipe here kills the connection entirely.
341345
if (dockerHead.length) socket.write(dockerHead);
342-
if (head.length) dockerSocket.write(head);
343346

344347
dockerSocket.on('error', (e) => {
345348
console.error('Docker proxy error', e);

test/interceptors/docker-terminal-interception.spec.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,34 @@ Successfully built <hash>
181181
expect(stderr).to.equal('');
182182
});
183183

184+
it("should support proxying 'docker exec'", async function () {
185+
const { server, httpsConfig } = await testSetup;
186+
187+
const terminalEnvOverrides = getTerminalEnvVars(server.port, httpsConfig, process.env, {});
188+
189+
// Launch a background Node.js container that stays running for 10 seconds
190+
const runResult = await spawnToResult(
191+
'docker', ['run', '-d', '--rm', 'node:14', '-e', 'setTimeout(() => {}, 10000000)'],
192+
{
193+
env: { ...process.env, ...terminalEnvOverrides }
194+
}
195+
);
196+
197+
expect(runResult.exitCode).to.equal(0);
198+
const containerId = runResult.stdout.trim();
199+
200+
const execResult = await spawnToResult(
201+
'docker', ['exec', containerId, 'node', '-e', 'console.log("exec" + "-test-" + "output")'],
202+
{
203+
env: { ...process.env, ...terminalEnvOverrides },
204+
}
205+
);
206+
207+
expect(execResult.exitCode).to.equal(0);
208+
expect(execResult.stderr).to.equal('');
209+
expect(execResult.stdout).to.include('exec-test-output'); // Executed command is run & output returned
210+
});
211+
184212
it("should intercept 'docker-compose up'", async function () {
185213
this.timeout(30000);
186214

0 commit comments

Comments
 (0)