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

Query parameters with semicolon ignored #1111

Closed
tiboleemans opened this issue Feb 1, 2024 · 3 comments · Fixed by #1114
Closed

Query parameters with semicolon ignored #1111

tiboleemans opened this issue Feb 1, 2024 · 3 comments · Fixed by #1114

Comments

@tiboleemans
Copy link

Description of the bug

Hoverfly can not handle sub-query parameters with a semicolon in it. It seems that this is ignored in this case. Probably because semicolon is a reserved character. But when we integrate with Here Geolocating we need to implement this in qq query-param.
Example:
13:42:46,697 | DEBUG | [main] | org.springframework.web.client.RestTemplate:127 | HTTP GET http://geocode.search.hereapi.com/v1/geocode?apiKey=someKey&lang=nl-BE&qq=country=BEL;postalCode=1234;city=SomeCity;street=SomeStreet;houseNumber=25%20a

Steps to reproduce the issue

Observed result

Hoverfly error messages seen (If none, say none)

13:42:46,748 | ERROR | [Thread-1] | hoverfly:67 | There was an error when matching error=Could not find a match for request, create or record a valid matcher first!

The following request was made, but was not matched by Hoverfly:

{
    "Path": "/v1/geocode",
    "Method": "GET",
    "Destination": "geocode.search.hereapi.com",
    "Scheme": "http",
    "Query": {
        "apiKey": [
            "someKey"
        ],
        "lang": [
            "nl-BE"
        ]
    },
    "Body": "",
    "FormData": {},
    "Headers": {
        "Accept": [
            "application/json"
        ],
        "Accept-Encoding": [
            "gzip"
        ],
        "Connection": [
            "Keep-Alive"
        ],
        "Content-Length": [
            "0"
        ],
        "User-Agent": [
            "okhttp/4.12.0"
        ]
    }
}

Whilst Hoverfly has the following state:

{}

The matcher which came closest was:

{
    "path": [
        {
            "matcher": "exact",
            "value": "/v1/geocode"
        }
    ],
    "method": [
        {
            "matcher": "exact",
            "value": "GET"
        }
    ],
    "destination": [
        {
            "matcher": "exact",
            "value": "geocode.search.hereapi.com"
        }
    ],
    "body": [
        {
            "matcher": "exact",
            "value": ""
        }
    ],
    "query": {
        "apiKey": [
            {
                "matcher": "exact",
                "value": "someKey"
            }
        ],
        "lang": [
            {
                "matcher": "exact",
                "value": "nl-BE"
            }
        ],
        "qq": [
            {
                "matcher": "exact",
                "value": "country=BEL;postalCode=1234;city=SomeCity;street=SomeStreet;houseNumber=25%20a"
            }
        ]
    }
}

But it did not match on the following fields:

[queries]

Which if hit would have given the following response:

If possible, add screenshots to help explain your problem

Expected result

Find a match for this input.

Additional relevant information

  1. Hoverfly version: v0.16.1 (but probably occurs since v0.15.0)
  2. This issue seems related to: Multiple entries of the same query parameter are encoded using ; (semicolon) #842 but is not the same
  3. Test setup:
    @Test
    void forwardGeocode(final Hoverfly hoverfly) {
        hoverfly.simulate(simulateForwardGeocoder(getRequestParametersWithHoverflyBug(), "/response.json"));
        callHereClientApi...
    }

    private SimulationSource simulateForwardGeocoder(final Map<String, RequestFieldMatcher<?>> queryParameters, final String successResponse) {
        final RequestMatcherBuilder matcherBuilder = service("geocode.search.hereapi.com").get("/v1/geocode");
        queryParameters.forEach(matcherBuilder::queryParam);
        final StubServiceBuilder serviceBuilder = matcherBuilder.willReturn(success(getResourceAsString(successResponse), APPLICATION_JSON_VALUE));
        return dsl(serviceBuilder);
    }

    /**
     * Hoverfly is not able to accept queries with a semi-colon. That is why our request is not matched when adding:
     * Key,value: "qq", newExactMatcher("country=BEL;postalCode=1234;city=SomeCity;street=SomeStreet;houseNumber=25%20a")
     * to the queryParameters. A bug has been reported. If this is fixed this test will fail because the qq parameter has to be added. Normally,
     * you should use {@link #getRequestParametersForHoverflyAfterBugFix()}
     */
    private static Map<String, RequestFieldMatcher<?>> getRequestParametersWithHoverflyBug() {
        return Map.of("apiKey", newExactMatcher("someKey"), "lang", newExactMatcher("nl-BE"));
    }

    private static Map<String, RequestFieldMatcher<?>> getRequestParametersForHoverflyAfterBugFix() {
        return Map.of("apiKey", newExactMatcher("someKey"), "lang", newExactMatcher("nl-BE"), "qq",
                      newExactMatcher("country=BEL;postalCode=1234;city=SomeCity;street=SomeStreet;houseNumber=25%20a"));
    }

@kapishmalik
Copy link
Collaborator

@tommysitu could you please help here?

tommysitu added a commit that referenced this issue Feb 23, 2024
@tommysitu
Copy link
Member

I've pushed a fix now. However it's incompatible with some legacy code for converting query into a string which I need to clean up first.

@tommysitu
Copy link
Member

The reason it didn't work is because req.URL.Query() in go explicitly filter out any compound query params when parsing for some reasons: https://github.com/golang/go/blob/master/src/net/url/url.go#L943 🤷

Actually there is an open issue here on the go project: golang/go#50034, however I can't find anything in the RFC that says semicolon is not allowed in the query param value.

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

Successfully merging a pull request may close this issue.

3 participants