apocas / docker-modem

Docker Remote API network stack driver.
Apache License 2.0
227 stars 109 forks source link

Sending non-empty body for some requests breaks docker > 1.12 #81

Closed rmg closed 7 years ago

rmg commented 7 years ago

Example of error:

starting container with non-empty request body was deprecated since v1.10 and removed in v1.12

This is caused by modem.dial sending {} and setting Transfer-Encoding: chunked on POST methods when no body is specified.

Here's a quick hack to work around it, illustrating the problem bits:

diff --git a/lib/modem.js b/lib/modem.js
index a096ad9..0b1f2db 100644
--- a/lib/modem.js
+++ b/lib/modem.js
@@ -140,14 +140,18 @@ Modem.prototype.dial = function(options, callback) {
     optionsf.headers['Content-Type'] = 'application/tar';
   } else if (opts && options.method === 'POST') {
     data = JSON.stringify(opts._body || opts);
-    optionsf.headers['Content-Type'] = 'application/json';
+    if (data !== '{}' && data !== '""') {
+      optionsf.headers['Content-Type'] = 'application/json';
+    } else {
+      data = undefined;
+    }
   }

   if (typeof data === "string") {
     optionsf.headers['Content-Length'] = Buffer.byteLength(data);
   } else if (Buffer.isBuffer(data) === true) {
     optionsf.headers['Content-Length'] = data.length;
-  } else if (optionsf.method === 'POST' || optionsf.method === 'PUT') {
+  } else if (optionsf.method === 'PUT' || options.hijack || options.openStdin) {
     // Has body
     optionsf.headers['Transfer-Encoding'] = 'chunked';
   }
apocas commented 7 years ago

I'm yet to publish https://github.com/apocas/docker-modem/pull/78

Not sure if not using chunked with POST might be a problem for this other issue :/

rmg commented 7 years ago

Ah, hadn't seen that one yet. I'll check my app against master and see if that's enough.

rmg commented 7 years ago

Oh, I was already testing against master, nevermind.

apocas commented 7 years ago
   if (data !== '{}' && data !== '""') {
      optionsf.headers['Content-Type'] = 'application/json';
    } else {
      data = undefined;
    }

Something is weird. With this fix, run exec test fails with 500 and "{"message":"EOF"}" This endpoint likes {} instead :/

Using what's in master right now, I can start a container without options.

rmg commented 7 years ago

Strange, my app uses exec and has no issues with it. Are you specifying any options to force it open? My app uses AttachStdin: true on the container.exec() and stdin: true, hijack: true on the exec.start.

rmg commented 7 years ago

This is probably relevant:

$ docker version
Client:
 Version:      17.06.0-ce-rc1
 API version:  1.30
 Go version:   go1.8.1
 Git commit:   7f8486a
 Built:        Wed May 31 02:56:01 2017
 OS/Arch:      darwin/amd64

Server:
 Version:      17.06.0-ce-rc1
 API version:  1.30 (minimum version 1.12)
 Go version:   go1.8.1
 Git commit:   7f8486a
 Built:        Wed May 31 03:00:14 2017
 OS/Arch:      linux/amd64
 Experimental: true
apocas commented 7 years ago

Ah I'm not yet on the rc. Will bump my version and test again.

apocas commented 7 years ago

How do I easily install that version? (latest stable is: 17.05.0-ce -- API version: 1.29)

There was a dev repo, but now with all this constant renaming/rebrand/product mess I can't find it in docs.

rmg commented 7 years ago

I'm using docker-for-mac and there's a toggle for switching between "edge" and "stable" for updates.

apocas commented 7 years ago

Same setup as you. Version: 17.06.0-ce-rc1 API version: 1.30

It's not the version but the options you are using. If you do not attach to stdin it fails.

https://github.com/apocas/dockerode/blob/master/test/container.js#L547 this test without stdin fails.

philjones88 commented 7 years ago

I'm hitting this too but with:

Version 17.06.0-rc1-ce-mac13 (18169)
Channel: edge
2425473dc2
{ Error: (HTTP code 400) unexpected - starting container with non-empty request body was deprecated since v1.10 and removed in v1.12
    at /Users/phil/Documents/Projects/bot/node_modules/docker-modem/lib/modem.js:239:17
    at getCause (/Users/phil/Documents/Projects/bot/node_modules/docker-modem/lib/modem.js:269:7)
    at Modem.buildPayload (/Users/phil/Documents/Projects/bot/node_modules/docker-modem/lib/modem.js:238:5)
    at IncomingMessage.<anonymous> (/Users/phil/Documents/Projects/bot/node_modules/docker-modem/lib/modem.js:214:14)
    at emitNone (events.js:110:20)
    at IncomingMessage.emit (events.js:207:7)
    at endReadableNT (_stream_readable.js:1045:12)
    at _combinedTickCallback (internal/process/next_tick.js:102:11)
    at process._tickCallback (internal/process/next_tick.js:161:9)
  reason: undefined,
  statusCode: 400,
  json: { message: 'starting container with non-empty request body was deprecated since v1.10 and removed in v1.12' } }
const container = this.dockerode.getContainer(myName);

container.attach({
    stream: true,
    stdout: true,
    stdin: true,
    stderr: true
}, (attachError, stream) => {
    container.modem.demuxStream(stream, process.stdout, process.stderr);
});

container.start({ }, (error, result) => {
    if (error) {
        console.error('Docker => error', error);
        reject(error);
    }

    console.log('Docker => finished');

    container.wait((waitError, waitResult) => {
        if (waitError) {
            console.error('Docker =>wait error', waitError);
        }

        console.log('Docker => wait finished');
    });
});
georgehdd commented 7 years ago

Hey, do you know when a release with this fix will be published to npm?

apocas commented 7 years ago

Later today.

Pedro Dias

On Jun 14, 2017 6:07 AM, "georgehdd" notifications@github.com wrote:

Hey, do you know when a release with this fix will be published to npm?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/apocas/docker-modem/issues/81#issuecomment-308320827, or mute the thread https://github.com/notifications/unsubscribe-auth/AAkdpfAoaAIT141aw2TcIlnKic784X6vks5sD2p8gaJpZM4NvcR5 .

apocas commented 7 years ago

Published.