{"action":"created","issue":{"url":"https://api.github.com/repos/brianmario/escape_utils/issues/23","labels_url":"https://api.github.com/repos/brianmario/escape_utils/issues/23/labels{/name}","comments_url":"https://api.github.com/repos/brianmario/escape_utils/issues/23/comments","events_url":"https://api.github.com/repos/brianmario/escape_utils/issues/23/events","html_url":"https://github.com/brianmario/escape_utils/issues/23","id":2757953,"number":23,"title":"Wanted: correct URI escaping","user":{"login":"technoweenie","id":21,"avatar_url":"https://avatars.githubusercontent.com/u/21?v=3","gravatar_id":"","url":"https://api.github.com/users/technoweenie","html_url":"https://github.com/technoweenie","followers_url":"https://api.github.com/users/technoweenie/followers","following_url":"https://api.github.com/users/technoweenie/following{/other_user}","gists_url":"https://api.github.com/users/technoweenie/gists{/gist_id}","starred_url":"https://api.github.com/users/technoweenie/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/technoweenie/subscriptions","organizations_url":"https://api.github.com/users/technoweenie/orgs","repos_url":"https://api.github.com/users/technoweenie/repos","events_url":"https://api.github.com/users/technoweenie/events{/privacy}","received_events_url":"https://api.github.com/users/technoweenie/received_events","type":"User","site_admin":true},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":8,"created_at":"2012-01-07T21:26:37Z","updated_at":"2015-03-24T21:47:31Z","closed_at":null,"body":"Look at the difference:\r\n\r\n```\r\n>> EscapeUtils.escape_uri 'a/ +b'\r\n=> \"a/%20+b\"\r\n>> EscapeUtils.escape_url 'a/ +b'\r\n=> \"a%2F+%2Bb\"\r\n```\r\n\r\nIMO both are wrong.  Also, the similarity in the two method names is ridiculous.  This is what I'd like to see:\r\n\r\n```\r\n>> EscapeUtils.escape_full_uri 'a/ +b'\r\n=> \"a/%20%2Bb\"\r\n>> EscapeUtils.escape_uri_path 'a/ +b'\r\n=> \"a%2F%20%2Bb\"\r\n```\r\n\r\nThis is closer to how javascript works:\r\n\r\n```\r\n> encodeURI('a/ +b')\r\n'a/%20+b'\r\n> encodeURIComponent('a/ +b')\r\n'a%2F%20%2Bb'\r\n```\r\n\r\nI like to escape `+` to be safe, because it's not entirely clear if you mean a `+` or a space.  If you agree, I'll happily hack it up and send you a pull.  If not, I'll do it anyway and force my fork in GitHub :) :)"},"comment":{"url":"https://api.github.com/repos/brianmario/escape_utils/issues/comments/85706954","html_url":"https://github.com/brianmario/escape_utils/issues/23#issuecomment-85706954","issue_url":"https://api.github.com/repos/brianmario/escape_utils/issues/23","id":85706954,"user":{"login":"ptoomey3","id":103360,"avatar_url":"https://avatars.githubusercontent.com/u/103360?v=3","gravatar_id":"","url":"https://api.github.com/users/ptoomey3","html_url":"https://github.com/ptoomey3","followers_url":"https://api.github.com/users/ptoomey3/followers","following_url":"https://api.github.com/users/ptoomey3/following{/other_user}","gists_url":"https://api.github.com/users/ptoomey3/gists{/gist_id}","starred_url":"https://api.github.com/users/ptoomey3/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ptoomey3/subscriptions","organizations_url":"https://api.github.com/users/ptoomey3/orgs","repos_url":"https://api.github.com/users/ptoomey3/repos","events_url":"https://api.github.com/users/ptoomey3/events{/privacy}","received_events_url":"https://api.github.com/users/ptoomey3/received_events","type":"User","site_admin":true},"created_at":"2015-03-24T21:47:31Z","updated_at":"2015-03-24T21:47:31Z","body":"Bump. It seems like there have a been a few more issues opened up related to this:\r\n\r\n* https://github.com/brianmario/escape_utils/issues/31\r\n* https://github.com/brianmario/escape_utils/issues/54\r\n* https://github.com/brianmario/escape_utils/issues/58\r\n\r\nThe issue raised in #31 is particularly troublesome:\r\n\r\n``` ruby\r\nq = \"plus+plus\"\r\nescaped = EscapeUtils.escape_url(q)\r\nEscapeUtils.unescape_url(escaped) == q\r\n=> false\r\n```\r\n\r\nSurely things should round trip unambiguously. Given that `escape_url` seems to form encode values we can look to the CGI module to see what we would expect:\r\n\r\n```\r\nCGI.unescape(\"a%2bb\")\r\n=> \"a+b\"\r\n\r\nEscapeUtils.unescape_url(\"a%2bb\")\r\n=> \"a b\"\r\n```\r\n\r\nRegarding @technoweenie's original point, I think he is correct in that there is currently no API to encode a component of a path correctly. Though @brianmario went through the other libraries above, I'll repeat his experiment with a bit of context for each:\r\n\r\n### URI\r\n\r\nOverall it is just not clear what context `escape` is meant to do.\r\n\r\n```\r\n>> URI.escape 'a +b'\r\n=> \"a%20+b\"\r\n```\r\n\r\nAt first glance it looks like that encoding is appropriate for a path component (where `+` does not require encoding). But, then you do:\r\n\r\n```\r\n>> URI.escape 'a/ +b'\r\n=> \"a/%20+b\"\r\n```\r\n\r\nDoh, that isn't useful as a path component. It seems like it is trying to interpret the string as a full URI  and escape only the \"special characters\" for each URI component. This is analogous to what JavaScript's `encodeURI` does. But, what if I want to send a `/` in the path? JavaScript has `encodeURIComponent` to handle this, but the URI module has no such equivalent method.\r\n\r\n### CGI\r\n\r\nCGI seems to be pretty easy to reason about. \r\n\r\n```\r\n>> CGI.escape 'a +b'\r\n=> \"a+%2Bb\"\r\n>> CGI.escape 'a/ +b'\r\n=> \"a%2F+%2Bb\"\r\n```\r\n\r\nIt looks like CGI simply encodes things appropriately for query/form parameter encoding. The CGI module has no awareness/support for encoding a path component. We can't use the CGI module for encoding path components because any space will get encoded as a `+`, which is not legit for a path component.\r\n\r\n### Rack\r\n\r\nThis one seems to do more or less what I would expect:\r\n\r\n```\r\n>> Rack::Utils.escape 'a/ +b'\r\n=> \"a%2F+%2Bb\"\r\n>> Rack::Utils.escape 'a +b'\r\n=> \"a+%2Bb\"\r\n```\r\n\r\nThe above works exactly like `CGI.escape` and is appropriate for query parameter encoding. But, unlike CGI, Rack also supports an `escape_path` method that works as follows:\r\n\r\n```\r\n>> Rack::Utils.escape_path 'a/ +b'\r\n=> \"a%2F%20%2Bb\"\r\n>> Rack::Utils.escape_path 'a +b'\r\n=> \"a%20%2Bb\"\r\n```\r\n\r\nTechnically you might hope for the `+` to have been left alone, but encoding it as `%2B` is perfectly fine since it shouldn't break any browser behavior and it is unambiguously decodable. \r\n\r\n### EscapeUtils\r\n\r\n`EscapeUtils#escape_url` seems to work like `CGI#escape` and is fine for encoding query parameters. And, it looks like `EscapeUtils.escape_uri` works more or less like JavaScript's `encodeURI`:\r\n\r\n```\r\nencodeURI('http://google.com/foo bar/ +b?yada=blah bar')\r\n\"http://google.com/foo%20bar/%20+b?yada=blah%20bar\"\r\n\r\nEscapeUtils.escape_uri('http://google.com/foo bar/ +b?yada=blah bar')\r\n=> \"http://google.com/foo%20bar/%20+b?yada=blah%20bar\"\r\n```\r\n\r\nThis is fine, but it is missing the JavaScript analogue of `encodeURIComponent`. So, maybe it would be best to simply add one additional API: `EscapeUtils#escape_uri_component` that models `encodeURIComponent` (which seems more or less the same as Rack's `Rack::Util::escape_path`):\r\n\r\n```\r\nencodeURIComponent('a/ +b')\r\n\"a%2F%20%2Bb\"\r\n\r\nRack::Utils.escape_path('a/ +b')\r\n=> \"a%2F%20%2Bb\"\r\n```\r\n\r\n@brianmario - Do you have any objects to the following two changes?:\r\n\r\n* Make `EscapeUtils#unescape_url` match the behavior of `CGI.unescape` so that things can roundtrip.\r\n* Add `EscapeUtils#escape_uri_component`/`EscapeUtils#unescape_uri_component` that can be used to correctly escape a component of a path?\r\n\r\n@technoweenie - Would that satisfy what you were looking for?"}}