Wrong parameter parsing

[luis4a0]

Describe the bug
Issuing multipass launch -n asdf -mem 2048M gives

launch failed: '2048M' is not a supported alias. Please use `multipass find` for supported image aliases.

Expected behavior
-mem should be recognized as a wrong parameter, before checking the validity of 2048M as an alias.

[townsend2010]

Hey @luis4a0,

Thanks for entering this. For the expected behavior, I'm not sure if this is what you meant, but -mem itself is itself a valid option (the -m part of it is the option), it's just that em is not a valid memory size and that is what needs to be validated. Same goes for all other options.

For example, it's possible someone may enter -cpus and that would be parsed as -c with pus being the number of CPUs given at launch.

[luis4a0]

The fact that -mem is a valid option escaped me. Thanks for pointing this out!

Now I see it's not a trivial fix. We need to parse in two stages (analogous to lex and yacc): first we generate tokens from all the options in the command line (that is, one for each unit separated by spaces) and then we parse the those tokens, combined in the order given. Maybe there is a simpler way to do it, I'll think about it.

[townsend2010]

Hey @luis4a0, ok, cool.

It should actually be quite trivial to fix. In validate_create_arguments(), instead of calling validate_image() early, call it after all of the other options have been validated. 😉

[ricab]

Perhaps this could be dealt with in the client: confirm it is a valid MemorySize before adding it to the request. That's already done for CPUs.

[luis4a0]

Thanks for your insights @townsend2010 and @ricab! I implemented the latter, it proved to be easier. In fact, I didn't remove check code for the daemon because it is still needed to check that the requested size is bigger than the minimum.

[Saviq]

I didn't remove check code for the daemon

You also can't trust the client to have done the validation :)

[ricab]

I didn't remove check code for the daemon

You also can't trust the client to have done the validation :)

Yep, the daemon of course needs to validate the contents of RPC requests (to be sure, I did not mean to suggest otherwise).