Skip to content

Multiple entries of the same query parameter are encoded using ; (semicolon) #842

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

Closed
perlun opened this issue May 31, 2019 · 12 comments
Closed
Assignees

Comments

@perlun
Copy link

perlun commented May 31, 2019

Description of the bug

In commit 1276c03 (PR #794), we fixed #792. However, the fix is still not optimal since query parameters where either part of the key=value part includes semicolon will be unable to be represented in the simulation JSON.

To be honest, my use case don't have a requirement to use semicolons at the moment. However, I see a risk with the current implementation in that it uses a separator which could potentially be a fully valid part of a query parameter for a "reserved" purpose like this. This could potentially be very confusing to our users.

Ideally, the values would be present multiple times in the JSON (i.e. no separator), but if this is considered too complex/time consuming, we should at the very least choose a separator which is more than a single character. ;;; would be an improvement (note, this is just a suggestion; feel free to come up with a better option if you like)

Steps to reproduce the issue

Make a query which includes the same query string multiple times.
Like this: http://localhost/some-api/some-resource?q=access:vod&q=order:latest&q=profile:vd

Observed result

"q": [
    {
        "matcher": "exact",
        "value": "access:vod;order:latest;profile:vd"
    }

Hoverfly error messages seen (If none, say none)

none

If possible, add screenshots to help explain your problem

Expected result

"q": [
    {
        "matcher": "exact",
        "value": "access:vod;;;order:latest;;;profile:vd"
    }

or (preferably):

"q": [
    {
        "matcher": "exact",
        "value": "access:vod"
    },
    {
        "matcher": "exact",
        "value": "order:latest"
    },
    {
        "matcher": "exact",
        "value": "profile:vd"
    }

or even this (I think this would be my personal favorite, in terms of readability of the JSON):

"q": [
    {
        "matcher": "exact",
        "value": [
            "access:vod",
            "order:latest",
            "profile:vd"
        ]
    }

Additional relevant information

  1. Hoverfly version:
+----------+--------------+
| hoverctl | master-3141  |
| hoverfly | v1.0.0-rc.2  |
+----------+--------------+
  1. Anything that might help us to diagnose the problem
@perlun perlun changed the title Multiple entries of the same query parameter are encoded using ; Multiple entries of the same query parameter are encoded using ; (semicolon) May 31, 2019
@tommysitu
Copy link
Member

Hi @perlun, joining multiple values using a symbol for a query is not a permanent solution, we prefer to implement what you suggest as well:

"q": [
    {
        "matcher": "exact",
        "value": [
            "access:vod",
            "order:latest",
            "profile:vd"
        ]
    }

We thought about this before, and the value field is an interface type in the data model, so that it can be an array.

There will be matcher types that are specific to array values, like contains, containsExactly or containsOnly.

@perlun
Copy link
Author

perlun commented May 31, 2019

@tommysitu Sounds great! As noted, it's not a huge problem for me at the moment but it was just something we discovered internally when we figured out how it's currently serialized to the JSON.

Btw, thanks for a great product! 👍

@tommysitu tommysitu added this to the v1.2 milestone Jul 3, 2019
@tommysitu tommysitu removed this from the v1.2 milestone Mar 4, 2020
@ahll
Copy link

ahll commented Dec 22, 2022

Hi, is this fixed?

Or some way working with queries like: q=access:vod&q=order:latest&q=profile:vd](http://localhost/some-api/some-resource?q=access:vod&q=order:latest&q=profile:vd

@kapishmalik
Copy link
Collaborator

kapishmalik commented Dec 23, 2022

@tommysitu I can work on this issue and fix this in above mentioned way. You can assign this issue to me.

@tommysitu
Copy link
Member

@kapishmalik that'll be awesome!

@kapishmalik
Copy link
Collaborator

@tommysitu Work is in progress. Can you mark this as WIP or assign this issue to me?

@kapishmalik
Copy link
Collaborator

@tommysitu Raised PR

@tommysitu
Copy link
Member

Thanks @kapishmalik your PR looks good!

@kapishmalik
Copy link
Collaborator

@tommysitu I feel I need to relook at contains and containsonly matcher. I am confused with existing implementation. I will raise PR to fix those.

@kapishmalik
Copy link
Collaborator

@tommysitu can you check this PR and review implementations and if they make more sense now.

@kapishmalik
Copy link
Collaborator

Thanks @tommysitu with reviews. I believe we are good with this issue now.

@kapishmalik
Copy link
Collaborator

@perlun this is done. You can use and let us know inase of any issues.

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