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>
Thu, 16 Apr 2020 01:03:21 +0000 (11:03 +1000)
commita0f864fd7be92b53ff3a9d36dda4aa3491470c7b
treec118f3b588556a1ac4fe1b673957eceb38b68c9c
parentd2cad5f0ae0da394942a80fd874f4a712d1d6e9e
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