[PATCH] virDomainMachineNameAppendValid: Handle special characters better

Michal Privoznik posted 1 patch 2 years, 10 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/ecc4380be197655c79855451f4cc5060845ebc47.1624630250.git.mprivozn@redhat.com
src/hypervisor/domain_driver.c | 8 ++++----
tests/virsystemdtest.c         | 3 +++
2 files changed, 7 insertions(+), 4 deletions(-)
[PATCH] virDomainMachineNameAppendValid: Handle special characters better
Posted by Michal Privoznik 2 years, 10 months ago
When constructing guest name for machined we have to be very
cautious as machined expects a name that's basically a valid URI.
Therefore, if there's a dot it has to be followed by a letter or
a number. And if there's a sequence of two or more dashes they
should be joined into a single dash. These rules are implemented
in virDomainMachineNameAppendValid(). There's the @skip variable
which is supposed to track whether it is safe to append a dot or
a dash into name. However, the variable is set to false (meaning
it is safe to append a dot or a dash) even if the current
character we are processing is not in the set of allowed
characters (and thus skipped over).

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1948433
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/hypervisor/domain_driver.c | 8 ++++----
 tests/virsystemdtest.c         | 3 +++
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c
index f7d49bc38a..29e11c0447 100644
--- a/src/hypervisor/domain_driver.c
+++ b/src/hypervisor/domain_driver.c
@@ -63,18 +63,18 @@ virDomainMachineNameAppendValid(virBuffer *buf,
             break;
 
         if (*name == '.' || *name == '-') {
-            if (!skip)
+            if (!skip) {
                 virBufferAddChar(buf, *name);
-            skip = true;
+                skip = true;
+            }
             continue;
         }
 
-        skip = false;
-
         if (!strchr(HOSTNAME_CHARS, *name))
             continue;
 
         virBufferAddChar(buf, *name);
+        skip = false;
     }
 
     /* trailing dashes or dots are not allowed */
diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c
index f1c9a4ee5f..a09b428a8a 100644
--- a/tests/virsystemdtest.c
+++ b/tests/virsystemdtest.c
@@ -736,6 +736,9 @@ mymain(void)
     TEST_MACHINE("demo.-.test.", NULL, 11, "qemu-11-demo.test");
     TEST_MACHINE("demo", "/tmp/root1", 1, "qemu-embed-0991f456-1-demo");
     TEST_MACHINE("demo", "/tmp/root2", 1, "qemu-embed-95d47ff5-1-demo");
+    TEST_MACHINE("|.-m", NULL, 1, "qemu-1-m");
+    TEST_MACHINE("Auto-esx7.0-rhel7.9-special-characters~!@#$%^&*_=+,?><:;|.\"[]()`\\-m",
+                 NULL, 1, "qemu-1-Auto-esx7.0-rhel7.9-special-characters.m");
 
 # define TESTS_PM_SUPPORT_HELPER(name, function) \
     do { \
-- 
2.31.1

Re: [PATCH] virDomainMachineNameAppendValid: Handle special characters better
Posted by Daniel Henrique Barboza 2 years, 9 months ago

On 6/25/21 11:10 AM, Michal Privoznik wrote:
> When constructing guest name for machined we have to be very
> cautious as machined expects a name that's basically a valid URI.
> Therefore, if there's a dot it has to be followed by a letter or
> a number. And if there's a sequence of two or more dashes they
> should be joined into a single dash. These rules are implemented
> in virDomainMachineNameAppendValid(). There's the @skip variable
> which is supposed to track whether it is safe to append a dot or
> a dash into name. However, the variable is set to false (meaning
> it is safe to append a dot or a dash) even if the current
> character we are processing is not in the set of allowed
> characters (and thus skipped over).
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1948433
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

>   src/hypervisor/domain_driver.c | 8 ++++----
>   tests/virsystemdtest.c         | 3 +++
>   2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c
> index f7d49bc38a..29e11c0447 100644
> --- a/src/hypervisor/domain_driver.c
> +++ b/src/hypervisor/domain_driver.c
> @@ -63,18 +63,18 @@ virDomainMachineNameAppendValid(virBuffer *buf,
>               break;
>   
>           if (*name == '.' || *name == '-') {
> -            if (!skip)
> +            if (!skip) {
>                   virBufferAddChar(buf, *name);
> -            skip = true;
> +                skip = true;
> +            }
>               continue;
>           }
>   
> -        skip = false;
> -
>           if (!strchr(HOSTNAME_CHARS, *name))
>               continue;
>   
>           virBufferAddChar(buf, *name);
> +        skip = false;
>       }
>   
>       /* trailing dashes or dots are not allowed */
> diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c
> index f1c9a4ee5f..a09b428a8a 100644
> --- a/tests/virsystemdtest.c
> +++ b/tests/virsystemdtest.c
> @@ -736,6 +736,9 @@ mymain(void)
>       TEST_MACHINE("demo.-.test.", NULL, 11, "qemu-11-demo.test");
>       TEST_MACHINE("demo", "/tmp/root1", 1, "qemu-embed-0991f456-1-demo");
>       TEST_MACHINE("demo", "/tmp/root2", 1, "qemu-embed-95d47ff5-1-demo");
> +    TEST_MACHINE("|.-m", NULL, 1, "qemu-1-m");
> +    TEST_MACHINE("Auto-esx7.0-rhel7.9-special-characters~!@#$%^&*_=+,?><:;|.\"[]()`\\-m",
> +                 NULL, 1, "qemu-1-Auto-esx7.0-rhel7.9-special-characters.m");
>   
>   # define TESTS_PM_SUPPORT_HELPER(name, function) \
>       do { \
>