[PATCH 0/2] esx: URI encode inventory objects twice
*** BLURB HERE *** Michal Prívozník (2): esx_util: Introduce esxUtil_EscapeInventoryObject() esx: URI encode inventory objects twice src/esx/esx_driver.c | 8 ++++---- src/esx/esx_util.c | 17 +++++++++++++++++ src/esx/esx_util.h | 3 +++ 3 files changed, 24 insertions(+), 4 deletions(-) -- 2.52.0
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
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
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
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
From: Michal Privoznik <mprivozn@redhat.com> While discouraged by a KB article to use special characters in inventory object names [1], ESX won't stop you. And thus users can end up with a datastore named "datastore2+", for instance. The datastore name (and datacenter path) are important when fetching/uploading a .vmx file (used in APIs like virDomainGetXMLDesc() or virDomainDefineXML()). And while we do URI encode both (dcPath and dsName), encoding them once is not enough. Cole Robinson discovered [2] that they need to be URI-encoded twice. Use newly introduced esxUtil_EscapeInventoryObject() helper to encode them twice. 1: https://knowledge.broadcom.com/external/article/386368/vcenter-inventory-obj... 2: https://issues.redhat.com/browse/RHEL-133729#comment-28604072 Resolves: https://issues.redhat.com/browse/RHEL-134127 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/esx/esx_driver.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 9f965811b1..02f30c2b19 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -2567,9 +2567,9 @@ esxDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) domain->conn->uri->server, domain->conn->uri->port); virBufferURIEncodeString(&buffer, directoryAndFileName); virBufferAddLit(&buffer, "?dcPath="); - virBufferURIEncodeString(&buffer, priv->primary->datacenterPath); + esxUtil_EscapeInventoryObject(&buffer, priv->primary->datacenterPath); virBufferAddLit(&buffer, "&dsName="); - virBufferURIEncodeString(&buffer, datastoreName); + esxUtil_EscapeInventoryObject(&buffer, datastoreName); url = virBufferContentAndReset(&buffer); @@ -3002,9 +3002,9 @@ esxDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) virBufferURIEncodeString(&buffer, escapedName); virBufferAddLit(&buffer, ".vmx?dcPath="); - virBufferURIEncodeString(&buffer, priv->primary->datacenterPath); + esxUtil_EscapeInventoryObject(&buffer, priv->primary->datacenterPath); virBufferAddLit(&buffer, "&dsName="); - virBufferURIEncodeString(&buffer, datastoreName); + esxUtil_EscapeInventoryObject(&buffer, datastoreName); url = virBufferContentAndReset(&buffer); -- 2.52.0
On Thu, Jan 08, 2026 at 09:23:22 +0100, Michal Privoznik wrote:
*** BLURB HERE ***
Michal Prívozník (2): esx_util: Introduce esxUtil_EscapeInventoryObject() esx: URI encode inventory objects twice
src/esx/esx_driver.c | 8 ++++---- src/esx/esx_util.c | 17 +++++++++++++++++ src/esx/esx_util.h | 3 +++ 3 files changed, 24 insertions(+), 4 deletions(-)
Sigh. Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
participants (4)
-
Jiri Denemark -
Michal Privoznik -
Michal Prívozník
-
Richard W.M. Jones