String parameter values are escaped in generated doc

[sam-kim]

When passing parameters in the API call with an iso8601 datetime parameter like this

params = {
  time: "2020-04-20T00:00:00Z",
}

the generated docs end up escaping the :s (and probably does for other special characters)

GET /api/v1/my_route?time=2020-04-20T00%3A00%3A00Z

I haven't dug into where the http routes are being generated, but we should be able to fix by unescaping.

[13] pry(main)> CGI.escape(":")
=> "%3A"
[14] pry(main)> URI::unescape("%3A")
=> ":"

[15] pry(main)> CGI.escape("2020-04-20T00:00:00Z")
=> "2020-04-20T00%3A00%3A00Z"
[16] pry(main)> URI::unescape("2020-04-20T00%3A00%3A00Z")
=> "2020-04-20T00:00:00Z"

[17] pry(main)> URI::unescape(CGI.escape("2020-04-20T00:00:00Z"))
=> "2020-04-20T00:00:00Z"

[syoder]

I think we need to fix request_path. Maybe we can collaborate on this - I'd love to get this fixed since I was able to make a lot of headway on making the rest of our generated docs not churn so much. So now this is the one thing that doesn't quite generate correctly.

[syoder]

Also need to change request_url for the curl examples.

I tried this over the weekend, and it worked:

      def request_path
        URI::unescape(@_group.request.fullpath) unless @_group.request.nil?
      end

But I'm not sure if we need to somehow only unescape the query params, or if we should just unescape the whole thing.

[cupakromer]

I believe Ruby has marked URI.unescape as obsolete. I think the new suggestion is either a different method or to use URI::DEFAULT_PARSER or URI::RFC2396_Parser#unescape directly. However, I'm not sure this is the correct approach. I believe the intent is to ensure the URLs produced are valid. Which means we need to escape them incase there are invalid characters (like spaces).

What we need to do is either find a different escape method which won't escape colons or find a way to configure the escaping to not escape colons.

[syoder]

We could monkeypatch String to not escape, like this:

  class String
    def to_query(key)
      "#{CGI.escape(key.to_param)}=#{to_param}"
    end
  end

Or we could create a "presenter" type object class that implements that, and then you'd have to use it whenever you want to inhibit escaping.

Both of those things could be done in the host app, rather than in this gem.

This is what Rails does to do the escaping.

[syoder]

So here's what I've got so far that I like - in the host app, I'm doing this:

      class DoNotEscape < SimpleDelegator
        def to_query(key)
          "#{CGI.escape(key.to_param)}=#{to_param}"
        end
      end

      class ::Object
        def not_escaped
          DoNotEscape.new(self)
        end
      end

Then you can define your params like this:

    {
      start_time: "2020-04-20T00:00:00Z".not_escaped,
      end_time: "2020-04-20T23:59:59Z".not_escaped,
    }

[sam-kim]

I believe Ruby has marked URI.unescape as obsolete.

Oh right, ruby-doc recommends using CGI.unescape instead. However, that will still be an issue since you don't want to unescape everything. I can't think of a simple way to escape (or unescape) only colons characters. Also there will probably be other characters that we will want to keep from escaping (')?

I've never used delegators so I will spend a little time reading up on them, but this seems like the favorable approach over 🙈-patching.