Commit Graph

9 Commits

Author SHA1 Message Date
Neels Hofmeyr 3da9aa6b67 ctrl: tighten CTRL input parsing
Validate that incoming CTRL commands...
- have decimal IDs,
- return error on trailing characters,
- have invalid characters in variable identifiers,
- send detailed error messages as reply to the requestor.

Adjust ctrl_test.{c,ok}, which best show the change in behavior.

Message handling causes log messages on stderr; previously, stderr was empty.
Add '[ignore]' in testsuite.at so that the nonempty stderr doesn't cause test
failures.

Change-Id: I96a9b6b6a3a5e0b80513aa9eaa727ae8c9c7d7a1
2017-12-20 15:50:24 +00:00
Neels Hofmeyr 1b8b152e79 ctrl_test: add two more current parsing bugs to prep for fix
Change-Id: Id11bc326be2f0bc2746a928354e416495a18baf7
2017-12-18 23:05:50 +00:00
Neels Hofmeyr f2c10f1082 ctrl: fix mem leak when handling GET_REPLY and SET_REPLY
In ctrl_handle_msg() (code recently propagated from handle_control_read()),
talloc_free() the parsed ctrl_cmd in all code paths. In particular, a free was
missing in case ctrl_cmd_handle() returns CTRL_CMD_HANDLED.

CTRL_CMD_HANDLED is triggered by GET_REPLY / SET_REPLY parsing, as show by
ctrl_test.c. With the memleak fixed, adjust expected test output and make a
detected mem leak abort the test immediately.

Change-Id: Id583b413f8b8bd16e5cf92a8a9e8663903646381
2017-12-18 23:05:50 +00:00
Neels Hofmeyr 6769ad6e0a ctrl_test: show memleak when receiving GET_REPLY and SET_REPLY commands
The "memleak!" output shows messages that lack a talloc_free() of the parsed
ctrl command buffer. The leak shall be fixed in a subsequent patch.

Change-Id: I2c3e4d08b769b9cd77593362ea36a28d681cd042
2017-12-18 23:05:50 +00:00
Neels Hofmeyr 83aee83ec6 ctrl_test: expand to test message handling and detect mem leaks
Subsequent patches that tighten CTRL input validation will make the results
more interesting.

Change-Id: Idd4cc7d193db1a7d761f72ed33ed46eea450a78f
2017-12-18 23:05:50 +00:00
Neels Hofmeyr 0ab6eca80a ctrl_test.c: replace print_escaped() with new osmo_escape_str()
Change-Id: I12d3828dcc925f97fde11c360f1d60f3bd8cad8b
2017-12-18 23:05:49 +00:00
Neels Hofmeyr 505c965e36 CTRL: add unit tests for CTRL command parsing
This uncovers some interesting behavior of the CTRL interface which we may want
to guard against in subsequent patches: trailing whitespace, ignored tokens,
special characters as cmd->id.

Change-Id: If7af06d50ca71fd528b08cd70310774d5a53f0f7
2017-09-27 14:04:23 +00:00
Neels Hofmeyr 7c1ec8c8e7 ctrl_type_vals: explicitly terminate
Don't use CTRL_TYPE_UNKNOWN as value_string[] terminator, use an explicit, more
obvious { 0, NULL } termination. Set an explicit string for CTRL_TYPE_UNKNOWN.
No other value_string[]s to date have such a "hidden" terminator.

BTW, a { 0, "string" } item is not a terminator, only { 0, NULL } is, so we can
set a string for CTRL_TYPE_UNKNOWN == 0.

Also, having a string value for CTRL_TYPE_UNKNOWN is not harmful because all
code paths explicitly check for the CTRL_TYPE_*s that are valid.

Adjust the test expectation.

From the ctrl_type_vals enum, remove the = 0, because it is implicitly 0
anyway.

One motivation to press this fixup: I am trying to add a script that checks
whether all value_string[]s are terminated to our jenkins jobs, and to find
that this one is terminated, it would need to interpret the CTRL_TYPE_UNKNOWN
constant, which would make things far more complex. At this point, all of the
value_string[]s have an explicit termination, and I would like to enforce this
from now on -- for readable code and to not spend more time on the validator.

The patch adding ctrl_type_vals (Icd4e96dd9f00876cb70b43cfcf42ab4f10311b28) was
accepted by another reviewer before I could reconfirm my -1, so this is a fixup
to enable the termination checking script patches.

Related: I2bc93ab4781487e7685cfb63091a489cd126b1a8 (adds script to libosmocore)
         I7fe3678b524d602fc6aa14bc0ed06308df809a3e (uses in jenkins.sh)
	 Icd4e96dd9f00876cb70b43cfcf42ab4f10311b28 (adds ctrl_type_vals)

Change-Id: Ia99f37464c7b36b587da2cc78f52c82725f02cbc
2017-03-15 13:47:20 +00:00
Max 70c7d4160d Use value_string for ctrl_type
Use value_string for enum ctrl_type instead of custom code. Add
corresponding unit tests.

Related: OS#1615
Change-Id: Icd4e96dd9f00876cb70b43cfcf42ab4f10311b28
2017-03-01 16:37:59 +00:00