[bug fix] Stop manual URI parsing/escaping and use CGI::parse

[smaeda-ks]

Problem

Sometimes, users need to pass URL strings that contain query strings as a parameter.
E.g., website_url parameter from the POST accounts/:account_id/cards/website endpoint (Ads API).

In this case, they have to escape the URL strings in order to let the parser to parse parameters correctly:

raw: https://developer.twitter.com/params?aaa=bbb&ccc=ddd
parsed: https%3A%2F%2Fdeveloper.twitter.com%2Fparams%3Faaa%3Dbbb%26ccc%3Dddd

$ twurl -XPOST -H ads-api.twitter.com '/6/accounts/18ce54t1ol3/cards/website?media_key=3_1102477685765206016&name=WebsiteCardTest-buggy&website_title=WebsiteCardTest-buggy&website_url=https%3A%2F%2Fdeveloper.twitter.com%2Fparams%3Faaa%3Dbbb%26ccc%3Dddd' | jq

but the problem is, this code block is doing some extra job and hence the parsed parameter I included in the above twurl request gets "double-escaped".
https://github.com/twitter/twurl/blob/v0.9.3/lib/twurl/cli.rb#L161-L167

so the above twurl request will actually fail like this:

$ twurl -XPOST -H ads-api.twitter.com '/6/accounts/18ce54t1ol3/cards/website?media_key=3_1102477685765206016&name=WebsiteCardTest-buggy&website_title=WebsiteCardTest-buggy&website_url=https%3A%2F%2Fdeveloper.twitter.com%2Fparams%3Faaa%3Dbbb%26ccc%3Dddd' | jq
{
  "errors": [
    {
      "code": "INVALID_PARAMETER",
      "message": "Invalid URL encoded params",
      "parameter": "website_url"
    }
  ],
  "request": {
    "params": {
      "name": "WebsiteCardTest-buggy",
      "account_id": "18ce54t1ol3",
      "media_key": "3_1102477685765206016",
      "website_title": "WebsiteCardTest-buggy",
      "website_url": "https%3A%2F%2Fdeveloper.twitter.com%2Fparams%3Faaa%3Dbbb%26ccc%3Dddd"
    }
  }
}

Solution

Just stop doing manual URI parsing and use the CGI::parse method instead.

Result

$ twurl -XPOST -H ads-api.twitter.com '/6/accounts/18ce54t1ol3/cards/website?media_key=3_1102477685765206016&name=WebsiteCardTest-buggy&website_title=WebsiteCardTest-buggy&website_url=https%3A%2F%2Fdeveloper.twitter.com%2Fparams%3Faaa%3Dbbb%26ccc%3Dddd' | jq
{
  "data": {
    "name": "WebsiteCardTest-buggy",
    "website_shortened_url": "https://t.co/Vji3Zyh076",
    "image_display_height": "800",
    "media_url": "https://pbs.twimg.com/media/D0zJnTDUcAA0snp.png",
    "website_display_url": "developer.twitter.com",
    "id": "84w31",
    "website_dest_url": "https://developer.twitter.com/params",
    "media_key": "3_1102477685765206016",
    "created_at": "2019-10-01T02:20:03Z",
    "image_display_width": "800",
    "website_title": "WebsiteCardTest-buggy",
    "card_uri": "card://1178857075960471554",
    "website_url": "https://developer.twitter.com/params?aaa=bbb&ccc=ddd",
    "updated_at": "2019-10-01T02:20:03Z",
    "deleted": false,
    "card_type": "WEBSITE"
  },
  "request": {
    "params": {
      "name": "WebsiteCardTest-buggy",
      "website_shortened_url": "https://t.co/Vji3Zyh076",
      "image_display_height": "800",
      "media_url": "https://pbs.twimg.com/media/D0zJnTDUcAA0snp.png",
      "website_display_url": "developer.twitter.com",
      "account_id": "18ce54t1ol3",
      "website_dest_url": "https://developer.twitter.com/params",
      "media_key": "3_1102477685765206016",
      "image_display_width": "800",
      "website_title": "WebsiteCardTest-buggy",
      "website_url": "https://developer.twitter.com/params?aaa=bbb&ccc=ddd",
      "card_type": "WEBSITE"
    }
  }
}

As you can see, with this patch,
"website_url": "https://developer.twitter.com/params?aaa=bbb&ccc=ddd",
which is what I expect to see.

Other bug fixes

This PR includes other bug fixes that will resolve the below issues:
#117
#77 (initial PR was: #108)

[coveralls]

Coverage decreased (-0.2%) to 87.982% when pulling e236eda on smaeda-ks:smaeda-ks/fix-uri-parse into 7eb9a13 on twitter:master.

[andypiper]

Hey @smaeda-ks thank you! i'll connect with you and @dlamacchia as I'm not sure what the best approach is to getting this merged. Appreciate it!

[smaeda-ks]

@andypiper @dlamacchia
Based on the test results, I'm fine with merging this to the master branch and see if other outstanding PRs can pass the test afterward. (I'm not talking about a version release here, but just maintaining the upstream branch. Releasing a new version would be a separate discussion, I guess.)

[CLAassistant]

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[smaeda-ks]

ping @dlamacchia

[smaeda-ks]

Just found some problems with this PR, will add more commits so please hold on. Thanks

[smaeda-ks]

@dlamacchia OK, so I fixed a bug in the latest commit: 5c05a68 and now I think it's finally safe to merge.

cc: @osowskit since you originally gave me an idea of this fix.

[smaeda-ks]

@dlamacchia @andypiper

I updated the description but now this PR includes two more major bug fixes:
#77
#117

Those are also parsing/escaping issues. I'm really eager to merge this fix.