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.

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

arrays within query parameters #91

Open
twifty opened this issue May 17, 2018 · 10 comments
Open

arrays within query parameters #91

twifty opened this issue May 17, 2018 · 10 comments

Comments

@twifty
Copy link

twifty commented May 17, 2018

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.

@ghost
Copy link

ghost commented Jun 15, 2018

@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

@apocas
Copy link
Owner

apocas commented Sep 23, 2019

Interesting, will check this one.

@apocas apocas added the bug label Sep 23, 2019
@apocas
Copy link
Owner

apocas commented Oct 5, 2019

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.

@ghost
Copy link

ghost commented Oct 6, 2019

@apocas the test case works because of key !== 't'

clone[key] = (opts[key] && typeof opts[key] === 'object' && key !== 't') ?

@apocas
Copy link
Owner

apocas commented Oct 8, 2019

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
Copy link
Owner

apocas commented Oct 8, 2019

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?

@ghost
Copy link

ghost commented Oct 8, 2019

@apocas looks like it was introduced with this commit: 8565c3f. 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.

@apocas
Copy link
Owner

apocas commented Oct 10, 2019

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)

@roddc
Copy link

roddc commented Aug 31, 2022

@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

@lstocchi
Copy link

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!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants