#2224 - Add parsing for multi-valued query params in non standard format [#hacktoberfest]

[kapishmalik]

Description:

This PR adds parsing for query params in non standard format. It will be able to parse following non-standard multi-valued query parameter formats along with standard one.

?id[0]=1&id[1]=2&id[2]=3
?id=1,2,3
?id=1|2|3
?id=[1,2,3]

References

#2224

Submitter checklist

• Recommended: Join WireMock Slack to get any help in #help-contributing or a project-specific channel like #wiremock-java
• Recommended: If you participate in Hacktoberfest 2023, make sure you're signed up there and in the WireMock form
• The PR request is well described and justified, including the body and the references
• The PR title represents the desired changelog entry
• The repository's code style is followed (see the contributing guide)
• Test coverage that demonstrates that the change works as expected

#hacktoberfest

[Mahoney]

@kapishmalik did you see this comment on the original issue?
#2224 (comment)

[kapishmalik]

@Mahoney yup I saw that comment but if we are adding support for all without config does that harm any sense? IMO, it will be complete overhead of doing that. @tomakehurst what do you want to say on this?

[tomakehurst]

Looking good so far @kapishmalik, a few observations:

• Would be good to see some negative tests. I suspect there are some cases that'll make this code blow up e.g. a decimal point in the index.
• What about matching? The change to Urls should also permit matching query params split this way, but some acceptance test cases showing it would be valuable.
• Concur with @Mahoney's about Expand support for multi-value query parameter formats #2224 (comment)

[tomakehurst]

The main issue with just enabling this behaviour without a config flag is that it'll be a breaking change for anyone who depends on the current behaviour.

I think we could just make it boolean e.g. enableAdvancedQueryParameterParsing.

[kapishmalik]

@tomakehurst getting hold of option enableAdvancedQueryParameterParsing from Diff class will be pain as method com.github.tomakehurst.wiremock.common.Urls#splitQuery(java.net.URI) is called from here as well and IMO, it will not be worth it to change and break this at multiple places. I believe, it will be more beneficial to ask end users to change as they are using this behaviour incorrectly. I am okay to go ahead with any of the approaches whatever you guys suggest.

[tomakehurst]

What if we did the splitting in the Request implementation, rather than in the matching/templating system?

We could extend QueryParameter so that it explicitly modelled these cases. Then if the toggle is enabled we use the enhanced parsing in WireMockHttpServletRequestAdapter and everything that consumes query parameters gets this enhanced model.