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

Proposal: Handle repeated query params in HTTP transport #238

Open
pauln opened this issue Sep 14, 2017 · 3 comments
Open

Proposal: Handle repeated query params in HTTP transport #238

pauln opened this issue Sep 14, 2017 · 3 comments

Comments

@pauln
Copy link
Contributor

pauln commented Sep 14, 2017

At present, repeated fields are expected to take the form of a serialised JSON array, i.e.
?foo=["bar","baz"]

However, it's common to simply repeat the query parameter, either with or without square brackets:
?foo=bar&foo=baz or ?foo[]=bar&foo[]=baz

It would therefore be nice to support this method of encoding repeated fields. The existing query param processing handles this (though it considers the square brackets to be part of the parameter name, so you need to check for both variants in the params map), so it should be pretty straightforward to support.

My main question is whether the existing behaviour must be maintained, and if so, whether we should provide a way to choose which approach to use (perhaps via a command-line flag) or generate code which supports both approaches.

@adamryman
Copy link
Member

We would need to support the old version. My vote would be to try the current version (so that existing implementations are fast) and then try these formats you mentioned.

Can you provide a link to an external spec describing the ?foo[]=bar way of doing query params?

In our integration tests we have two supported ways of doing query params, with your first way commented out as we have not implemented it.

I give this proposal a 👍 and would be happy to see these additional forms supported.

@pauln
Copy link
Contributor Author

pauln commented Sep 14, 2017

The main place I've seen the square brackets used is in PHP, where the square brackets cause it to be parsed as an array (see parse_str() Example #1). If you don't use square brackets, PHP will simply take the last value it encounters for the parameter. I'm given to understand that Ruby also uses square brackets.

@zaquestion
Copy link
Member

zaquestion commented Sep 14, 2017

IIRC this method was not taken on earlier due to complications with nested types. @hasLeland do you remember?

In any case, golang supports this method (without []) out of the box, implementation could be as simple as check for >1 parameter here and dealing with the conversion func logic.

Here truss decides how to convert/decode a parameter, if its a repeated field its going to aim for the json decode functionality. Possibly the runtime check could looks at the length of queryParams and if the type is repeated. Then you would need to convert the []string to []Type and skip over the json logic.

Generally speaking I'd like to keep truss supporting few solid cases over omnibus support for everything. This keeps truss simple and fast. For that reason I'd prefer we leave the PHP (?foo[]=bar) case out of this.

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

3 participants