[Ovmsdev] Warning in Duktape
Michael Balzer
dexter at expeedo.de
Sat Aug 3 15:09:05 HKT 2024
I've just merged your PR.
`OvmsCommand.Register()` is well designed.
On security: I don't think we actually need to allow adding user
commands to system command groups at all.
I'd opt for allowing only command root "usr", and optionally "x…"
(vehicle specific namespaces), for user commands. Allowing "x…" should
be a choice/config of the vehicle module.
We could add a flag to the command definition and extend the
registration accordingly to…
OvmsCommand* RegisterCommand(const char* name, const char* title,
OvmsCommandExecuteCallback_t execute =
NULL,
const char *usage = "", int min = 0,
int max = 0, bool secure = true,
OvmsCommandValidateCallback_t validate
= NULL,
*bool allow_user_commands = false*);
Similar to the config param "usr" init, we then generally create an
empty command root "usr" on system init with allow_user_commands = true.
Vehicles can decide to create their full "x…" root, or some "x… usr"
placeholder with that grant.
What do you think?
Regards,
Michael
Am 22.07.24 um 02:36 schrieb Michael Geddes via OvmsDev:
> I have tidied up the implementation significantly and I've fixed up
> all the life-time issues, as well as disallowing overriding existing
> commands.
> (Though you could still add a sub-command to a system command). It
> now persists quite happily, doesn't crash on GC and functions as
> expected :). I've also implemented shut-down registering so it cleans
> up before getting to the destructor.
>
> It's now down to security and the question of whether we differentiate
> user plugin commands from system ones.
>
> //.ichael
>
> On Sun, 21 Jul 2024 at 08:34, Michael Geddes
> <frog at bunyip.wheelycreek.net> wrote:
>
> Well since I've been doing some Duktape stuff lately, I recalled
> this loose end and have come up with a solution.
>
> There were a few issues - please let me know if you have a problem
> with any of them.
>
> Firstly the Command registration map is using const char * and we
> can't guarantee the lifetime of the duktape strings so I've moved
> them over to std::string.
> This applies to OvmsCommandMap itself as well as
> OvmsCommand - m_name, m_title and m_usage_template.
>
> I think the intention was to have something like this for the
> register command.. it was missing a parameter. It now looks like this:
> OvmsCommand.Register( function, parent, command, description,
> parameter_desc, minargs, maxargs).
> Missing maxargs will mean maxargs = minargs.
> Missing minargs will mean no parameters.
> The calling function will be passed 2 parameters: the name of the
> command and an array of strings (the argv).
> For sub-commands, these will be separated by '/'. So something
> like "me/too".
> I think this way is simple but effective.
> It would be good to add completion, but that can be added later.
>
> One of the things I wanted to talk about is what security
> protections we need to add to prevent a script from taking over an
> existing function! (or future function).
>
> When vim initially added user commands and functions it was a
> free-for-all but somebody soon realised there needed to be some
> restrictions to avoid issues..
> so what they did was force user function and commands to start
> with a capital letter. I would like people's thoughts on whether
> we need to do something similar.
>
> It might be that we would be satisfied with failing if system
> commands were attempted to be overridden. Should we allow adding
> sub-commands to system commands?
> I can see that might be useful..
>
> I have added a sample below of a working function:
>
> --8<----
>
> mycommand = function(c, argv){
> print("Hello There: "+c+"\n");
> if (argv.length == 0)
> print("Simple\n");
> else
> print("Script: "+argv[0]+"\n")
> }
> OvmsCommand.Register(mycommand, "", "me", "My Script", "[string]",
> 0, 3)
>
> /OVMS#/ *me xx*
> Hello There: me
> Script: xx
> /OVMS# /*me xx yy*
> Hello There: me
> Script: xx
> /OVMS#/ *me xx yy*
> Hello There: me
> Script: xx
> /OVMS#/
>
> On Sun, 23 Oct 2022 at 22:15, Michael Balzer <dexter at expeedo.de>
> wrote:
>
> That's an open end, implementing command registration for Duktape.
>
> There's a thread on this… found it:
> http://lists.openvehicles.com/pipermail/ovmsdev/2020-July/006952.html
>
> Regards,
> Michael
>
>
> Am 23.10.22 um 15:38 schrieb Michael Geddes:
>> There's an unused variable 'dcc' warning in the below code.
>>
>> It seems that the 'Perform the callback' is not being 'performed!
>>
>> Is this deliberate?
>>
>>
>> //.ichael
>>
>>
>>
>> void DukOvmsCommandRegisterRun(int verbosity, OvmsWriter*
>> writer, OvmsCommand* cmd, int argc, const char* const* argv)
>> {
>> ESP_LOGD(TAG, "DukOvmsCommandRegisterRun(%s)",cmd->GetName());
>> auto it = MyDuktape.m_cmdmap.find(cmd);
>> if (it == MyDuktape.m_cmdmap.end())
>> {
>> ESP_LOGE(TAG, "Command '%s' cannot be found in
>> registry",cmd->GetName());
>> return;
>> }
>> else
>> {
>> DuktapeConsoleCommand* dcc = it->second;
>> // Perform the callback
>> }
>> }
>>
>> _______________________________________________
>> OvmsDev mailing list
>> OvmsDev at lists.openvehicles.com
>> http://lists.openvehicles.com/mailman/listinfo/ovmsdev
>
> --
> Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal
> Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
>
> _______________________________________________
> OvmsDev mailing list
> OvmsDev at lists.openvehicles.com
> http://lists.openvehicles.com/mailman/listinfo/ovmsdev
>
>
> _______________________________________________
> OvmsDev mailing list
> OvmsDev at lists.openvehicles.com
> http://lists.openvehicles.com/mailman/listinfo/ovmsdev
--
Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal
Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openvehicles.com/pipermail/ovmsdev/attachments/20240803/40d5a8c1/attachment-0001.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 203 bytes
Desc: OpenPGP digital signature
URL: <http://lists.openvehicles.com/pipermail/ovmsdev/attachments/20240803/40d5a8c1/attachment-0001.sig>
More information about the OvmsDev
mailing list