fix: percent-encode ';' in form values

PveHttpClient.EncodeFormValue encoded &, =, +, space, and % but left ';' literal. PVE's application/x-www-form-urlencoded parser treats a raw ';' as a field separator (the historical alternative to '&'), so a value like boot=order=scsi0;ide2 was split into 'boot=order=scsi0' plus an empty 'ide2' field, and PVE rejected the PUT with "ide2: unable to parse drive options". This broke any multi-device boot order set via Set-PveVmConfig -AdditionalConfig, and any other value containing ';'. Encode ';' as %3B. Safe under the existing minimal-encoding policy that keeps ':' and '!' literal for cluster-join: cluster-join payloads never contain ';', and PVE url-decodes config form values. Tracked as F088. Closes #64. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-07-26 07:58:14 +00:00 · 2026-05-22 14:04:31 -05:00
parent 5e5a8fcda4
commit 3fd770d84c
3 changed files with 134 additions and 0 deletions
@@ -0,0 +1,99 @@
+using System;
+using System.Collections.Generic;
+using System.Net;
+using System.Net.Http;
+using System.Reflection;
+using System.Threading;
+using System.Threading.Tasks;
+using PSProxmoxVE.Core.Authentication;
+using PSProxmoxVE.Core.Client;
+using Xunit;
+
+namespace PSProxmoxVE.Core.Tests.Client
+{
+    public class PveHttpClientFormEncodingTests
+    {
+        private static void SetInnerHttpClient(PveHttpClient client, HttpClient newInner)
+        {
+            var field = typeof(PveHttpClient).GetField("_httpClient",
+                BindingFlags.Instance | BindingFlags.NonPublic)!;
+            ((HttpClient)field.GetValue(client)!).Dispose();
+            field.SetValue(client, newInner);
+        }
+
+        private static PveSession NewSession()
+        {
+            return new PveSession("pve.example.com", 8006, false,
+                "root@pam!token=aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee");
+        }
+
+        private static (PveHttpClient client, CapturingHandler handler) NewCapturingClient()
+        {
+            var client = new PveHttpClient(NewSession());
+            var handler = new CapturingHandler();
+            SetInnerHttpClient(client, new HttpClient(handler));
+            return (client, handler);
+        }
+
+        [Fact]
+        public async Task PostAsync_SemicolonInValue_IsPercentEncoded()
+        {
+            var (client, handler) = NewCapturingClient();
+            using (client)
+            {
+                await client.PostAsync("nodes/pve/qemu/100/config",
+                    new Dictionary<string, string> { ["boot"] = "order=scsi0;ide2" });
+            }
+
+            // PVE treats a raw ';' as a form-field separator, so it must be encoded.
+            Assert.Contains("boot=order%3Dscsi0%3Bide2", handler.LastBody);
+            Assert.DoesNotContain(";", handler.LastBody);
+        }
+
+        [Fact]
+        public async Task PutAsync_SemicolonInValue_IsPercentEncoded()
+        {
+            var (client, handler) = NewCapturingClient();
+            using (client)
+            {
+                await client.PutAsync("nodes/pve/qemu/100/config",
+                    new Dictionary<string, string> { ["boot"] = "order=ide2;virtio0;net0" });
+            }
+
+            Assert.Contains("boot=order%3Dide2%3Bvirtio0%3Bnet0", handler.LastBody);
+            Assert.DoesNotContain(";", handler.LastBody);
+        }
+
+        [Fact]
+        public async Task PostAsync_CommaAndColonInValue_RemainLiteral()
+        {
+            var (client, handler) = NewCapturingClient();
+            using (client)
+            {
+                // Drive options use literal commas; cluster-join values use literal colons.
+                // Minimal-encoding policy must keep both unescaped.
+                await client.PostAsync("nodes/pve/qemu/100/config",
+                    new Dictionary<string, string> { ["ide2"] = "nas:iso/win.iso,media=cdrom" });
+            }
+
+            Assert.Contains("ide2=nas:iso/win.iso,media%3Dcdrom", handler.LastBody);
+        }
+
+        private sealed class CapturingHandler : HttpMessageHandler
+        {
+            public string LastBody { get; private set; } = string.Empty;
+
+            protected override async Task<HttpResponseMessage> SendAsync(
+                HttpRequestMessage request, CancellationToken cancellationToken)
+            {
+                LastBody = request.Content == null
+                    ? string.Empty
+                    : await request.Content.ReadAsStringAsync().ConfigureAwait(false);
+                return new HttpResponseMessage(HttpStatusCode.OK)
+                {
+                    Content = new StringContent("{\"data\":null}")
+                };
+            }
+        }
+    }
+}