CIV-01-021 - Improve entity name sanitization
authorTim Otten <totten@civicrm.org>
Wed, 4 Mar 2020 02:54:50 +0000 (18:54 -0800)
committerSeamus Lee <seamuslee001@gmail.com>
Sat, 11 Apr 2020 20:49:43 +0000 (06:49 +1000)
commit589b2f176278e7d6ecf80d69f8ad568f6dd0f8c5
tree67a73374c46d327c3dfab664ce522e1d036f5dfb
parent7deeeda9ac6c1c96d3b121baa80242053294b5b9
CIV-01-021 - Improve entity name sanitization

Before
------

* There exist two functions which purport to take an API entity name and sanitize it,
  producing a canonical API entity name. (`\Civi\API\Request::normalizeEntityName`
  and `_civicrm_api_get_camel_name`)
* The two functions are identical for typical inputs. Both call `convertStringToCamel()`.
* The difference relates to unusual/unspecified input characters like `/` or `.` or `+`.
   * `_civicrm_api_get_camel_name()` allows/returns unusual characters.
   * `normalizeEntityName()` filters them out via `\CRM_Utils_String::munge()`

After
-----

* `_civicrm_api_get_camel_name()` just calls `normalizeEntityName()`
* A unit-test provides some comparison/contrast between the old+new behaviors.

Comments
--------

I came into this because CIV-01-021 pointed out that `_civicrm_api_get_camel_name()` had
insufficient sanitization of wonky inputs and could potentially lead to unexpected file-reads.

You can potentially address those wonky inputs by filtering them out or by throwing an exception.
I initially started doing an exception... but it turned out that `normalizeEntityName()` was already
filtering out and didn't really need a change. Also, regardless of the policy, the functions should be
brought into alignment.

Anyway, it seemed like this was the simpler change - it keeps `normalizeEntityName()` working exactly
as before, and only changes `_civicrm_api_get_camel_name()` to match.
api/api.php
tests/phpunit/api/v3/UtilsTest.php