docker-modem icon indicating copy to clipboard operation
docker-modem copied to clipboard

arrays within query parameters

Open twifty opened this issue 7 years ago • 10 comments

The API states that some of the query parameters can be duplicated. for example the 'extrahosts' and 't' parameters of ImageBuild.

I see this library makes use of querystrings, and correctly converts objects to JSON strings. But, it doesn't handle arrays. So, for example, given an object like:

{
   extrahosts: [
     "somehost:162.242.195.82",
     "otherhost:50.31.209.229",
   ]
}

querystrings will covert to 'extrahosts[0]=somehost:162.242.195.82&extrahosts[1]=otherhost:50.31.209.229' (escaped of course).

docker-py uses the requests package which simply repeats the name and is consistent with the API docs: 'extrahosts=somehost:162.242.195.82&extrahosts=otherhost:50.31.209.229' .

While the array notation may work on some systems, it all depends on the binary the daemon uses. It may very well break on some OS's or daemon versions.

twifty avatar May 17 '18 22:05 twifty

@twifty arrays aren't being querystringify'd properly due to buildQuerystring (https://github.com/apocas/docker-modem/blob/master/lib/modem.js#L350). Arrays are being stringify'd before being passed to querystring.

/cc @apocas

ghost avatar Jun 15 '18 10:06 ghost

Interesting, will check this one.

apocas avatar Sep 23 '19 19:09 apocas

I can't replicated this, there's even a test for this.

https://github.com/apocas/docker-modem/blob/master/test/modem_test.js#L109

INPUT:

 var opts = {
      "limit": 12,
      "filters": {
        "label": ["staging", "env=green"]
      },
      "t": ["repo:latest", "repo:1.0.0"]
    };

OUTPUT: limit=12&filters={"label":["staging","env=green"]}&t=repo:latest&t=repo:1.0.0

The array "t" was properly converted.

apocas avatar Oct 05 '19 00:10 apocas

@apocas the test case works because of key !== 't' https://github.com/apocas/docker-modem/blob/01cac7434c1927a5ccd41dbe493ae661b1274815/lib/modem.js#L351

ghost avatar Oct 06 '19 09:10 ghost

Missed that. Can't remember why that was done. I suspect that endpoints didn't behave the same way, regarding input.

Will check if removing the stringify doesn't break anything.

apocas avatar Oct 08 '19 00:10 apocas

Passes: https://github.com/apocas/dockerode/blob/master/test/docker.js#L648 Fails: https://github.com/apocas/dockerode/blob/master/test/docker.js#L658

So we can't just remove the stringify. Add that parameter as an exception?

apocas avatar Oct 08 '19 00:10 apocas

@apocas looks like it was introduced with this commit: https://github.com/apocas/docker-modem/commit/8565c3ffb9158329ae98c0b3ae666f163ed09f51. Regarding my use case, as far as I remember it was related to passing an array of environment variables when creating/starting (not sure) a Docker container using dockerode. It was not working and debug'd down to this part of the code in docker-modem.

ghost avatar Oct 08 '19 06:10 ghost

But we can't just remove this condition because the are endpoints expecting the data "stringifed". Like https://github.com/apocas/dockerode/blob/master/test/docker.js#L658

On the other hand there are parameters that are expected by Docker to be repeated like "t" in imageBuild. https://docs.docker.com/engine/api/v1.40/#operation/ImageBuild "A name and optional tag to apply to the image in the name:tag format. If you omit the tag the default latest value is assumed. You can provide several t parameters."

If "extrahosts" is one of this cases we can add it to the exception list, like "t". (Docker documentation does not detail this enough in this use case)

apocas avatar Oct 10 '19 15:10 apocas

@apocas I tried to get multiple images, I need to pass names as array to query, but it doesn't work. I think you need to check the opts[key] is Array or not

roddc avatar Aug 31 '22 06:08 roddc

Same here, i was working with the getImages function from dockerode and i discovered the query string is not created as expected. My object

{
names: ["name1", "name2"]
}

gets changed to

{
names: "[\"name1\", \"name2\"]"
}

in the cloned object and the querystring created by it it's incorrect. /images/get?names=%5B%22name1%22%2C%22name2%22%5D instead of /images/get?names=name1&names=name2

Happy to help if you want!!

lstocchi avatar Mar 22 '24 17:03 lstocchi