[pact-jvm-provider-gradle] Paths containing url encoded text cannot be verified

[chrisholmes]

When a path in the generated json pact file contains some already url encoded text (e.g. xx%20xx -> xx%2520xx), it is url encoded again after it is read in for use by gradle pactVerify which results in the verification failing.

This does not occur when the same generated json file is run using pact-provider-proxy.

[bethesque]

We need to work out what we're doing with the URL encoding, so it's consistent between implementations. Currently, the Ruby impl leaves the URL encoding up to the user. It would seem that the JVM version does not.

Arguments for and against doing it automatically?

[uglyog]

I think the rule should be that URLs and query parameters should be URL Decoded in the pact file. The JVM will be different depending on the HTTP client and web container used.

[bethesque]

So, are you saying that some JVM HTTP clients will automatically re-URL
encode the string they've been given?

If we're going to change this, I think we should change this as part of
2.0, because it will be a breaking change. At the moment, the ruby mock
service compares the actual query string and path to the expected one, and
it does an exact string match without any processing, which means that it
expects the "expected" values to have already been URL encoded. The URL
encoded values are then written to the pact file. This means that in the
provider, the client should not do anything to the path and query, it
should just replay it.

Serialising the URL decoded value in the pact will mean that in the mock
service we will need to add logic to URL decode the actual path and query
before comparison, and URL encode the path and query before we replay the
request in the provider.

I'm not sure if this is a good thing or not. Trying to think of the pros
and cons. One of the cons is that URL encoding is actually kind of fiddly,
and I'm afraid that different libraries will do it differently. For
example, in the path, a space is converted to %20, but in the query, a
space is converted to a +, so the URL encoding for the path and the query
needs to be done separately, and then the two strings joined together. This
increases the possibility that the implementations will get out of sync,
and that the request that actually gets sent will be differently encoded to
the request that was expected. I guess we could write some examples in the
pact-specification for URL encoding/decoding to try and mitigate this.

On Sat, Oct 25, 2014 at 1:42 PM, Ronald Holshausen <notifications@github.com

wrote:

I think the rule should be that URLs and query parameters should be URL
Decoded in the pact file. The JVM will be different depending on the HTTP
client and web container used.

—
Reply to this email directly or view it on GitHub
#66 (comment).

[uglyog]

All HTTP clients will encode the path and query string. This issue is that the JVM mock server (which uses the Netty scala library) does not URL decode the path before passing it to the handlers.

Here is an example of the problem:

1. The consumer test sets up the pact interactions and mock server and the client code executes a request with the following path: /path/TEST PATH/2014-14-06 23:22:21
2. The HTTP client library URL encodes this path to /path/TEST+PATH/2014-14-06+23%3A22%3A21 and executes the request.
3. The Netty based mock server receives the request with path /path/TEST+PATH/2014-14-06+23%3A22%3A21, validates it and stores it in the pact file. For this to work the consumer test had to URL encode the path in the expectations.
4. On verification the pact file is loaded and a HTTP request is created with path '/path/TEST+PATH/2014-14-06+23%3A22%3A21'
5. The HTTP client library (in this case the Groovy Rest HTTP client) URL encodes the path and query parameters (as it is required to do) to /path/TEST%2BPATH/2014-14-06%2B23%253A22%253A21.

I have some consumer tests at my current client where we had to URL encode the paths in the expectations so that the mock server will match them correctly. Consumer tests should not have to do this.

Secondly, I found the default Java URL encoder uses a plus for spaces, while some clients will use a %20 for the query parameters as well (retrofit is an example of one). For our tests where we are using retrofit as the client, the consumer tests used the default Java encoder and then replaced all pluses with %20 to get them to match. Not ideal.

However, all decoders will decode both %20 and + to a space. Thus I recommend that the mock servers decode the path and query parameters, and store them decoded in the pact file.

[bethesque]

Yeah, I was going to say, the space between the TEST and PATH should be a
%20 not a +. The things you learn when you maintain a HTTP mocking library.

So, I think I agree with you. This will be a breaking change however. If we
change the mock server now, all the previously generated pacts will fail.
Can we make it a configurable parameter, with the current behaviour
maintained as the default, and then swap the behaviour to default to
un-encoded params for 2.0?

I'm not sure if we can add test cases for this, but we'll need to note it
in a spec somewhere.

I'm trying to think of a language agnostic test harness web app we can
write to ensure that we have cross domain consistency with things like
this. The matching specs aren't really going to help, because it's the
behaviour around the encoding/decoding before the matching is done that
will change, not the matching logic itself.

On Sun, Oct 26, 2014 at 11:14 AM, Ronald Holshausen <
notifications@github.com> wrote:

All HTTP clients will encode the path and query string. This issue is that
the JVM mock server (which uses the Netty scala library) does not URL
decode the path before passing it to the handlers.

Here is an example of the problem:

The consumer test sets up the pact interactions and mock server and
the client code executes a request with the following path: /path/TEST
PATH/2014-14-06 23:22:21
2.

The HTTP client library URL encodes this path to
/path/TEST+PATH/2014-14-06+23%3A22%3A21 and executes the request.
3.

