Query parameters with semicolon ignored

tiboleemans commented 9 months ago

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

Hoverfly version: v0.16.1 (but probably occurs since v0.15.0)
This issue seems related to: Multiple entries of the same query parameter are encoded using ; (semicolon) #842 but is not the same

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 commented 9 months ago

@tommysitu could you please help here?

tommysitu commented 9 months ago

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 commented 9 months ago

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: https://github.com/golang/go/issues/50034, however I can't find anything in the RFC that says semicolon is not allowed in the query param value.

SpectoLabs / hoverfly