esx: URI encode inventory objects twice

[PATCH 1/2] esx_util: Introduce esxUtil_EscapeInventoryObject()

Posted by Michal Privoznik via Devel 3 weeks, 3 days ago

From: Michal Privoznik <mprivozn@redhat.com>

The aim of this helper function is to URI-encode given string
twice. There's a bug (fixed in next commit) in which we're unable
to fetch .vmx file for a domain if corresponding datastore
contains some special characters (like +). Cole Robinson
discovered that encoding datastore twice enables libvirt to work
around the issue [2]. Well, this function does exactly that.
It was tested with the following inputs and all worked
flawlessly: "datastore", "datastore2", "datastore2+",
"datastore3+-@", "data store2+".

1: https://issues.redhat.com/browse/RHEL-134127
2: https://issues.redhat.com/browse/RHEL-133729#comment-28604072

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/esx/esx_util.c | 17 +++++++++++++++++
 src/esx/esx_util.h |  3 +++
 2 files changed, 20 insertions(+)

diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c
index 7ee0e5f7c0..e47ea36730 100644
--- a/src/esx/esx_util.c
+++ b/src/esx/esx_util.c
@@ -448,3 +448,20 @@ esxUtil_EscapeForXml(const char *string)
 
     return virBufferContentAndReset(&buffer);
 }
+
+
+
+/* esxUtil_EscapeInventoryObject:
+ * @buf: the buffer to append to
+ * @string: the string argument which will be URI-encoded
+ *
+ * URI-encode given @string TWICE and append the result to the @buf.
+ */
+void
+esxUtil_EscapeInventoryObject(virBuffer *buf, const char *string)
+{
+    g_autoptr(GString) escaped = g_string_new(NULL);
+
+    g_string_append_uri_escaped(escaped, string, NULL, false);
+    virBufferURIEncodeString(buf, escaped->str);
+}
diff --git a/src/esx/esx_util.h b/src/esx/esx_util.h
index 58bc44e744..29f01e0c15 100644
--- a/src/esx/esx_util.h
+++ b/src/esx/esx_util.h
@@ -22,6 +22,7 @@
 #pragma once
 
 #include "internal.h"
+#include "virbuffer.h"
 #include "viruri.h"
 
 #define ESX_VI_CHECK_ARG_LIST(val) \
@@ -67,3 +68,5 @@ void esxUtil_ReplaceSpecialWindowsPathChars(char *string);
 char *esxUtil_EscapeDatastoreItem(const char *string);
 
 char *esxUtil_EscapeForXml(const char *string);
+
+void esxUtil_EscapeInventoryObject(virBuffer *buf, const char *string);
-- 
2.52.0

Re: [PATCH 1/2] esx_util: Introduce esxUtil_EscapeInventoryObject()

Posted by Richard W.M. Jones via Devel 3 weeks, 3 days ago

On Thu, Jan 08, 2026 at 09:23:23AM +0100, Michal Privoznik via Devel wrote:
> From: Michal Privoznik <mprivozn@redhat.com>
> 
> The aim of this helper function is to URI-encode given string
> twice. There's a bug (fixed in next commit) in which we're unable
> to fetch .vmx file for a domain if corresponding datastore
> contains some special characters (like +). Cole Robinson
> discovered that encoding datastore twice enables libvirt to work
> around the issue [2]. Well, this function does exactly that.
> It was tested with the following inputs and all worked
> flawlessly: "datastore", "datastore2", "datastore2+",
> "datastore3+-@", "data store2+".
> 
> 1: https://issues.redhat.com/browse/RHEL-134127
> 2: https://issues.redhat.com/browse/RHEL-133729#comment-28604072
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/esx/esx_util.c | 17 +++++++++++++++++
>  src/esx/esx_util.h |  3 +++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c
> index 7ee0e5f7c0..e47ea36730 100644
> --- a/src/esx/esx_util.c
> +++ b/src/esx/esx_util.c
> @@ -448,3 +448,20 @@ esxUtil_EscapeForXml(const char *string)
>  
>      return virBufferContentAndReset(&buffer);
>  }
> +
> +
> +

3 blank lines between functions is OK??

> +/* esxUtil_EscapeInventoryObject:
> + * @buf: the buffer to append to
> + * @string: the string argument which will be URI-encoded
> + *
> + * URI-encode given @string TWICE and append the result to the @buf.
> + */
> +void
> +esxUtil_EscapeInventoryObject(virBuffer *buf, const char *string)

It's nit-picky but should we explain here why we are double-encoding
the string (ie. to workaround a VMware bug)?

But other than that, for the whole series:

Reviewed-by: Richard W.M. Jones <rjones@redhat.com>

Rich.

> +{
> +    g_autoptr(GString) escaped = g_string_new(NULL);
> +
> +    g_string_append_uri_escaped(escaped, string, NULL, false);
> +    virBufferURIEncodeString(buf, escaped->str);
> +}
> diff --git a/src/esx/esx_util.h b/src/esx/esx_util.h
> index 58bc44e744..29f01e0c15 100644
> --- a/src/esx/esx_util.h
> +++ b/src/esx/esx_util.h
> @@ -22,6 +22,7 @@
>  #pragma once
>  
>  #include "internal.h"
> +#include "virbuffer.h"
>  #include "viruri.h"
>  
>  #define ESX_VI_CHECK_ARG_LIST(val) \
> @@ -67,3 +68,5 @@ void esxUtil_ReplaceSpecialWindowsPathChars(char *string);
>  char *esxUtil_EscapeDatastoreItem(const char *string);
>  
>  char *esxUtil_EscapeForXml(const char *string);
> +
> +void esxUtil_EscapeInventoryObject(virBuffer *buf, const char *string);
> -- 
> 2.52.0

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

