On 06/19/2018 09:47 AM, Cole Robinson wrote:
On 06/18/2018 07:52 PM, John Ferlan wrote:
On 06/18/2018 01:57 PM, Anya Harter wrote:
Add comma escaping for cfg->spiceTLSx509certdir and graphics->data.spice.rendernode.
Signed-off-by: Anya Harter <aharter@redhat.com> --- src/qemu/qemu_command.c | 11 ++++++++--- tests/qemuxml2argvdata/name-escape.args | 5 +++-- tests/qemuxml2argvdata/name-escape.xml | 1 + tests/qemuxml2argvtest.c | 5 +++++ 4 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a9a5e200e9..36dccea9b2 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7974,8 +7974,11 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, !cfg->spicePassword) virBufferAddLit(&opt, "disable-ticketing,");
- if (hasSecure) - virBufferAsprintf(&opt, "x509-dir=%s,", cfg->spiceTLSx509certdir); + if (hasSecure) { + virBufferAddLit(&opt, "x509-dir="); + virQEMUBuildBufferEscapeComma(&opt, cfg->spiceTLSx509certdir); + virBufferAsprintf(&opt, ",");
make syntax-check would have told you:
src/qemu/qemu_command.c:7980: virBufferAsprintf(&opt, ","); src/qemu/qemu_command.c:8090: virBufferAsprintf(&opt, ",");
So here and below, I changed to virBufferAddLit before pushing.
+ }
switch (graphics->data.spice.defaultMode) { case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: @@ -8082,7 +8085,9 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, goto error; }
- virBufferAsprintf(&opt, "rendernode=%s,", graphics->data.spice.rendernode); + virBufferAddLit(&opt, "rendernode="); + virQEMUBuildBufferEscapeComma(&opt, graphics->data.spice.rendernode); + virBufferAsprintf(&opt, ","); } }
Reviewed-by: John Ferlan <jferlan@redhat.com>
John
From the last time I passed through this when Sukrit posted patches, still to do are qemuBuildHostNetStr, qemuBuildDiskThrottling (for group name), and various qemuBuildNetworkDriveURI, qemuBuildNetworkDriveStr, and qemuGetDriveSourceString.
Oh cool, I didn't realize you had found more examples! I looked at some of these with Anya before the patch series.
NetworkDriveURI is a private subset of NetworkDriveStr, so the former doesn't need any direct changes AFAICT.
qemuGetDriveSourceString is called outside qemu_command.c, for example passing the result to qemu monitor commands. Anyone know if comma escaping applies there too? Same with qemuBuildHostNetStr
From what I can tell qemuGetDriveSourceString and qemuBuildHostNetStr usages outside of qemu_command.c should _not_ have comma escaping, which makes sense as the comma isn't used as a delimiter in those substrings. So the comma escaping should be done at the call sites of those functions in qemu_command.c
Thanks, Cole