Skip to content
GitLab

Ruby: Avoid double escaping path items

Merged Administrator requested to merge github/fork/ccouzens/ruby_avoid_double_escaping into master

Created by: ccouzens

URI.encode is obsolete. CGI.escape, URI.encode_www_form or URI.encode_www_form_component are recommended instead. https://ruby-doc.org/stdlib-2.6/libdoc/uri/rdoc/URI/Escape.html#method-i-escape

URI.encode has different behaviour to CGI.escape:

URI.encode('hello/world?test%string')
=> "hello/world?test%25string"
CGI.escape('hello/world?test%string')
=> "hello%2Fworld%3Ftest%25string"

I recently raised pull request #3039 201cbdce

That pull request escapes path items at insertion.

Before either pull request, the path item 'hello?world' would go into the URL as 'hello?world'. That behaviour was insecure as if an attacker could control the path item value, they could change the URL the application connected to.

After #3039 'hello?world' would go in as 'hello%253Fworld'. This was safer than before, but it's still not correct. If I'd realised at the time, I would have made it correct at the time.

What this pull request does is make it go in as 'hello%35world', which is correct.

ApiClient::build_request_url was URI.encoding the whole path. This wasn't protecting against all undesirable characters in the path items, but was escaping % characters a 2nd time which was unhelpful.

I have additionally removed URI.encode from Configuration::base_url as I can't see any benefit it could be bringing. There is no justification for it in the commit where it was originally added: 47c8597d

CC: ruby technical committee @cliffano @zlx @autopp

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\. If contributing template-only or documentation-only changes which will change sample output, be sure to build the project first.
  • Filed the PR against the correct branch: master, 4.1.x, 5.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

see above