Re: [PATCH 1/2] esx_util: Introduce esxUtil_EscapeInventoryObject()

Posted by Michal Prívozník via Devel 3 weeks, 3 days ago

On 1/8/26 11:09, Richard W.M. Jones wrote:
> On Thu, Jan 08, 2026 at 09:23:23AM +0100, Michal Privoznik via Devel wrote:
>> From: Michal Privoznik <mprivozn@redhat.com>
>>
>> The aim of this helper function is to URI-encode given string
>> twice. There's a bug (fixed in next commit) in which we're unable
>> to fetch .vmx file for a domain if corresponding datastore
>> contains some special characters (like +). Cole Robinson
>> discovered that encoding datastore twice enables libvirt to work
>> around the issue [2]. Well, this function does exactly that.
>> It was tested with the following inputs and all worked
>> flawlessly: "datastore", "datastore2", "datastore2+",
>> "datastore3+-@", "data store2+".
>>
>> 1: https://issues.redhat.com/browse/RHEL-134127
>> 2: https://issues.redhat.com/browse/RHEL-133729#comment-28604072
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>  src/esx/esx_util.c | 17 +++++++++++++++++
>>  src/esx/esx_util.h |  3 +++
>>  2 files changed, 20 insertions(+)
>>
>> diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c
>> index 7ee0e5f7c0..e47ea36730 100644
>> --- a/src/esx/esx_util.c
>> +++ b/src/esx/esx_util.c
>> @@ -448,3 +448,20 @@ esxUtil_EscapeForXml(const char *string)
>>  
>>      return virBufferContentAndReset(&buffer);
>>  }
>> +
>> +
>> +
> 
> 3 blank lines between functions is OK??

It matches style used in the file. But yeah, I can drop this one extra
line and post a patch to drops the other extra lines.

> 
>> +/* esxUtil_EscapeInventoryObject:
>> + * @buf: the buffer to append to
>> + * @string: the string argument which will be URI-encoded
>> + *
>> + * URI-encode given @string TWICE and append the result to the @buf.
>> + */
>> +void
>> +esxUtil_EscapeInventoryObject(virBuffer *buf, const char *string)
> 
> It's nit-picky but should we explain here why we are double-encoding
> the string (ie. to workaround a VMware bug)?

It's in the commit message, but yeah, I'll add it into a comment too.
Does the following sound reasonable?

 * URI-encode given @string TWICE and append the result to the @buf. This is
 * to be used with inventory objects (like 'dcPath' and 'dsName') to work
 * around a VMware bug in which once round of URI-encoding is not enough.


> 
> But other than that, for the whole series:
> 
> Reviewed-by: Richard W.M. Jones <rjones@redhat.com>

Thanks!

Michal

Re: [PATCH 1/2] esx_util: Introduce esxUtil_EscapeInventoryObject()

Posted by Richard W.M. Jones via Devel 3 weeks, 3 days ago

On Thu, Jan 08, 2026 at 01:52:47PM +0100, Michal Prívozník wrote:
> On 1/8/26 11:09, Richard W.M. Jones wrote:
> > On Thu, Jan 08, 2026 at 09:23:23AM +0100, Michal Privoznik via Devel wrote:
> >> From: Michal Privoznik <mprivozn@redhat.com>
> >>
> >> The aim of this helper function is to URI-encode given string
> >> twice. There's a bug (fixed in next commit) in which we're unable
> >> to fetch .vmx file for a domain if corresponding datastore
> >> contains some special characters (like +). Cole Robinson
> >> discovered that encoding datastore twice enables libvirt to work
> >> around the issue [2]. Well, this function does exactly that.
> >> It was tested with the following inputs and all worked
> >> flawlessly: "datastore", "datastore2", "datastore2+",
> >> "datastore3+-@", "data store2+".
> >>
> >> 1: https://issues.redhat.com/browse/RHEL-134127
> >> 2: https://issues.redhat.com/browse/RHEL-133729#comment-28604072
> >>
> >> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> >> ---
> >>  src/esx/esx_util.c | 17 +++++++++++++++++
> >>  src/esx/esx_util.h |  3 +++
> >>  2 files changed, 20 insertions(+)
> >>
> >> diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c
> >> index 7ee0e5f7c0..e47ea36730 100644
> >> --- a/src/esx/esx_util.c
> >> +++ b/src/esx/esx_util.c
> >> @@ -448,3 +448,20 @@ esxUtil_EscapeForXml(const char *string)
> >>  
> >>      return virBufferContentAndReset(&buffer);
> >>  }
> >> +
> >> +
> >> +
> > 
> > 3 blank lines between functions is OK??
> 
> It matches style used in the file. But yeah, I can drop this one extra
> line and post a patch to drops the other extra lines.
> 
> > 
> >> +/* esxUtil_EscapeInventoryObject:
> >> + * @buf: the buffer to append to
> >> + * @string: the string argument which will be URI-encoded
> >> + *
> >> + * URI-encode given @string TWICE and append the result to the @buf.
> >> + */
> >> +void
> >> +esxUtil_EscapeInventoryObject(virBuffer *buf, const char *string)
> > 
> > It's nit-picky but should we explain here why we are double-encoding
> > the string (ie. to workaround a VMware bug)?
> 
> It's in the commit message, but yeah, I'll add it into a comment too.
> Does the following sound reasonable?
> 
>  * URI-encode given @string TWICE and append the result to the @buf. This is
>  * to be used with inventory objects (like 'dcPath' and 'dsName') to work
>  * around a VMware bug in which once round of URI-encoding is not enough.

Seems fine, thanks.

Rich.

> 
> > 
> > But other than that, for the whole series:
> > 
> > Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
> 
> Thanks!
> 
> Michal

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW