URL with URL-unsafe characters causes invalid URI errors

[amcaplan]

I'm testing an API where we have the name of the resource as the unique identifier. If the name includes a space or any other URL-unsafe character, I see the following:

Failure/Error: it { is_expected.to validate(:get, '/resources/{name}', 200, params) }
URI::InvalidURIError:
 bad URI(is not URI?): http://www.example.com:80/resources/name of thing

I would expect apivore to convert this to

/resources/name+of+thing

so my tests can remain legible.

I'm happy to write a patch; it would just involve adding CGI::escape to https://github.com/westfieldlabs/apivore/blob/275e799ea72c2a5961b56b1742c2e1d09daf308c/lib/apivore/validator.rb#L50 as is done internally in Rails' Object#to_query (see https://github.com/rails/rails/blob/52ce6ece8c8f74064bb64e0a0b1ddd83092718e1/activesupport/lib/active_support/core_ext/object/to_query.rb#L12).

Please let me know if this would be valuable, or if the thought is that that actually should be an error.

[lucasrenan]

@amcaplan is it possible to use the to_query rails method?

[amcaplan]

No. to_query converts a key-value pair into key=value for use in a URL query string. CGI::escape is for a single String. It's also in the Ruby standard library, already used in any Rails app, so I'm not sure why we'd not want to use it.

[gwshaw]

@amcaplan This would be a breaking change as as any string already escaped would be doubly escaped and then fail. It could be considered for a later Apivore version.

Personally, I don't see: it { is_expected.to validate(:get, '/resources/{CGI.escape(name)}', 200, params) } as illegible. It is also explicit that there can be things in name that need special treatment. Often a significant understanding of code can be found by seeing how the tests are written.

There are also many ways to bury and refactor the above into Rails-like URL helpers, thus:

def resources_path(name)
  "/resources/#{CGI.escape(name)}"
end

it { is_expected.to validate(:get, resources_path(name), 200, params) }

and many other options.

[gwshaw]

Breaking change

[amcaplan]

@gwshaw I think I understood what you said, but I don't think it was expressed clearly. The validation URL has to exactly match what the Swagger spec has. So if Swagger has a URL '/resources/{name}', the URL to validate has to be '/resources/{name}' exactly, with name set in params. If you try:

it { is_expected.to validate(:get, '/resources/escaped+name', 200, {})

the endpoint to validate won't be recognized by Apivore.

Instead, you have to do:

let(:params) {{ 'name' => 'string with spaces') }}
it { is_expected.to validate(:get, '/resources/{name}', 200, params) }

which of course breaks, because Apivore tries the URI /resources/string with spaces, which isn't a valid URI.

I'm still pretty sure you're right, so I've rephrased a bit.

For posterity, here's my understanding:

1. it { is_expected.to validate(:get, '/resources/{name}', 200, params) } fails if params['name'] is a non-URL-safe String.
2. I suggested calling CGI::escape on URL params to make sure they're URL-safe.
3. @gwshaw pointed out that if you already escaped it (e.g., params['name'] = 'string+with+spaces'), it'll be re-escaped ('string%2Bwith%2Bspaces'), which would be a breaking change.
4. Hence, the recommended solution in this case is to escape it yourself:

   let(:params) {{ 'name' => CGI.escape("string with spaces") }}
   it { is_expected.to validate(:get, '/resources/{name}', 200, params) }

Is that right?

[gwshaw]

Sorry, I misinterpreted your code and my examples were then incorrect, but yes, you need to escape it yourself as you show in your example.