The Netty based mock server receives the request with path
/path/TEST+PATH/2014-14-06+23%3A22%3A21, validates it and stores it in
the pact file. For this to work the consumer test had to URL encode the
path in the expectations.
4.

On verification the pact file is loaded and a HTTP request is created
with path '/path/TEST+PATH/2014-14-06+23%3A22%3A21'
5.

The HTTP client library (in this case the Groovy Rest HTTP client) URL
encodes the path and query parameters (as it is required to do) to
/path/TEST%2BPATH/2014-14-06%2B23%253A22%253A21.

I have some consumer tests at my current client where we had to URL encode
the paths in the expectations so that the mock server will match them
correctly. Consumer tests should not have to do this.

Secondly, I found the default Java URL encoder uses a plus for spaces,
while some clients will use a %20 for the query parameters as well
(retrofit is an example of one). For our tests where we are using retrofit
as the client, the consumer tests used the default Java encoder and then
replaced all pluses with %20 to get them to match. Not ideal.

However, all decoders will decode both %20 and + to a space. Thus I
recommend that the mock servers decode the path and query parameters, and
store them decoded in the pact file.

—
Reply to this email directly or view it on GitHub
#66 (comment).

[uglyog]

I released version 2.1.2 to address this issue

[bethesque]

Does that mean it's now not compatible with the ruby implementation? Can we
work out what our migration strategy is?

On Mon, Oct 27, 2014 at 11:43 AM, Ronald Holshausen <
notifications@github.com> wrote:

I released version 2.1.2 to address this issue

—
Reply to this email directly or view it on GitHub
#66 (comment).

[chrisholmes]

Unfortunately, this has not solved our problem and now any url encoding we were expecting at our Provider is no longer being received causing it to return a 404.

For example, we have some dynamic routing in our Provider that uses a url encoded address in the path to reference an entity id which might look something like this:

/some/path/http%3A%2F%2Fwww.foo.com%2FSAML2%2FMD/to/resource/

where http://www.foo.com is the non-encoded entity id. The path above is what we are setting as part of the expectation with the MockProvider as that is what we expect the Consumer to send. However, the value being written to the json and being sent to the Provider during verification looks like:

/some/path/http://www.foo.com/SAML2/MD/to/resource/

[bethesque]

Are you using ruby to verify the pact?

On Tuesday, October 28, 2014, Christopher Holmes notifications@github.com
wrote:

Unfortunately, this has not solved our problem and now any url encoding we
were expecting at our Provider is no longer being received causing it to
return a 404.

For example, we have some dynamic routing in our Provider that uses a url
encoded address in the path to reference an entity id which might look
something like this:

/some/path/http%3A%2F%2Fwww.foo.com%2FSAML2%2FMD/to/resource/

where http://www.foo.com is the non-encoded entity id. The path above is
what we are setting as part of the expectation with the MockProvider as
that is what we expect the Consumer to send. However, the value being
written to the json and being sent to the Provider during verification
looks like:

/some/path/http://www.foo.com/SAML2/MD/to/resource/

—
Reply to this email directly or view it on GitHub
#66 (comment).

[uglyog]

I'm also having some issues with this, so I might reverse the commit and leave the fix in the gradle plugin

[uglyog]

This has been released with version 2.1.5

[bethesque]

Does Netty still have this "problem"?

[uglyog]

No idea, but my assumption would be that it would.

[JimmyLv]

@uglyog Hi, actually I want "+" symbol in path, because I want use a datetime string in query parameter like this "xxxx/xxx?datetime=2011-12-03T10:15:30+01:00", how can I fix it?

[uglyog]

Is this an issue you currently have? Query parameters with a + should work.

[JimmyLv]

@uglyog like this in my pact json file:

"request": "GET",
"path": "/api/xxx/xxx",
"headers": {
},
"query": "datetime=2011-12-03T10:15:30+01:00"

and will be sent to my provider service (using SpringBoot).

then actually I will get "2011-12-03T10:15:30 01:00" ("+" havs lost) when I map it from requestDTO, so that it will throw invalidation exception due to my @DateTimeFormat(iso = DateTimeFormat.ISO.DATE_TIME) annotation.

[uglyog]

Ok, looks like the + is being incorrectly replaced with a space. I will try and find a fix for you.

If you are able, %2B is the encoded value for a +, so if you can use that it may work.

[JimmyLv]

@uglyog Thanks, but I have try to replace it with %2B alone, and it will still be distinguished as %2B string in my service. What I mean just like this error msg: Invalid format: "2015-08-20T10:15:30%2B01:00" is malformed at "%2B01:00"

[JimmyLv]

@uglyog waiting for your reply, T^T

[uglyog]

There is a change to the Pact specification to store the query strings in a different form, which will have to be implemented before I can resolve this issue. It will take a bit of work to fix.

[JimmyLv]

@uglyog Thank you, I has changed it to using '2011-12-03T10:15:30Z' datetime format and it works now.

[uglyog]

Version 3.1.0 allows V3 format pact files which should resolve this issue.

[nikhilkr123]

@uglyog
I am still facing this problem with 3.3.1.
%2F in my URL path is getting converted into / at provider side.
I am using pact version 3.0.0