[Bug 47899] New: Review the VM name validation

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=47899

Bug ID: 47899
           Summary: Review the VM name validation
           Product: Wine-Testbot
           Version: unspecified
          Hardware: x86
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: unknown
          Assignee: [e-mail]
          Reporter: [e-mail]
      Distribution: ---

The web interface allows the creation of VMs that have dashes in their name.
But when untainting its parameters LibvirtTool considers such names to be
invalid, so that trying to revert these VMm fails.

There is a VM::Validate() method that should validate the VM name among others,
but obviously it cannot be used to check and untaint script parameters.

So the VM name validation criterions need to be checked and a standard function
provided.

[Bug 47899] Review the VM name validation

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=47899

--- Comment #1 from François Gouget  ---
Why were dashes added to the VM name?

Because Windows represents a locale in the form 'lang-COUNTRY': the chain goes
like this: intl.cpl wants dashes -> SetWinLocale wants dashes -> locale
snapshot names contain dashes -> why not name the VM the same.

Plan:
* Don't allow dashes in the VM names.
* Add an UntaintVMName() function.
* Use Unix-style locales everywhere and do the Unix to Windows conversion in
SetWinLocale.
* This impacts LibvirtTool.
* Rename live snapshots to use underscored locale names the next time it's
necessary to recreate them.

[Bug 47899] Review the VM name validation

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=47899

François Gouget  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|[e-mail]        |[e-mail]

[Bug 47899] Review the VM name validation

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=47899

--- Comment #2 from François Gouget  ---
This is mostly done. What remains is the SetWinLocale part and renaming past
snapshot names.

* Not allowing dashes in VM names is fixed by this commit:
commit 77c97d74279b208bf05d2da7453d4defbe2464fb

testbot: Fix the VM Missions validation.

$Item->Xxx("foo") does not go through PutColValue()!
    Instead AUTOLOAD sets the specified property directly, maybe fori
    performance reasons, and PutColValue() is only used by DBIBackend.pm.

WineTestBot::VMs was overriding PutColValue() to only revalidate a VM's
    missions field if either the VM Type of Missions field were modified.
    But instead ValidateMissions was always true after loading a VM from the
    database (because of the PutColValue() calls), and not when creating a
    new VM (in which case all the properties are set through AUTOLOAD).

So take a snapshot of the initial Type and Missions values when the
    database code resets the IsModified field by calling ResetModified();
    and then revalidate them whenever they have changed.

Signed-off-by: Francois Gouget 
    Signed-off-by: Alexandre Julliard

* A VM::UntaintName() method has been added in this commit.

commit 4bb065f0b60710ae3c91d3444bf1df1febd39023
Author:     Francois Gouget 
AuthorDate: Mon Dec 2 13:55:07 2019 +0100

testbot: Validate VM names and add UntaintName().

This prevents discrepencies between the VM creation interface and
    LibvirtTool's validation of VM names.

Signed-off-by: Francois Gouget 
    Signed-off-by: Alexandre Julliard